Remove unintended changes

flutter · fluttergithubbot · Sep 3, 2020 · Aug 26, 2020 · Aug 26, 2020 · Aug 26, 2020
commit 8f7593cae7768f070d642b91d91294f0f0a3c94c
diff --git a/packages/flutter/lib/src/foundation/change_notifier.dart b/packages/flutter/lib/src/foundation/change_notifier.dart
@@ -2,12 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:collection';
+
 import 'package:meta/meta.dart';
 
 import 'assertions.dart';
 import 'basic_types.dart';
 import 'diagnostics.dart';
-import 'observer_list.dart';
 
 /// An object that maintains a list of listeners.
 ///
@@ -93,18 +94,22 @@ abstract class ValueListenable<T> extends Listenable {
   T get value;
 }
 
+class _ListenerEntry extends LinkedListEntry<_ListenerEntry> {
+  _ListenerEntry(this.listener);
+  final VoidCallback listener;
+}
+
 /// A class that can be extended or mixed in that provides a change notification
 /// API using [VoidCallback] for notifications.
 ///
-/// [ChangeNotifier] is optimized for small numbers (one or two) of listeners.
-/// It is O(N) for adding and removing listeners and O(N²) for dispatching
+/// It is O(1) for adding listeners and O(N) for removing listeners and dispatching
 /// notifications (where N is the number of listeners).
 ///
 /// See also:
 ///
 ///  * [ValueNotifier], which is a [ChangeNotifier] that wraps a single value.
 class ChangeNotifier implements Listenable {
-  ObserverList<VoidCallback>? _listeners = ObserverList<VoidCallback>();
+  LinkedList<_ListenerEntry>? _listeners = LinkedList<_ListenerEntry>();
 
   bool _debugAssertNotDisposed() {
     assert(() {
@@ -146,7 +151,7 @@ class ChangeNotifier implements Listenable {
   @override
   void addListener(VoidCallback listener) {
     assert(_debugAssertNotDisposed());
-    _listeners!.add(listener);
+    _listeners!.add(_ListenerEntry(listener));
   }
 
   /// Remove a previously registered closure from the list of closures that are
@@ -171,7 +176,12 @@ class ChangeNotifier implements Listenable {
   @override
   void removeListener(VoidCallback listener) {
     assert(_debugAssertNotDisposed());
-    _listeners!.remove(listener);
+    for (final _ListenerEntry entry in _listeners!) {
+      if (entry.listener == listener) {
+        entry.unlink();
+        return;
+      }
+    }
   }
 
   /// Discards any resources used by the object. After this is called, the
@@ -205,27 +215,29 @@ class ChangeNotifier implements Listenable {
   @visibleForTesting
   void notifyListeners() {
     assert(_debugAssertNotDisposed());
-    if (_listeners != null) {
-      final List<VoidCallback> localListeners = List<VoidCallback>.from(_listeners!);
-      for (final VoidCallback listener in localListeners) {
-        try {
-          if (_listeners!.contains(listener))
-            listener();
-        } catch (exception, stack) {
-          FlutterError.reportError(FlutterErrorDetails(
-            exception: exception,
-            stack: stack,
-            library: 'foundation library',
-            context: ErrorDescription('while dispatching notifications for $runtimeType'),
-            informationCollector: () sync* {
-              yield DiagnosticsProperty<ChangeNotifier>(
-                'The $runtimeType sending notification was',
-                this,
-                style: DiagnosticsTreeStyle.errorProperty,
-              );
-            },
-          ));
-        }
+    if (_listeners!.isEmpty)
+      return;
+
+    final List<_ListenerEntry> localListeners = List<_ListenerEntry>.from(_listeners!);
+
+    for (final _ListenerEntry entry in localListeners) {
+      try {
+        if (entry.list != null)
+          entry.listener();
+      } catch (exception, stack) {
+        FlutterError.reportError(FlutterErrorDetails(
+          exception: exception,
+          stack: stack,
+          library: 'foundation library',
+          context: ErrorDescription('while dispatching notifications for $runtimeType'),
+          informationCollector: () sync* {
+            yield DiagnosticsProperty<ChangeNotifier>(
+              'The $runtimeType sending notification was',
+              this,
+              style: DiagnosticsTreeStyle.errorProperty,
+            );
+          },
+        ));
       }
     }
   }

diff --git a/packages/flutter/lib/src/services/asset_bundle.dart b/packages/flutter/lib/src/services/asset_bundle.dart
@@ -71,13 +71,11 @@ abstract class AssetBundle {
     // that the null-handling logic is dead code).
     if (data == null)
       throw FlutterError('Unable to load asset: $key'); // ignore: dead_code
-    // 50 KB of data should take 2-3 ms to parse on a Moto G4, and about 400 μs
-    // on a Pixel 4.
-    if (data.lengthInBytes < 50 * 1024) {
+    if (data.lengthInBytes < 10 * 1024) {
+      // 10KB takes about 3ms to parse on a Pixel 2 XL.
+      // See: https://github.com/dart-lang/sdk/issues/31954
       return utf8.decode(data.buffer.asUint8List());
     }
-    // For strings larger than 50 KB, run the computation in an isolate to
-    // avoid causing main thread jank.
     return compute(_utf8decode, data, debugLabel: 'UTF8 decode for "$key"');
   }
 

diff --git a/packages/flutter/test/foundation/change_notifier_test.dart b/packages/flutter/test/foundation/change_notifier_test.dart
@@ -139,6 +139,67 @@ void main() {
     log.clear();
   });
 
+  test('During notifyListeners, a listener was added and removed immediately', () {
+    final TestNotifier source = TestNotifier();
+    final List<String> log = <String>[];
+
+    final VoidCallback listener3 = () { log.add('listener3'); };
+    final VoidCallback listener2 = () { log.add('listener2'); };
+    void listener1() {
+      log.add('listener1');
+      source.addListener(listener2);
+      source.removeListener(listener2);
+      source.addListener(listener3);
+    }
+
+    source.addListener(listener1);
+
+    source.notify();
+
+    expect(log, <String>['listener1']);
+  });
+
+  test(
+      'If a listener in the middle of the list of listeners removes itself, '
+      'notifyListeners still notifies all listeners', () {
+    final TestNotifier source = TestNotifier();
+    final List<String> log = <String>[];
+
+    void selfRemovingListener() {
+      log.add('selfRemovingListener');
+      source.removeListener(selfRemovingListener);
+    }
+    final VoidCallback listener1 = () { log.add('listener1'); };
+
+    source.addListener(listener1);
+    source.addListener(selfRemovingListener);
+    source.addListener(listener1);
+
+    source.notify();
+
+    expect(log, <String>['listener1', 'selfRemovingListener', 'listener1']);
+  });
+
+  test('If the first listener removes itself, notifyListeners still notify all listeners', () {
+    final TestNotifier source = TestNotifier();
+    final List<String> log = <String>[];
+
+    void selfRemovingListener() {
+      log.add('selfRemovingListener');
+      source.removeListener(selfRemovingListener);
+    }
+    void listener1() {
+      log.add('listener1');
+    }
+
+    source.addListener(selfRemovingListener);
+    source.addListener(listener1);
+
+    source.notifyListeners();
+
+    expect(log, <String>['selfRemovingListener', 'listener1']);
+  });
+
   test('Merging change notifiers', () {
     final TestNotifier source1 = TestNotifier();
     final TestNotifier source2 = TestNotifier();

diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart
@@ -562,11 +562,9 @@ class HotRunner extends ResidentRunner {
         } on vm_service.RPCError {
           // Do nothing, we're already subcribed.
         }
-        // Ideally this would wait for kIsolateRunnable, but either this subscription occurs too
-        // late to receive the event, or it is not forwarded for hot restarted isolates.
         isolateNotifications.add(
           device.vmService.onIsolateEvent.firstWhere((vm_service.Event event) {
-            return event.kind == vm_service.EventKind.kServiceExtensionAdded;
+            return event.kind == vm_service.EventKind.kIsolateRunnable;
           }),
         );
       }