Fix: Assertion failure in RawScrollbarState with dynamic controller assignment #173156
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This PR correctly fixes an assertion failure in RawScrollbarState by deferring the scroll position validation until user interaction. The logic change is sound and well-commented.
I've added one suggestion to improve the new test case by making it more robust and ensuring it fully verifies the scrollbar's interactivity after the dynamic controller assignment, which also aligns better with the test's comments.
Comment on lines
3699
to
3708
| // Verify scrollbar is now working properly | ||
| expect(find.byType(RawScrollbar), findsOneWidget); | ||
|
|
||
| // Verify that user can interact with the scrollbar without errors | ||
| final Finder scrollbarFinder = find.byType(RawScrollbar); | ||
| expect(scrollbarFinder, findsOneWidget); | ||
|
|
||
| // The scrollbar should be functional for interaction now | ||
| final RenderBox scrollbarBox = tester.renderObject(scrollbarFinder); | ||
| expect(scrollbarBox, isNotNull); |
Contributor
There was a problem hiding this comment.
The comments here suggest that user interaction is verified, but the test only checks for the widget's presence. To make the test more robust and align with the comments, you should add a step to interact with the scrollbar, for example, by dragging the thumb.
This will also verify that the assertion, which was moved to the interaction handlers, does not fail when the user interacts with the scrollbar.
Additionally, the check for find.byType(RawScrollbar) is redundant.
// Verify scrollbar is now working properly and is interactive.
final Finder scrollbarFinder = find.byType(RawScrollbar);
expect(scrollbarFinder, findsOneWidget);
expect(scrollController.offset, 0.0);
// Drag the thumb to ensure it's interactive and doesn't assert.
// The scrollbar should be at the top-right of the scrollable area.
final Offset thumbPos = tester.getTopRight(scrollbarFinder) + const Offset(-3.0, 20.0);
final TestGesture gesture = await tester.startGesture(thumbPos);
await tester.pumpAndSettle(); // Wait for gesture to be recognized.
await gesture.moveBy(const Offset(0.0, 50.0));
await tester.pumpAndSettle();
// Check that the scroll view has scrolled.
expect(scrollController.offset, greaterThan(0.0));
await gesture.up();
rkishan516
force-pushed
the
scrollbar-controller-assertion
branch
from
7335b6d to
87d7f83
Compare
Piinks
reviewed
Aug 4, 2025
Contributor
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Hey @rkishan516 it looks like there are many tests failing currently. Can you take a look?
| // for interaction, ensure we are set up properly. | ||
| assert(_debugCheckHasValidScrollPosition()); | ||
| } | ||
| // Validation is performed only when the user interacts with the scrollbar, |
Contributor
There was a problem hiding this comment.
I am not sure the method here makes sense then, _validateInteractions. Should there be more updates here?
Contributor
Author
There was a problem hiding this comment.
Logically, It can be removed.
Contributor
Author
Sure, I will fix. Edit: Will try to pickup by friday or weekend. |
rkishan516
force-pushed
the
scrollbar-controller-assertion
branch
from
87d7f83 to
55381cf
Compare
| // Interactive scrollbars need to be properly configured. If it is visible | ||
| // for interaction, ensure we are set up properly. | ||
| // Don't assert immediately if we're in the middle of updating the widget | ||
| // as the controller may not be attached yet in that frame |
Contributor
There was a problem hiding this comment.
super nit: Add a period at the end of this comment.
| expect(scrollbarFinder, findsOneWidget); | ||
| expect(scrollController.offset, 0.0); | ||
|
|
||
| // Test that scrolling works without assertions |
Contributor
There was a problem hiding this comment.
nit: add period at the end of this comment and the comment below.
| ElevatedButton( | ||
| onPressed: () { | ||
| setState(() { | ||
| // This reproduces the exact scenario from the issue: |
Contributor
There was a problem hiding this comment.
nit: maybe this can be:
// This reproduces the exact scenario from the linked issue:
// Initially the controller is null, thumbVisibility is false, and interactive is false.
// Then, in the next frame, the controller is non-null, thumbVisibility is true, and interactive is true.
55381cf to
e0c7ef1
Compare
Contributor
Author
|
@Renzo-Olivares I have addressed all the comments. |
| await tester.drag(find.byType(SingleChildScrollView), const Offset(0.0, -100.0)); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| // Verify the scroll happened |
Contributor
There was a problem hiding this comment.
nit: period at the end of this comment.
Renzo-Olivares
approved these changes
Aug 29, 2025
Contributor
Renzo-Olivares
left a comment
Renzo-Olivares
left a comment
There was a problem hiding this comment.
LGTM pending one last comment. Thank you for the contribution!
rkishan516
force-pushed
the
scrollbar-controller-assertion
branch
from
e0c7ef1 to
04154e2
Compare
Renzo-Olivares
added
the
autosubmit
flutter-dashboard
bot
removed
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 3, 2025
…ntroller assignment (flutter/flutter#173156)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 3, 2025
…ntroller assignment (flutter/flutter#173156)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 3, 2025
…ntroller assignment (flutter/flutter#173156)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 4, 2025
…ntroller assignment (flutter/flutter#173156)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 4, 2025
…ntroller assignment (flutter/flutter#173156)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 4, 2025
…ntroller assignment (flutter/flutter#173156)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 4, 2025
…ntroller assignment (flutter/flutter#173156)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 5, 2025
…ntroller assignment (flutter/flutter#173156)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 5, 2025
…ntroller assignment (flutter/flutter#173156)
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Sep 5, 2025
Roll Flutter from 6b18740d5a23 to 87d5b753196c (88 revisions) flutter/flutter@6b18740...87d5b75 2025-09-05 [email protected] [ Device Lab ] Add regression testing for flutter/flutter#174952 (flutter/flutter#174956) 2025-09-05 [email protected] Roll Skia from 0ca53adfc6cc to 845ec125e94c (2 revisions) (flutter/flutter#174978) 2025-09-05 [email protected] [a11y-app] Fix NavigationRail leading and trailing labels (flutter/flutter#174861) 2025-09-05 [email protected] Roll Skia from d7e99be07d5d to 0ca53adfc6cc (5 revisions) (flutter/flutter#174972) 2025-09-05 [email protected] Roll Fuchsia Linux SDK from izfNA3o_2zL4Cjqmy... to xG_uERsxHvUwFHpF2... (flutter/flutter#174970) 2025-09-04 [email protected] Fix IconButton.color overrided by IconButtomTheme (flutter/flutter#174515) 2025-09-04 [email protected] [web] Reuse chrome instance to run all flutter tests (flutter/flutter#174957) 2025-09-04 [email protected] fix(Semantics): Ensure semantics properties take priority over button's (flutter/flutter#174473) 2025-09-04 [email protected] Make every LLDB Init error message actionable (flutter/flutter#174726) 2025-09-04 [email protected] Fix table cell semantics rect alignment issues. (flutter/flutter#174914) 2025-09-04 [email protected] Fix: Use route navigator for CupertinoSheetRoute pop (flutter/flutter#173103) 2025-09-04 [email protected] [ Widget Preview] Add `group` property to `Preview` (flutter/flutter#174849) 2025-09-04 [email protected] Allow OverlayPortal.overlayChildLayoutBuilder to choose root Overlay (flutter/flutter#174239) 2025-09-04 [email protected] Roll Skia from 7c2f502e3304 to d7e99be07d5d (18 revisions) (flutter/flutter#174936) 2025-09-04 [email protected] Remove 'terms of use' wording from web_unicode (flutter/flutter#174939) 2025-09-04 [email protected] [ Tool ] Remove leftover Android x86 deprecation warning constant (flutter/flutter#174941) 2025-09-04 [email protected] Prevent potential crash when accessing window in FlutterSceneDelegate (flutter/flutter#174873) 2025-09-04 [email protected] Roll Packages from 42bb347 to 98580c6 (5 revisions) (flutter/flutter#174943) 2025-09-04 [email protected] [ Device Lab ] Fix wakefulness check to only match log entries with string values (flutter/flutter#174953) 2025-09-04 [email protected] Fix expanded DropdownMenu panel is shorter than text field (flutter/flutter#174443) 2025-09-04 [email protected] Add a `viewController` property to the ios/macOS `FlutterPluginRegistrar` protocol (flutter/flutter#174168) 2025-09-03 [email protected] Roll Fuchsia Linux SDK from J3T_wZqL_57mRfWky... to izfNA3o_2zL4Cjqmy... (flutter/flutter#174908) 2025-09-03 [email protected] Wires up Android API to set section locale (flutter/flutter#173364) 2025-09-03 [email protected] Delete impeller::SPrintF. (flutter/flutter#174900) 2025-09-03 [email protected] Make sure that a DropdownMenuFormField doesn't crash in 0x0 environment (flutter/flutter#174777) 2025-09-03 [email protected] Remove unnecessary `presubmit_max_attempts` from .ci.yaml (flutter/flutter#174885) 2025-09-03 [email protected] Use local canvaskit in `dart_data_asset_test.dart` (flutter/flutter#174891) 2025-09-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Mark Linux web_canvaskit_tests_7_last as bringup (#174878)" (flutter/flutter#174897) 2025-09-03 [email protected] Correct intrinsics calculation for CupertinoTextField with placeholder (flutter/flutter#174889) 2025-09-03 [email protected] Considers large title height in CupertinoNavigationBar's preferred size (flutter/flutter#173722) 2025-09-03 [email protected] [A11y] Add semantics for CupertinoExpansionTile (flutter/flutter#174480) 2025-09-03 [email protected] Fix LinearProgressIndicator track painting. (flutter/flutter#173108) 2025-09-03 [email protected] update triage documentation to include team-android (flutter/flutter#174850) 2025-09-03 [email protected] Update `test_timeout_secs` to match `timeout` for `Linux web_skwasm_tests_*` and `Linux web_canvaskit_tests_*` (flutter/flutter#174881) 2025-09-03 [email protected] Roll Packages from 5d785a0 to 42bb347 (10 revisions) (flutter/flutter#174876) 2025-09-03 [email protected] Fixup formatting of gn files in the old buildroot. (flutter/flutter#174852) 2025-09-03 [email protected] Roll Dart SDK to 3.10.0-162.1.beta (flutter/flutter#174834) 2025-09-03 [email protected] Mark Linux web_canvaskit_tests_7_last as bringup (flutter/flutter#174878) 2025-09-02 [email protected] [Impeller] Fix overdraw in DrawRect geometry (flutter/flutter#174735) 2025-09-02 [email protected] Patch .clang-format files to specify C++20. (flutter/flutter#174848) 2025-09-02 [email protected] Add data assets (flutter/flutter#174685) 2025-09-02 [email protected] refactors `FlutterPlugin.kt` to use one line statement in the `into` bloc (flutter/flutter#174759) 2025-09-02 [email protected] Ensures initial semantics state is sent to engine (flutter/flutter#174845) 2025-09-02 [email protected] [Android] Break up plugin_test integration tests (flutter/flutter#174728) 2025-09-02 [email protected] Fix: Assertion failure in RawScrollbarState with dynamic controller assignment (flutter/flutter#173156) 2025-09-02 [email protected] Roll Skia from 359f3d7cc7ed to 7c2f502e3304 (1 revision) (flutter/flutter#174844) ...
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Sep 18, 2025
…ssignment (flutter#173156) Fix: Assertion failure in RawScrollbarState with dynamic controller assignment fixes: flutter#172614 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
Jaineel-Mamtora
pushed a commit
to Jaineel-Mamtora/flutter_forked
that referenced
this pull request
Sep 24, 2025
…ssignment (flutter#173156) Fix: Assertion failure in RawScrollbarState with dynamic controller assignment fixes: flutter#172614 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
danferreira
pushed a commit
to danferreira/packages
that referenced
this pull request
Oct 22, 2025
…r#9962) Roll Flutter from 6b18740d5a23 to 87d5b753196c (88 revisions) flutter/flutter@6b18740...87d5b75 2025-09-05 [email protected] [ Device Lab ] Add regression testing for flutter/flutter#174952 (flutter/flutter#174956) 2025-09-05 [email protected] Roll Skia from 0ca53adfc6cc to 845ec125e94c (2 revisions) (flutter/flutter#174978) 2025-09-05 [email protected] [a11y-app] Fix NavigationRail leading and trailing labels (flutter/flutter#174861) 2025-09-05 [email protected] Roll Skia from d7e99be07d5d to 0ca53adfc6cc (5 revisions) (flutter/flutter#174972) 2025-09-05 [email protected] Roll Fuchsia Linux SDK from izfNA3o_2zL4Cjqmy... to xG_uERsxHvUwFHpF2... (flutter/flutter#174970) 2025-09-04 [email protected] Fix IconButton.color overrided by IconButtomTheme (flutter/flutter#174515) 2025-09-04 [email protected] [web] Reuse chrome instance to run all flutter tests (flutter/flutter#174957) 2025-09-04 [email protected] fix(Semantics): Ensure semantics properties take priority over button's (flutter/flutter#174473) 2025-09-04 [email protected] Make every LLDB Init error message actionable (flutter/flutter#174726) 2025-09-04 [email protected] Fix table cell semantics rect alignment issues. (flutter/flutter#174914) 2025-09-04 [email protected] Fix: Use route navigator for CupertinoSheetRoute pop (flutter/flutter#173103) 2025-09-04 [email protected] [ Widget Preview] Add `group` property to `Preview` (flutter/flutter#174849) 2025-09-04 [email protected] Allow OverlayPortal.overlayChildLayoutBuilder to choose root Overlay (flutter/flutter#174239) 2025-09-04 [email protected] Roll Skia from 7c2f502e3304 to d7e99be07d5d (18 revisions) (flutter/flutter#174936) 2025-09-04 [email protected] Remove 'terms of use' wording from web_unicode (flutter/flutter#174939) 2025-09-04 [email protected] [ Tool ] Remove leftover Android x86 deprecation warning constant (flutter/flutter#174941) 2025-09-04 [email protected] Prevent potential crash when accessing window in FlutterSceneDelegate (flutter/flutter#174873) 2025-09-04 [email protected] Roll Packages from 42bb347 to 98580c6 (5 revisions) (flutter/flutter#174943) 2025-09-04 [email protected] [ Device Lab ] Fix wakefulness check to only match log entries with string values (flutter/flutter#174953) 2025-09-04 [email protected] Fix expanded DropdownMenu panel is shorter than text field (flutter/flutter#174443) 2025-09-04 [email protected] Add a `viewController` property to the ios/macOS `FlutterPluginRegistrar` protocol (flutter/flutter#174168) 2025-09-03 [email protected] Roll Fuchsia Linux SDK from J3T_wZqL_57mRfWky... to izfNA3o_2zL4Cjqmy... (flutter/flutter#174908) 2025-09-03 [email protected] Wires up Android API to set section locale (flutter/flutter#173364) 2025-09-03 [email protected] Delete impeller::SPrintF. (flutter/flutter#174900) 2025-09-03 [email protected] Make sure that a DropdownMenuFormField doesn't crash in 0x0 environment (flutter/flutter#174777) 2025-09-03 [email protected] Remove unnecessary `presubmit_max_attempts` from .ci.yaml (flutter/flutter#174885) 2025-09-03 [email protected] Use local canvaskit in `dart_data_asset_test.dart` (flutter/flutter#174891) 2025-09-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Mark Linux web_canvaskit_tests_7_last as bringup (#174878)" (flutter/flutter#174897) 2025-09-03 [email protected] Correct intrinsics calculation for CupertinoTextField with placeholder (flutter/flutter#174889) 2025-09-03 [email protected] Considers large title height in CupertinoNavigationBar's preferred size (flutter/flutter#173722) 2025-09-03 [email protected] [A11y] Add semantics for CupertinoExpansionTile (flutter/flutter#174480) 2025-09-03 [email protected] Fix LinearProgressIndicator track painting. (flutter/flutter#173108) 2025-09-03 [email protected] update triage documentation to include team-android (flutter/flutter#174850) 2025-09-03 [email protected] Update `test_timeout_secs` to match `timeout` for `Linux web_skwasm_tests_*` and `Linux web_canvaskit_tests_*` (flutter/flutter#174881) 2025-09-03 [email protected] Roll Packages from 5d785a0 to 42bb347 (10 revisions) (flutter/flutter#174876) 2025-09-03 [email protected] Fixup formatting of gn files in the old buildroot. (flutter/flutter#174852) 2025-09-03 [email protected] Roll Dart SDK to 3.10.0-162.1.beta (flutter/flutter#174834) 2025-09-03 [email protected] Mark Linux web_canvaskit_tests_7_last as bringup (flutter/flutter#174878) 2025-09-02 [email protected] [Impeller] Fix overdraw in DrawRect geometry (flutter/flutter#174735) 2025-09-02 [email protected] Patch .clang-format files to specify C++20. (flutter/flutter#174848) 2025-09-02 [email protected] Add data assets (flutter/flutter#174685) 2025-09-02 [email protected] refactors `FlutterPlugin.kt` to use one line statement in the `into` bloc (flutter/flutter#174759) 2025-09-02 [email protected] Ensures initial semantics state is sent to engine (flutter/flutter#174845) 2025-09-02 [email protected] [Android] Break up plugin_test integration tests (flutter/flutter#174728) 2025-09-02 [email protected] Fix: Assertion failure in RawScrollbarState with dynamic controller assignment (flutter/flutter#173156) 2025-09-02 [email protected] Roll Skia from 359f3d7cc7ed to 7c2f502e3304 (1 revision) (flutter/flutter#174844) ...
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 12, 2025
…ntroller assignment (flutter/flutter#173156)
lucaantonelli
pushed a commit
to lucaantonelli/flutter
that referenced
this pull request
Nov 21, 2025
…ssignment (flutter#173156) Fix: Assertion failure in RawScrollbarState with dynamic controller assignment fixes: flutter#172614 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix: Assertion failure in RawScrollbarState with dynamic controller assignment
fixes: #172614
Pre-launch Checklist
///).