From dc3d6603f2a4d587df7511ea914e1474589637e1 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Thu, 11 May 2023 17:31:23 +0200 Subject: [PATCH] Make dpr available during Scrollable disposal (#126535) Fixes https://github.com/flutter/flutter/issues/126454. In certain situations (see linked bug) the Scrollable needs access to the DPR of the View it is drawn into during disposal. Since inherited widget lookups are not allowed during disposal, we cannot look up the DPR dynamically in these situations. Instead, we have to cache the DPR when lookups are still allowed. --- .../lib/src/widgets/scroll_context.dart | 4 ++ .../lib/src/widgets/scroll_position.dart | 4 +- .../flutter/lib/src/widgets/scrollable.dart | 8 +++- .../test/widgets/nested_scroll_view_test.dart | 37 +++++++++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/packages/flutter/lib/src/widgets/scroll_context.dart b/packages/flutter/lib/src/widgets/scroll_context.dart index c83dd11243..19406f0c92 100644 --- a/packages/flutter/lib/src/widgets/scroll_context.dart +++ b/packages/flutter/lib/src/widgets/scroll_context.dart @@ -42,6 +42,10 @@ abstract class ScrollContext { /// The direction in which the widget scrolls. AxisDirection get axisDirection; + /// The [FlutterView.devicePixelRatio] of the view that the [Scrollable] this + /// [ScrollContext] is associated with is drawn into. + double get devicePixelRatio; + /// Whether the contents of the widget should ignore [PointerEvent] inputs. /// /// Setting this value to true prevents the use from interacting with the diff --git a/packages/flutter/lib/src/widgets/scroll_position.dart b/packages/flutter/lib/src/widgets/scroll_position.dart index ceedeff737..075b32cfca 100644 --- a/packages/flutter/lib/src/widgets/scroll_position.dart +++ b/packages/flutter/lib/src/widgets/scroll_position.dart @@ -12,7 +12,6 @@ import 'package:flutter/scheduler.dart'; import 'basic.dart'; import 'framework.dart'; -import 'media_query.dart'; import 'notification_listener.dart'; import 'page_storage.dart'; import 'scroll_activity.dart'; @@ -20,7 +19,6 @@ import 'scroll_context.dart'; import 'scroll_metrics.dart'; import 'scroll_notification.dart'; import 'scroll_physics.dart'; -import 'view.dart'; export 'scroll_activity.dart' show ScrollHoldController; @@ -241,7 +239,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { } @override - double get devicePixelRatio => MediaQuery.maybeDevicePixelRatioOf(context.storageContext) ?? View.of(context.storageContext).devicePixelRatio; + double get devicePixelRatio => context.devicePixelRatio; /// Update the scroll position ([pixels]) to a given pixel value. /// diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index 3b96584198..994071efcc 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -28,6 +28,7 @@ import 'scrollable_helpers.dart'; import 'selectable_region.dart'; import 'selection_container.dart'; import 'ticker_provider.dart'; +import 'view.dart'; import 'viewport.dart'; export 'package:flutter/physics.dart' show Tolerance; @@ -530,6 +531,10 @@ class ScrollableState extends State with TickerProviderStateMixin, R @override TickerProvider get vsync => this; + @override + double get devicePixelRatio => _devicePixelRatio; + late double _devicePixelRatio; + @override BuildContext? get notificationContext => _gestureDetectorKey.currentContext; @@ -596,6 +601,7 @@ class ScrollableState extends State with TickerProviderStateMixin, R @override void didChangeDependencies() { _mediaQueryGestureSettings = MediaQuery.maybeGestureSettingsOf(context); + _devicePixelRatio = MediaQuery.maybeDevicePixelRatioOf(context) ?? View.of(context).devicePixelRatio; _updatePosition(); super.didChangeDependencies(); } @@ -980,7 +986,7 @@ class ScrollableState extends State with TickerProviderStateMixin, R @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); - properties.add(DiagnosticsProperty('position', position)); + properties.add(DiagnosticsProperty('position', _position)); properties.add(DiagnosticsProperty('effective physics', _physics)); } } diff --git a/packages/flutter/test/widgets/nested_scroll_view_test.dart b/packages/flutter/test/widgets/nested_scroll_view_test.dart index c062cba1fd..c418746510 100644 --- a/packages/flutter/test/widgets/nested_scroll_view_test.dart +++ b/packages/flutter/test/widgets/nested_scroll_view_test.dart @@ -2871,6 +2871,43 @@ void main() { expect(appBarHeight(tester), expandedAppBarHeight); expect(tester.getSize(expandedTitleClip).height, expandedAppBarHeight - collapsedAppBarHeight); }); + + testWidgets('NestedScrollView does not crash when inner scrollable changes while scrolling', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/126454. + Widget buildApp({required bool nested}) { + final Widget innerScrollable = ListView( + children: const [SizedBox(height: 1000)], + ); + return MaterialApp( + home: Scaffold( + body: NestedScrollView( + headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) { + return [ + SliverAppBar( + title: const Text('Books'), + pinned: true, + expandedHeight: 150.0, + forceElevated: innerBoxIsScrolled, + ), + ]; + }, + body: nested ? Container(child: innerScrollable) : innerScrollable, + ), + ), + ); + } + + await tester.pumpWidget(buildApp(nested: false)); + + // Start a scroll. + final TestGesture scrollDrag = await tester.startGesture(tester.getCenter(find.byType(ListView))); + await tester.pump(); + await scrollDrag.moveBy(const Offset(0, 50)); + await tester.pump(); + + // Restructuring inner scrollable while scroll is in progress shouldn't crash. + await tester.pumpWidget(buildApp(nested: true)); + }); } double appBarHeight(WidgetTester tester) => tester.getSize(find.byType(AppBar, skipOffstage: false)).height;