Fix inherited widget notifications in Localizations (#12213)

* Fix inherited widget notifications in Localizations

* address comments

packages/flutter/lib/src/material/tabs.dart

@@ -794,7 +794,7 @@ class _TabBarState extends State<TabBar> {
 }
 
 /// A page view that displays the widget which corresponds to the currently
-/// selected tab. Typically used in conjuction with a [TabBar].
+/// selected tab. Typically used in conjunction with a [TabBar].
 ///
 /// If a [TabController] is not provided, then there must be a [DefaultTabController]
 /// ancestor.

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

@@ -3287,9 +3287,31 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
   @mustCallSuper
   void didChangeDependencies() {
     assert(_active); // otherwise markNeedsBuild is a no-op
+    _debugCheckOwnerBuildTargetExists('didChangeDependencies');
     markNeedsBuild();
   }
 
+  void _debugCheckOwnerBuildTargetExists(String methodName) {
+    assert(() {
+      if (owner._debugCurrentBuildTarget == null) {
+        throw new FlutterError(
+          '$methodName for ${widget.runtimeType} was called at an '
+          'inappropriate time.\n'
+          '\n'
+          'It may only be called while the widgets are being built. A possible '
+          'cause of this error is when $methodName is called during '
+          'one of:\n'
+          '\n'
+          ' * network I/O event\n'
+          ' * file I/O event\n'
+          ' * timer\n'
+          ' * microtask (caused by Future.then, async/await, scheduleMicrotask)'
+        );
+      }
+      return true;
+    });
+  }
+
   /// Returns a description of what caused this element to be created.
   ///
   /// Useful for debugging the source of an element.
@@ -3952,6 +3974,7 @@ class InheritedElement extends ProxyElement {
   /// by first obtaining their [InheritedElement] using
   /// [BuildContext.ancestorInheritedElementForWidgetOfExactType].
   void dispatchDidChangeDependencies() {
+    _debugCheckOwnerBuildTargetExists('dispatchDidChangeDependencies');
     for (Element dependent in _dependents) {
       assert(() {
         // check that it really is our descendant

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

@@ -206,6 +206,7 @@ class _LocalizationsScope extends InheritedWidget {
     Key key,
     @required this.locale,
     @required this.localizationsState,
+    @required this.loadGeneration,
     Widget child,
   }) : super(key: key, child: child) {
     assert(localizationsState != null);
@@ -214,10 +215,14 @@ class _LocalizationsScope extends InheritedWidget {
   final Locale locale;
   final _LocalizationsState localizationsState;
 
+  /// A monotonically increasing number that changes after localizations
+  /// delegates have finished loading new data. When this number changes, it
+  /// triggers inherited widget notifications.
+  final int loadGeneration;
+
   @override
   bool updateShouldNotify(_LocalizationsScope old) {
-    // Changes in Localizations.locale trigger a load(), see _LocalizationsState.didUpdateWidget()
+    return loadGeneration != old.loadGeneration;
-    return false;
   }
 }
 
@@ -431,6 +436,11 @@ class _LocalizationsState extends State<Localizations> {
   final GlobalKey _localizedResourcesScopeKey = new GlobalKey();
   Map<Type, dynamic> _typeToResources = <Type, dynamic>{};
 
+  /// A monotonically increasing number that increases after localizations
+  /// delegates have finished loading new data, triggering inherited widget
+  /// notifications.
+  int _loadGeneration = 0;
+
   Locale get locale => _locale;
   Locale _locale;
 
@@ -494,9 +504,8 @@ class _LocalizationsState extends State<Localizations> {
         setState(() {
           _typeToResources = value;
           _locale = locale;
+          _loadGeneration += 1;
         });
-        final InheritedElement scopeElement = _localizedResourcesScopeKey.currentContext;
-        scopeElement?.dispatchDidChangeDependencies();
       });
     }
   }
@@ -521,6 +530,7 @@ class _LocalizationsState extends State<Localizations> {
       key: _localizedResourcesScopeKey,
       locale: _locale,
       localizationsState: this,
+      loadGeneration: _loadGeneration,
       child: new Directionality(
         textDirection: _textDirection,
         child: widget.child,

packages/flutter/test/material/localizations_test.dart

@@ -25,6 +25,18 @@ class FooMaterialLocalizationsDelegate extends LocalizationsDelegate<MaterialLoc
   bool shouldReload(FooMaterialLocalizationsDelegate old) => false;
 }
 
+/// A localizations delegate that does not contain any useful data, and is only
+/// used to trigger didChangeDependencies upon locale change.
+class _DummyLocalizationsDelegate extends LocalizationsDelegate<DummyLocalizations> {
+  @override
+  Future<DummyLocalizations> load(Locale locale) async => new DummyLocalizations();
+
+  @override
+  bool shouldReload(_DummyLocalizationsDelegate old) => true;
+}
+
+class DummyLocalizations {}
+
 Widget buildFrame({
   Locale locale,
   Iterable<LocalizationsDelegate<dynamic>> delegates,
@@ -293,4 +305,21 @@ void main() {
     expect(tester.widget<Text>(find.byKey(textKey)).data, 'id_JV');
   });
 
+  testWidgets('Localizations is compatible with ChangeNotifier.dispose() called during didChangeDependencies', (WidgetTester tester) async {
+    // PageView calls ScrollPosition.dispose() during didChangeDependencies.
+    await tester.pumpWidget(new MaterialApp(
+      supportedLocales: const <Locale>[
+        const Locale('en', 'US'),
+        const Locale('es', 'ES'),
+      ],
+      localizationsDelegates: <_DummyLocalizationsDelegate>[
+        new _DummyLocalizationsDelegate(),
+      ],
+      home: new PageView(),
+    ));
+
+    await tester.binding.setLocale('es', 'US');
+    await tester.pump();
+    await tester.pumpWidget(new Container());
+  });
 }