From bcc93bca2388401d57d5cc2ba70c4a33bfde7066 Mon Sep 17 00:00:00 2001
From: Michael Goderbauer <goderbauer@google.com>
Date: Wed, 13 Nov 2019 11:31:20 -0800
Subject: [PATCH] Make disposing a ScrollPosition with pixels=null legal
 (#44617)

---
 .../flutter/lib/src/widgets/page_view.dart    | 24 -------
 .../lib/src/widgets/scroll_position.dart      |  1 -
 packages/flutter/test/material/tabs_test.dart |  3 +-
 .../widgets/scrollable_in_overlay_test.dart   | 64 +++++++++++++++++++
 4 files changed, 65 insertions(+), 27 deletions(-)
 create mode 100644 packages/flutter/test/widgets/scrollable_in_overlay_test.dart

diff --git a/packages/flutter/lib/src/widgets/page_view.dart b/packages/flutter/lib/src/widgets/page_view.dart
index 1e7ab5346c1..9580501dcd0 100644
--- a/packages/flutter/lib/src/widgets/page_view.dart
+++ b/packages/flutter/lib/src/widgets/page_view.dart
@@ -333,29 +333,6 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
   final int initialPage;
   double _pageToUseOnStartup;
 
-  /// If [pixels] isn't set by [applyViewportDimension] before [dispose] is
-  /// called, this could throw an assert as [pixels] will be set to null.
-  ///
-  /// With [Tab]s, this happens when there are nested [TabBarView]s and there
-  /// is an attempt to warp over the nested tab to a tab adjacent to it.
-  ///
-  /// This flag will be set to true once the dimensions have been established
-  /// and [pixels] is set.
-  bool isInitialPixelsValueSet = false;
-
-  @override
-  void dispose() {
-    // TODO(shihaohong): remove workaround once these issues have been
-    // resolved, https://github.com/flutter/flutter/issues/32054,
-    // https://github.com/flutter/flutter/issues/32056
-    // Sets `pixels` to a non-null value before `ScrollPosition.dispose` is
-    // invoked if it was never set by `applyViewportDimension`.
-    if (pixels == null && !isInitialPixelsValueSet) {
-      correctPixels(0);
-    }
-    super.dispose();
-  }
-
   @override
   double get viewportFraction => _viewportFraction;
   double _viewportFraction;
@@ -422,7 +399,6 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
 
     if (newPixels != oldPixels) {
       correctPixels(newPixels);
-      isInitialPixelsValueSet = true;
       return false;
     }
     return result;
diff --git a/packages/flutter/lib/src/widgets/scroll_position.dart b/packages/flutter/lib/src/widgets/scroll_position.dart
index f1ade32c8ff..a661a339e7f 100644
--- a/packages/flutter/lib/src/widgets/scroll_position.dart
+++ b/packages/flutter/lib/src/widgets/scroll_position.dart
@@ -693,7 +693,6 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
 
   @override
   void dispose() {
-    assert(pixels != null);
     activity?.dispose(); // it will be null if it got absorbed by another ScrollPosition
     _activity = null;
     super.dispose();
diff --git a/packages/flutter/test/material/tabs_test.dart b/packages/flutter/test/material/tabs_test.dart
index f43a22c3094..8c2ceef5cf3 100644
--- a/packages/flutter/test/material/tabs_test.dart
+++ b/packages/flutter/test/material/tabs_test.dart
@@ -978,8 +978,7 @@ void main() {
     expect(tabController.index, 0);
   });
 
-  testWidgets('Nested TabBarView sets ScrollController pixels to non-null value '
-  'when disposed before it is set by the applyViewportDimension', (WidgetTester tester) async {
+  testWidgets('Can switch to non-neighboring tab in nested TabBarView without crashing', (WidgetTester tester) async {
     // This is a regression test for https://github.com/flutter/flutter/issues/18756
     final TabController _mainTabController = TabController(length: 4, vsync: const TestVSync());
     final TabController _nestedTabController = TabController(length: 2, vsync: const TestVSync());
diff --git a/packages/flutter/test/widgets/scrollable_in_overlay_test.dart b/packages/flutter/test/widgets/scrollable_in_overlay_test.dart
new file mode 100644
index 00000000000..41e6bad6971
--- /dev/null
+++ b/packages/flutter/test/widgets/scrollable_in_overlay_test.dart
@@ -0,0 +1,64 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter/material.dart';
+
+void main() {
+  test('Can dispose ScrollPosition when pixels is null', () {
+    final ScrollPosition position = ScrollPositionWithSingleContext(
+      initialPixels: null,
+      keepScrollOffset: false,
+      physics: const AlwaysScrollableScrollPhysics(),
+      context: ScrollableState(),
+    );
+
+    expect(position.pixels, isNull);
+    position.dispose(); // Should not throw/assert.
+  });
+
+  testWidgets('scrollable in hidden overlay does not crash when unhidden', (WidgetTester tester) async {
+    // Regression test for https://github.com/flutter/flutter/issues/44269.
+    final TabController controller = TabController(vsync: const TestVSync(), length: 1);
+
+    final OverlayEntry entry1 = OverlayEntry(
+      maintainState: true,
+      opaque: true,
+      builder: (BuildContext context) {
+        return TabBar(
+          isScrollable: true,
+          controller: controller,
+          tabs: const <Tab>[
+            Tab(text: 'Main'),
+          ],
+        );
+      },
+    );
+    final OverlayEntry entry2 = OverlayEntry(
+      maintainState: true,
+      opaque: true,
+      builder: (BuildContext context) {
+        return const Text('number2');
+      },
+    );
+
+    await tester.pumpWidget(
+      MaterialApp(
+        home: Material(
+          child: Overlay(
+            initialEntries: <OverlayEntry>[
+              entry1,
+              entry2,
+            ],
+          ),
+        ),
+      ),
+    );
+
+    entry2.remove();
+    await tester.pump();
+    expect(tester.takeException(), isNull);
+  });
+}