Reland "Remove hack from PageView." (#142172)

Original PR: https://github.com/flutter/flutter/pull/141533
Failure: https://fusion2.corp.google.com/presubmit/601217743/OCL:601217743:BASE:601219708:1706132224874:9a4bcab3/targets
Fix: [b/321743868](https://b.corp.google.com/321743868), http://cl/601219001 (added as g3 fix.

examples/api/test/widgets/page_view/page_view.0_test.dart

@@ -49,7 +49,7 @@ void main() {
 
     // Verify that page view index is also updated with same index to page indicator.
     final PageView pageView = tester.widget<PageView>(find.byType(PageView));
-    expect(pageView.controller.page, 1);
+    expect(pageView.controller!.page, 1);
 
     // Tap backward button on page indicator area.
     await tester.tap(find.byIcon(Icons.arrow_left_rounded));

packages/flutter/lib/src/widgets/page_view.dart

@@ -576,11 +576,6 @@ class PageScrollPhysics extends ScrollPhysics {
   bool get allowImplicitScrolling => false;
 }
 
-// Having this global (mutable) page controller is a bit of a hack. We need it
-// to plumb in the factory for _PagePosition, but it will end up accumulating
-// a large list of scroll positions. As long as you don't try to actually
-// control the scroll positions, everything should be fine.
-final PageController _defaultPageController = PageController();
 const PageScrollPhysics _kPagePhysics = PageScrollPhysics();
 
 /// A scrollable list that works page by page.
@@ -645,7 +640,7 @@ class PageView extends StatefulWidget {
     super.key,
     this.scrollDirection = Axis.horizontal,
     this.reverse = false,
-    PageController? controller,
+    this.controller,
     this.physics,
     this.pageSnapping = true,
     this.onPageChanged,
@@ -656,8 +651,7 @@ class PageView extends StatefulWidget {
     this.clipBehavior = Clip.hardEdge,
     this.scrollBehavior,
     this.padEnds = true,
-  }) : controller = controller ?? _defaultPageController,
-       childrenDelegate = SliverChildListDelegate(children);
+  }) : childrenDelegate = SliverChildListDelegate(children);
 
   /// Creates a scrollable list that works page by page using widgets that are
   /// created on demand.
@@ -688,7 +682,7 @@ class PageView extends StatefulWidget {
     super.key,
     this.scrollDirection = Axis.horizontal,
     this.reverse = false,
-    PageController? controller,
+    this.controller,
     this.physics,
     this.pageSnapping = true,
     this.onPageChanged,
@@ -701,8 +695,7 @@ class PageView extends StatefulWidget {
     this.clipBehavior = Clip.hardEdge,
     this.scrollBehavior,
     this.padEnds = true,
-  }) : controller = controller ?? _defaultPageController,
-       childrenDelegate = SliverChildBuilderDelegate(
+  }) : childrenDelegate = SliverChildBuilderDelegate(
          itemBuilder,
          findChildIndexCallback: findChildIndexCallback,
          childCount: itemCount,
@@ -719,11 +712,11 @@ class PageView extends StatefulWidget {
   /// {@end-tool}
   ///
   /// {@macro flutter.widgets.PageView.allowImplicitScrolling}
-  PageView.custom({
+  const PageView.custom({
     super.key,
     this.scrollDirection = Axis.horizontal,
     this.reverse = false,
-    PageController? controller,
+    this.controller,
     this.physics,
     this.pageSnapping = true,
     this.onPageChanged,
@@ -734,7 +727,7 @@ class PageView extends StatefulWidget {
     this.clipBehavior = Clip.hardEdge,
     this.scrollBehavior,
     this.padEnds = true,
-  }) : controller = controller ?? _defaultPageController;
+  });
 
   /// Controls whether the widget's pages will respond to
   /// [RenderObject.showOnScreen], which will allow for implicit accessibility
@@ -776,7 +769,7 @@ class PageView extends StatefulWidget {
 
   /// An object that can be used to control the position to which this page
   /// view is scrolled.
-  final PageController controller;
+  final PageController? controller;
 
   /// How the page view should respond to user input.
   ///
@@ -848,10 +841,37 @@ class PageView extends StatefulWidget {
 class _PageViewState extends State<PageView> {
   int _lastReportedPage = 0;
 
+  late PageController _controller;
+
   @override
   void initState() {
     super.initState();
-    _lastReportedPage = widget.controller.initialPage;
+    _initController();
+    _lastReportedPage = _controller.initialPage;
+  }
+
+  @override
+  void dispose() {
+    if (widget.controller == null) {
+      _controller.dispose();
+    }
+    super.dispose();
+  }
+
+
+  void _initController() {
+    _controller = widget.controller ?? PageController();
+  }
+
+  @override
+  void didUpdateWidget(PageView oldWidget) {
+    if (oldWidget.controller != widget.controller) {
+      if (oldWidget.controller == null) {
+        _controller.dispose();
+      }
+      _initController();
+    }
+    super.didUpdateWidget(oldWidget);
   }
 
   AxisDirection _getDirection(BuildContext context) {
@@ -892,7 +912,7 @@ class _PageViewState extends State<PageView> {
       child: Scrollable(
         dragStartBehavior: widget.dragStartBehavior,
         axisDirection: axisDirection,
-        controller: widget.controller,
+        controller: _controller,
         physics: physics,
         restorationId: widget.restorationId,
         scrollBehavior: widget.scrollBehavior ?? ScrollConfiguration.of(context).copyWith(scrollbars: false),
@@ -908,7 +928,7 @@ class _PageViewState extends State<PageView> {
             clipBehavior: widget.clipBehavior,
             slivers: <Widget>[
               SliverFillViewport(
-                viewportFraction: widget.controller.viewportFraction,
+                viewportFraction: _controller.viewportFraction,
                 delegate: widget.childrenDelegate,
                 padEnds: widget.padEnds,
               ),
@@ -924,7 +944,7 @@ class _PageViewState extends State<PageView> {
     super.debugFillProperties(description);
     description.add(EnumProperty<Axis>('scrollDirection', widget.scrollDirection));
     description.add(FlagProperty('reverse', value: widget.reverse, ifTrue: 'reversed'));
-    description.add(DiagnosticsProperty<PageController>('controller', widget.controller, showName: false));
+    description.add(DiagnosticsProperty<PageController>('controller', _controller, showName: false));
     description.add(DiagnosticsProperty<ScrollPhysics>('physics', widget.physics, showName: false));
     description.add(FlagProperty('pageSnapping', value: widget.pageSnapping, ifFalse: 'snapping disabled'));
     description.add(FlagProperty('allowImplicitScrolling', value: widget.allowImplicitScrolling, ifTrue: 'allow implicit scrolling'));

packages/flutter/test/material/tabs_test.dart

@@ -1521,7 +1521,7 @@ void main() {
     expect(tabController.index, 1);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     // The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -1570,7 +1570,7 @@ void main() {
     expect(tabController.index, 0);
 
     final PageView pageView = tester.widget<PageView>(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     expect(position.pixels, 0.0);
@@ -1631,7 +1631,7 @@ void main() {
     expect(tabController.index, 1);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
 
     // The TabView was initialized with viewportFraction as 0.8
     // So it's expected the PageView inside would obtain the same viewportFraction
@@ -1674,7 +1674,7 @@ void main() {
     expect(tabController.index, 1);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
 
     // The TabView was initialized with default viewportFraction
     // So it's expected the PageView inside would obtain the value 1
@@ -1719,13 +1719,13 @@ void main() {
 
     await tester.pumpWidget(buildFrame(0.8));
     PageView pageView = tester.widget(find.byType(PageView));
-    PageController pageController = pageView.controller;
+    PageController pageController = pageView.controller!;
     expect(pageController.viewportFraction, 0.8);
 
     // Rebuild with a different viewport fraction.
     await tester.pumpWidget(buildFrame(0.5));
     pageView = tester.widget(find.byType(PageView));
-    pageController = pageView.controller;
+    pageController = pageView.controller!;
     expect(pageController.viewportFraction, 0.5);
   });
 
@@ -1900,7 +1900,7 @@ void main() {
     expect(tabController.index, 1);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     // The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -1944,7 +1944,7 @@ void main() {
     expect(tabController.index, 0);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     // The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -1989,7 +1989,7 @@ void main() {
     expect(tabController.index, 0);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     // The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -2038,7 +2038,7 @@ void main() {
     expect(tabController.index, 0);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     // The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -2089,7 +2089,7 @@ void main() {
     expect(tabController.index, 0);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     // The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -2285,7 +2285,7 @@ void main() {
     expect(tabController.index, 1);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     // The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -2471,7 +2471,7 @@ void main() {
     expect(tabController.index, 1);
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     // The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -4247,7 +4247,7 @@ void main() {
 
     await tester.pumpWidget(buildFrame(15));
     PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController1 = pageView.controller;
+    final PageController pageController1 = pageView.controller!;
     TabController tabController = DefaultTabController.of(tester.element(find.text('Page 14')));
     expect(tabController.index, 14);
     expect(pageController1.page, 14);
@@ -4255,7 +4255,7 @@ void main() {
     // Rebuild with a new default tab controller with more tabs.
     await tester.pumpWidget(buildFrame(10));
     pageView = tester.widget(find.byType(PageView));
-    final PageController pageController2 = pageView.controller;
+    final PageController pageController2 = pageView.controller!;
     tabController = DefaultTabController.of(tester.element(find.text('Page 9')));
     expect(tabController.index, 9);
     expect(pageController2.page, 9);
@@ -5104,7 +5104,7 @@ void main() {
     double expectedIndicatorLeft = canvas.indicatorRect.left;
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     void pageControllerListener() {
       // Whenever TabBarView scrolls due to changing TabController's index,
       // check if indicator stays idle in its expectedIndicatorLeft
@@ -5264,7 +5264,7 @@ void main() {
     ));
 
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     final ScrollPosition position = pageController.position;
 
     expect(tabController.index, 0);
@@ -5754,7 +5754,7 @@ void main() {
 
     await tester.pumpWidget(buildFrame(controller1, showLast: true));
     final PageView pageView = tester.widget(find.byType(PageView));
-    final PageController pageController = pageView.controller;
+    final PageController pageController = pageView.controller!;
     await tester.tap(find.text('three'));
     await tester.pumpAndSettle();
     expect(controller1.index, 2);
@@ -5821,7 +5821,7 @@ void main() {
 
     await tester.pumpWidget(buildFrame(controller1, showLast: true));
     PageView pageView = tester.widget(find.byType(PageView));
-    PageController pageController = pageView.controller;
+    PageController pageController = pageView.controller!;
     await tester.tap(find.text('three'));
     await tester.pumpAndSettle();
     expect(controller1.index, 2);
@@ -5831,7 +5831,7 @@ void main() {
     await tester.pumpWidget(buildFrame(controller2, showLast: false));
     await tester.pumpAndSettle();
     pageView = tester.widget(find.byType(PageView));
-    pageController = pageView.controller;
+    pageController = pageView.controller!;
     expect(controller2.index, 0);
     expect(pageController.page, 0);
 
@@ -5839,7 +5839,7 @@ void main() {
     await tester.pumpWidget(buildFrame(controller1, showLast: true));
     await tester.pumpAndSettle();
     pageView = tester.widget(find.byType(PageView));
-    pageController = pageView.controller;
+    pageController = pageView.controller!;
     expect(controller1.index, 2);
     expect(pageController.page, 2);
   });

packages/flutter/test/widgets/page_view_test.dart

@@ -1312,4 +1312,77 @@ void main() {
     expect(attach, 1);
     expect(detach, 1);
   });
+
+  group('$PageView handles change of controller', () {
+    final GlobalKey key = GlobalKey();
+
+    Widget createPageView(PageController? controller) {
+      return MaterialApp(
+        home: Scaffold(
+          body: PageView(
+            key: key,
+            controller: controller,
+            children: const <Widget>[
+              Center(child: Text('0')),
+              Center(child: Text('1')),
+              Center(child: Text('2')),
+            ],
+          ),
+        ),
+      );
+    }
+
+    Future<void> testPageViewWithController(PageController controller, WidgetTester tester, bool controls) async  {
+      int curentVisiblePage() {
+        return int.parse(tester.widgetList(find.byType(Text)).whereType<Text>().first.data!);
+      }
+
+      final int initialPageInView = curentVisiblePage();
+
+      for (int i = 0; i < 3; i++) {
+        if (controls) {
+          controller.jumpToPage(i);
+          await tester.pumpAndSettle();
+          expect(curentVisiblePage(), i);
+        } else {
+          expect(()=> controller.jumpToPage(i), throwsAssertionError);
+          expect(curentVisiblePage(), initialPageInView);
+        }
+      }
+    }
+
+    testWidgets('null to value', (WidgetTester tester) async {
+      final PageController controller = PageController();
+      addTearDown(controller.dispose);
+      await tester.pumpWidget(createPageView(null));
+      await tester.pumpWidget(createPageView(controller));
+      await testPageViewWithController(controller, tester, true);
+    });
+
+    testWidgets('value to value', (WidgetTester tester) async {
+      final PageController controller1 = PageController();
+      addTearDown(controller1.dispose);
+      final PageController controller2 = PageController();
+      addTearDown(controller2.dispose);
+      await tester.pumpWidget(createPageView(controller1));
+      await testPageViewWithController(controller1, tester, true);
+      await tester.pumpWidget(createPageView(controller2));
+      await testPageViewWithController(controller1, tester, false);
+      await testPageViewWithController(controller2, tester, true);
+    });
+
+    testWidgets('value to null', (WidgetTester tester) async {
+      final PageController controller = PageController();
+      addTearDown(controller.dispose);
+      await tester.pumpWidget(createPageView(controller));
+      await testPageViewWithController(controller, tester, true);
+      await tester.pumpWidget(createPageView(null));
+      await testPageViewWithController(controller, tester, false);
+    });
+
+    testWidgets('null to null', (WidgetTester tester) async {
+      await tester.pumpWidget(createPageView(null));
+      await tester.pumpWidget(createPageView(null));
+    });
+  });
 }