Fix Listenable.merge to not leak (#26313)

packages/flutter/lib/src/animation/listener_helpers.dart

@@ -108,9 +108,11 @@ mixin AnimationLocalListenersMixin {
   ///
   /// Listeners can be added with [addListener].
   void removeListener(VoidCallback listener) {
-    _listeners.remove(listener);
+    final bool removed = _listeners.remove(listener);
+    if (removed) {
       didUnregisterListener();
     }
+  }
 
   /// Calls all the listeners.
   ///
@@ -172,9 +174,11 @@ mixin AnimationLocalStatusListenersMixin {
   ///
   /// Listeners can be added with [addStatusListener].
   void removeStatusListener(AnimationStatusListener listener) {
-    _statusListeners.remove(listener);
+    final bool removed = _statusListeners.remove(listener);
+    if (removed) {
       didUnregisterListener();
     }
+  }
 
   /// Calls all the status listeners.
   ///

packages/flutter/lib/src/foundation/change_notifier.dart

@@ -220,19 +220,23 @@ class ChangeNotifier implements Listenable {
   }
 }
 
-class _MergingListenable extends ChangeNotifier {
-  _MergingListenable(this._children) {
-    for (Listenable child in _children)
-      child?.addListener(notifyListeners);
-  }
+class _MergingListenable extends Listenable {
+  _MergingListenable(this._children);
 
   final List<Listenable> _children;
 
   @override
-  void dispose() {
-    for (Listenable child in _children)
-      child?.removeListener(notifyListeners);
-    super.dispose();
+  void addListener(VoidCallback listener) {
+    for (final Listenable child  in _children) {
+      child?.addListener(listener);
+    }
+  }
+
+  @override
+  void removeListener(VoidCallback listener) {
+    for (final Listenable child in _children) {
+      child?.removeListener(listener);
+    }
   }
 
   @override

packages/flutter/test/animation/listener_helpers_test.dart

+// 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_test/flutter_test.dart';
+import 'package:flutter/animation.dart';
+import 'package:flutter/widgets.dart';
+
+void main() {
+  test('AnimationLocalStatusListenersMixin with AnimationLazyListenerMixin - removing unregistered listener is no-op', () {
+    final _TestAnimationLocalStatusListeners uut = _TestAnimationLocalStatusListeners();
+    final AnimationStatusListener fakeListener = (AnimationStatus status) {};
+    uut.removeStatusListener(fakeListener);
+    expect(uut.callsToStart, 0);
+    expect(uut.callsToStop, 0);
+  });
+
+  test('AnimationLocalListenersMixin with AnimationLazyListenerMixin - removing unregistered listener is no-op', () {
+    final _TestAnimationLocalListeners uut = _TestAnimationLocalListeners();
+    final VoidCallback fakeListener = () {};
+    uut.removeListener(fakeListener);
+    expect(uut.callsToStart, 0);
+    expect(uut.callsToStop, 0);
+  });
+}
+
+class _TestAnimationLocalStatusListeners with AnimationLocalStatusListenersMixin, AnimationLazyListenerMixin {
+  int callsToStart = 0;
+  int callsToStop = 0;
+
+  @override
+  void didStartListening() {
+    callsToStart += 1;
+  }
+
+  @override
+  void didStopListening() {
+    callsToStop += 1;
+  }
+}
+
+class _TestAnimationLocalListeners with AnimationLocalListenersMixin, AnimationLazyListenerMixin {
+  int callsToStart = 0;
+  int callsToStop = 0;
+
+  @override
+  void didStartListening() {
+    callsToStart += 1;
+  }
+
+  @override
+  void didStopListening() {
+    callsToStop += 1;
+  }
+}

packages/flutter/test/foundation/change_notifier_test.dart

@@ -9,6 +9,8 @@ class TestNotifier extends ChangeNotifier {
   void notify() {
     notifyListeners();
   }
+
+  bool get isListenedTo => hasListeners;
 }
 
 class HasListenersTester<T> extends ValueNotifier<T> {
@@ -180,12 +182,12 @@ void main() {
     log.clear();
   });
 
-  test('Can dispose merged notifier', () {
+  test('Can remove from merged notifier', () {
     final TestNotifier source1 = TestNotifier();
     final TestNotifier source2 = TestNotifier();
     final List<String> log = <String>[];
 
-    final ChangeNotifier merged = Listenable.merge(<Listenable>[source1, source2]);
+    final Listenable merged = Listenable.merge(<Listenable>[source1, source2]);
     final VoidCallback listener = () { log.add('listener'); };
 
     merged.addListener(listener);
@@ -193,8 +195,8 @@ void main() {
     source2.notify();
     expect(log, <String>['listener', 'listener']);
     log.clear();
-    merged.dispose();
 
+    merged.removeListener(listener);
     source1.notify();
     source2.notify();
     expect(log, isEmpty);
@@ -229,7 +231,7 @@ void main() {
     final TestNotifier source1 = TestNotifier();
     final TestNotifier source2 = TestNotifier();
 
-    ChangeNotifier listenableUnderTest = Listenable.merge(<Listenable>[]);
+    Listenable listenableUnderTest = Listenable.merge(<Listenable>[]);
     expect(listenableUnderTest.toString(), 'Listenable.merge([])');
 
     listenableUnderTest = Listenable.merge(<Listenable>[null]);
@@ -254,6 +256,26 @@ void main() {
     );
   });
 
+  test('Listenable.merge does not leak', () {
+    // Regression test for https://github.com/flutter/flutter/issues/25163.
+
+    final TestNotifier source1 = TestNotifier();
+    final TestNotifier source2 = TestNotifier();
+    final VoidCallback fakeListener = () {};
+
+    final Listenable listenableUnderTest = Listenable.merge(<Listenable>[source1, source2]);
+    expect(source1.isListenedTo, isFalse);
+    expect(source2.isListenedTo, isFalse);
+    listenableUnderTest.addListener(fakeListener);
+    expect(source1.isListenedTo, isTrue);
+    expect(source2.isListenedTo, isTrue);
+
+    listenableUnderTest.removeListener(fakeListener);
+    expect(source1.isListenedTo, isFalse);
+    expect(source2.isListenedTo, isFalse);
+  });
+
+
   test('hasListeners', () {
     final HasListenersTester<bool> notifier = HasListenersTester<bool>(true);
     expect(notifier.testHasListeners, isFalse);