Fixes PageView.onPageChanged to only report changes in range (#8553)

* clamp the pixels used in the page calculation to be within min + max extents to avoid pages outside of the range of children

* add test to check onPageChanged not fired on overscroll

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

@@ -176,7 +176,7 @@ class _PagePosition extends ScrollPosition {
     return page * viewportDimension * viewportFraction;
   }
 
-  double get page => pixels == null ? null : getPageFromPixels(pixels, viewportDimension);
+  double get page => pixels == null ? null : getPageFromPixels(pixels.clamp(minScrollExtent, maxScrollExtent), viewportDimension);
 
   @override
   bool applyViewportDimension(double viewportDimension) {

packages/flutter/test/widgets/page_view_test.dart

@@ -28,7 +28,7 @@ void main() {
             child: new Text(state),
           ),
         );
-      }).toList()
+      }).toList(),
     ));
 
     await tester.tap(find.text('Alabama'));
@@ -60,7 +60,8 @@ void main() {
     expect(log, equals(<String>['Alaska']));
     log.clear();
 
-    await tester.fling(find.byType(PageView), const Offset(-200.0, 0.0), 1000.0);
+    await tester.fling(
+        find.byType(PageView), const Offset(-200.0, 0.0), 1000.0);
     await tester.pumpUntilNoTransientCallbacks(_frameDuration);
 
     expect(find.text('Alabama'), findsNothing);
@@ -75,7 +76,8 @@ void main() {
     expect(find.text('Arizona'), findsNothing);
   });
 
-  testWidgets('PageView does not squish when overscrolled', (WidgetTester tester) async {
+  testWidgets('PageView does not squish when overscrolled',
+      (WidgetTester tester) async {
     await tester.pumpWidget(new MaterialApp(
       theme: new ThemeData(platform: TargetPlatform.iOS),
       home: new PageView(
@@ -91,7 +93,8 @@ void main() {
     ));
 
     Size sizeOf(int i) => tester.getSize(find.byKey(new ValueKey<int>(i)));
-    double leftOf(int i) => tester.getTopLeft(find.byKey(new ValueKey<int>(i))).x;
+    double leftOf(int i) =>
+        tester.getTopLeft(find.byKey(new ValueKey<int>(i))).x;
 
     expect(leftOf(0), equals(0.0));
     expect(sizeOf(0), equals(const Size(800.0, 600.0)));
@@ -118,15 +121,18 @@ void main() {
         height: 400.0,
         child: new PageView(
           controller: controller,
-          children: kStates.map<Widget>((String state) => new Text(state)).toList(),
+          children:
+              kStates.map<Widget>((String state) => new Text(state)).toList(),
         ),
       ),
     ));
 
     expect(find.text('California'), findsOneWidget);
 
-    controller.nextPage(duration: const Duration(milliseconds: 150), curve: Curves.ease);
-    await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100));
+    controller.nextPage(
+        duration: const Duration(milliseconds: 150), curve: Curves.ease);
+    await tester
+        .pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100));
 
     expect(find.text('Colorado'), findsOneWidget);
 
@@ -136,15 +142,18 @@ void main() {
         height: 400.0,
         child: new PageView(
           controller: controller,
-          children: kStates.map<Widget>((String state) => new Text(state)).toList(),
+          children:
+              kStates.map<Widget>((String state) => new Text(state)).toList(),
         ),
       ),
     ));
 
     expect(find.text('Colorado'), findsOneWidget);
 
-    controller.previousPage(duration: const Duration(milliseconds: 150), curve: Curves.ease);
-    await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100));
+    controller.previousPage(
+        duration: const Duration(milliseconds: 150), curve: Curves.ease);
+    await tester
+        .pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100));
 
     expect(find.text('California'), findsOneWidget);
   });
@@ -155,7 +164,8 @@ void main() {
         width: 600.0,
         height: 400.0,
         child: new PageView(
-          children: kStates.map<Widget>((String state) => new Text(state)).toList(),
+          children:
+              kStates.map<Widget>((String state) => new Text(state)).toList(),
         ),
       ),
     ));
@@ -163,7 +173,8 @@ void main() {
     expect(find.text('Alabama'), findsOneWidget);
 
     await tester.scroll(find.byType(PageView), const Offset(-1250.0, 0.0));
-    await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100));
+    await tester
+        .pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100));
 
     expect(find.text('Arizona'), findsOneWidget);
 
@@ -172,7 +183,8 @@ void main() {
         width: 250.0,
         height: 100.0,
         child: new PageView(
-          children: kStates.map<Widget>((String state) => new Text(state)).toList(),
+          children:
+              kStates.map<Widget>((String state) => new Text(state)).toList(),
         ),
       ),
     ));
@@ -184,7 +196,8 @@ void main() {
         width: 450.0,
         height: 400.0,
         child: new PageView(
-          children: kStates.map<Widget>((String state) => new Text(state)).toList(),
+          children:
+              kStates.map<Widget>((String state) => new Text(state)).toList(),
         ),
       ),
     ));
@@ -198,7 +211,8 @@ void main() {
         width: 0.0,
         height: 0.0,
         child: new PageView(
-          children: kStates.map<Widget>((String state) => new Text(state)).toList(),
+          children:
+              kStates.map<Widget>((String state) => new Text(state)).toList(),
         ),
       ),
     ));
@@ -210,7 +224,8 @@ void main() {
         width: 200.0,
         height: 200.0,
         child: new PageView(
-          children: kStates.map<Widget>((String state) => new Text(state)).toList(),
+          children:
+              kStates.map<Widget>((String state) => new Text(state)).toList(),
         ),
       ),
     ));
@@ -221,13 +236,16 @@ void main() {
   testWidgets('Page changes at halfway point', (WidgetTester tester) async {
     final List<int> log = <int>[];
     await tester.pumpWidget(new PageView(
-      onPageChanged: (int page) { log.add(page); },
+      onPageChanged: (int page) {
+        log.add(page);
+      },
       children: kStates.map<Widget>((String state) => new Text(state)).toList(),
     ));
 
     expect(log, isEmpty);
 
-    final TestGesture gesture = await tester.startGesture(const Point(100.0, 100.0));
+    final TestGesture gesture =
+        await tester.startGesture(const Point(100.0, 100.0));
     // The page view is 800.0 wide, so this move is just short of halfway.
     await gesture.moveBy(const Offset(-380.0, 0.0));
 
@@ -274,10 +292,12 @@ void main() {
         itemBuilder: (BuildContext context, int index) {
           return new Container(
             height: 200.0,
-            color: index % 2 == 0 ? const Color(0xFF0000FF) : const Color(0xFF00FF00),
+            color: index % 2 == 0
+                ? const Color(0xFF0000FF)
+                : const Color(0xFF00FF00),
             child: new Text(kStates[index]),
           );
-        }
+        },
       );
     }
 
@@ -312,10 +332,12 @@ void main() {
         itemBuilder: (BuildContext context, int index) {
           return new Container(
             height: 200.0,
-            color: index % 2 == 0 ? const Color(0xFF0000FF) : const Color(0xFF00FF00),
+            color: index % 2 == 0
+                ? const Color(0xFF0000FF)
+                : const Color(0xFF00FF00),
             child: new Text(kStates[index]),
           );
-        }
+        },
       );
     }
 
@@ -330,7 +352,8 @@ void main() {
     controller.jumpToPage(10);
     await tester.pump();
 
-    expect(tester.getTopLeft(find.text('Connecticut')), const Point(-50.0, 0.0));
+    expect(
+        tester.getTopLeft(find.text('Connecticut')), const Point(-50.0, 0.0));
     expect(tester.getTopLeft(find.text('Delaware')), const Point(50.0, 0.0));
     expect(tester.getTopLeft(find.text('Florida')), const Point(150.0, 0.0));
     expect(tester.getTopLeft(find.text('Georgia')), const Point(250.0, 0.0));
@@ -342,7 +365,8 @@ void main() {
   });
 
   testWidgets('PageView large viewportFraction', (WidgetTester tester) async {
-    final PageController controller = new PageController(viewportFraction: 5/4);
+    final PageController controller =
+        new PageController(viewportFraction: 5/4);
 
     Widget build(PageController controller) {
       return new PageView.builder(
@@ -351,21 +375,48 @@ void main() {
         itemBuilder: (BuildContext context, int index) {
           return new Container(
             height: 200.0,
-            color: index % 2 == 0 ? const Color(0xFF0000FF) : const Color(0xFF00FF00),
+            color: index % 2 == 0
+                ? const Color(0xFF0000FF)
+                : const Color(0xFF00FF00),
             child: new Text(kStates[index]),
           );
-        }
+        },
       );
     }
 
     await tester.pumpWidget(build(controller));
 
     expect(tester.getTopLeft(find.text('Alabama')), const Point(-100.0, 0.0));
-    expect(tester.getBottomRight(find.text('Alabama')), const Point(900.0, 600.0));
+    expect(
+        tester.getBottomRight(find.text('Alabama')), const Point(900.0, 600.0));
 
     controller.jumpToPage(10);
     await tester.pump();
 
     expect(tester.getTopLeft(find.text('Hawaii')), const Point(-100.0, 0.0));
   });
+
+  testWidgets('PageView does not report page changed on overscroll',
+      (WidgetTester tester) async {
+    final PageController controller =
+        new PageController(initialPage: kStates.length - 1);
+    int changeIndex = 0;
+    Widget build() {
+      return new PageView(
+        children:
+            kStates.map<Widget>((String state) => new Text(state)).toList(),
+        controller: controller,
+        onPageChanged: (int page) {
+          changeIndex = page;
+        },
+      );
+    }
+
+    await tester.pumpWidget(build());
+    controller.jumpToPage(kStates.length * 2); // try to move beyond max range
+    // change index should be zero, shouldn't fire onPageChanged
+    expect(changeIndex, 0);
+    await tester.pump();
+    expect(changeIndex, 0);
+  });
 }