Make AndroidView take gesture recognizer factories. (#21657)

Before this PR AndroidView's gestureRecognizers field was a list of
gesture recognizers. This was problematic as when the widget was rebuilt
with the same gesture recognizer instances we would try to re-join the
recognizers to a gesture arena team and crash (as a OneSeqeunceGestureRecognizer
team can only be set once).

With this change, we instead take a set of factories.
This allows AndroidView to create the gesture recognizers just before
adding them to the team, and thus be sure that they are only added once to a
team.

The factories are identified by the type of the object they create, this
allows AndroidView to know when it is given an equivalent set of gesture
recognizer factories, and do nothing in that case.

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

@@ -280,3 +280,23 @@ class _LazyListIterator<E> implements Iterator<E> {
     return true;
   }
 }
+
+/// A factory interface that also reports the type of the created objects.
+class Factory<T> {
+  /// Creates a new factory.
+  ///
+  /// The `constructor` parameter must not be null.
+  const Factory(this.constructor) : assert(constructor != null);
+
+  /// Creates a new object of type T.
+  final ValueGetter<T> constructor;
+
+  /// The type of the objects created by this factory.
+  Type get type => T;
+
+  @override
+  String toString() {
+    return 'Factory(type: $type)';
+  }
+}
+

packages/flutter/lib/src/rendering/platform_view.dart

@@ -48,7 +48,7 @@ enum _PlatformViewState {
 ///
 /// RenderAndroidView participates in Flutter's [GestureArena]s, and dispatches touch events to the
 /// Android view iff it won the arena. Specific gestures that should be dispatched to the Android
-/// view can be specified in [RenderAndroidView.gestureRecognizers]. If
+/// view can be specified with factories in [RenderAndroidView.gestureRecognizers]. If
 /// [RenderAndroidView.gestureRecognizers] is empty, the gesture will be dispatched to the Android
 /// view iff it was not claimed by any other gesture recognizer.
 ///
@@ -61,14 +61,14 @@ class RenderAndroidView extends RenderBox {
   RenderAndroidView({
     @required AndroidViewController viewController,
     @required this.hitTestBehavior,
-    List<OneSequenceGestureRecognizer> gestureRecognizers = const <OneSequenceGestureRecognizer> [],
+    @required Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizers,
   }) : assert(viewController != null),
        assert(hitTestBehavior != null),
        assert(gestureRecognizers != null),
        _viewController = viewController
   {
     _motionEventsDispatcher = _MotionEventsDispatcher(globalToLocal, viewController);
-    this.gestureRecognizers = gestureRecognizers;
+    updateGestureRecognizers(gestureRecognizers);
   }
 
   _PlatformViewState _state = _PlatformViewState.uninitialized;
@@ -94,17 +94,40 @@ class RenderAndroidView extends RenderBox {
 
   /// Which gestures should be forwarded to the Android view.
   ///
-  /// The gesture recognizers on this list participate in the gesture arena for each pointer
-  /// that was put down on the render box. If any of the recognizers on this list wins the
+  /// Gesture recognizers created by factories in this set participate in the gesture arena for each
+  /// pointer that was put down on the render box. If any of the recognizers on this list wins the
   /// gesture arena, the entire pointer event sequence starting from the pointer down event
   /// will be dispatched to the Android view.
-  set gestureRecognizers(List<OneSequenceGestureRecognizer> recognizers) {
-    assert(recognizers != null);
-    if (recognizers == _gestureRecognizer?.gestureRecognizers) {
+  ///
+  /// The `gestureRecognizers` property must not contain more than one factory with the same [Factory.type].
+  ///
+  /// Setting a new set of gesture recognizer factories with the same [Factory.type]s as the current
+  /// set has no effect, because the factories' constructors would have already been called with the previous set.
+  ///
+  /// Any active gesture arena the Android view participates in is rejected when the
+  /// set of gesture recognizers is changed.
+  void updateGestureRecognizers(Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizers) {
+    assert(gestureRecognizers != null);
+    assert(_factoriesTypeSet(gestureRecognizers).length == gestureRecognizers.length);
+    if (_factoryTypesSetEquals(gestureRecognizers, _gestureRecognizer?.gestureRecognizerFactories)) {
       return;
     }
     _gestureRecognizer?.dispose();
-    _gestureRecognizer = _AndroidViewGestureRecognizer(_motionEventsDispatcher, recognizers);
+    _gestureRecognizer = _AndroidViewGestureRecognizer(_motionEventsDispatcher, gestureRecognizers);
+  }
+
+  static bool _factoryTypesSetEquals<T>(Set<Factory<T>> a, Set<Factory<T>> b) {
+    if (a == b) {
+      return true;
+    }
+    if (a == null ||  b == null) {
+      return false;
+    }
+    return setEquals(_factoriesTypeSet(a), _factoriesTypeSet(b));
+  }
+
+  static Set<Type> _factoriesTypeSet<T>(Set<Factory<T>> factories) {
+    return factories.map<Type>((Factory<T> factory) => factory.type).toSet();
   }
 
   @override
@@ -211,8 +234,14 @@ class RenderAndroidView extends RenderBox {
 }
 
 class _AndroidViewGestureRecognizer extends OneSequenceGestureRecognizer {
-  _AndroidViewGestureRecognizer(this.dispatcher, List<OneSequenceGestureRecognizer> gestureRecognizers) {
-    this.gestureRecognizers = gestureRecognizers;
+  _AndroidViewGestureRecognizer(this.dispatcher, this.gestureRecognizerFactories) {
+    team = GestureArenaTeam();
+    team.captain = this;
+    _gestureRecognizers = gestureRecognizerFactories.map(
+      (Factory<OneSequenceGestureRecognizer> factory) {
+        return factory.constructor()..team = team;
+      }
+    ).toSet();
   }
 
   final _MotionEventsDispatcher dispatcher;
@@ -230,16 +259,8 @@ class _AndroidViewGestureRecognizer extends OneSequenceGestureRecognizer {
   // We use OneSequenceGestureRecognizers as they support gesture arena teams.
   // TODO(amirh): get a list of GestureRecognizers here.
   // https://github.com/flutter/flutter/issues/20953
-  List<OneSequenceGestureRecognizer> _gestureRecognizers;
-  List<OneSequenceGestureRecognizer> get gestureRecognizers => _gestureRecognizers;
-  set gestureRecognizers(List<OneSequenceGestureRecognizer> recognizers) {
-    _gestureRecognizers = recognizers;
-    team = GestureArenaTeam();
-    team.captain = this;
-    for (OneSequenceGestureRecognizer recognizer in _gestureRecognizers) {
-      recognizer.team = team;
-    }
-  }
+  final Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizerFactories;
+  Set<OneSequenceGestureRecognizer> _gestureRecognizers;
 
   @override
   void addPointer(PointerDownEvent event) {

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

@@ -50,7 +50,7 @@ import 'framework.dart';
 class AndroidView extends StatefulWidget {
   /// Creates a widget that embeds an Android view.
   ///
-  /// The `viewType`, `hitTestBehavior`, and `gestureRecognizers` parameters must not be null.
+  /// The `viewType` and `hitTestBehavior` parameters must not be null.
   /// If `creationParams` is not null then `creationParamsCodec` must not be null.
   AndroidView({ // ignore: prefer_const_constructors_in_immutables
                 // TODO(aam): Remove lint ignore above once dartbug.com/34297 is fixed
@@ -59,12 +59,11 @@ class AndroidView extends StatefulWidget {
     this.onPlatformViewCreated,
     this.hitTestBehavior = PlatformViewHitTestBehavior.opaque,
     this.layoutDirection,
-    this.gestureRecognizers = const <OneSequenceGestureRecognizer> [],
+    this.gestureRecognizers,
     this.creationParams,
-    this.creationParamsCodec
+    this.creationParamsCodec,
   }) : assert(viewType != null),
        assert(hitTestBehavior != null),
-        assert(gestureRecognizers != null),
        assert(creationParams == null || creationParamsCodec != null),
        super(key: key);
 
@@ -92,11 +91,14 @@ class AndroidView extends StatefulWidget {
 
   /// Which gestures should be forwarded to the Android view.
   ///
-  /// The gesture recognizers on this list participate in the gesture arena for each pointer
-  /// that was put down on the widget. If any of the recognizers on this list wins the
+  /// The gesture recognizers built by factories in this set participate in the gesture arena for
+  /// each pointer that was put down on the widget. If any of these recognizers win the
   /// gesture arena, the entire pointer event sequence starting from the pointer down event
   /// will be dispatched to the Android view.
   ///
+  /// When null, an empty set of gesture recognizer factories is used, in which case a pointer event sequence
+  /// will only be dispatched to the Android view if no other member of the arena claimed it.
+  ///
   /// For example, with the following setup vertical drags will not be dispatched to the Android
   /// view as the vertical drag gesture is claimed by the parent [GestureDetector].
   /// ```dart
@@ -104,12 +106,11 @@ class AndroidView extends StatefulWidget {
   ///   onVerticalDragStart: (DragStartDetails d) {},
   ///   child: AndroidView(
   ///     viewType: 'webview',
-  ///     gestureRecognizers: <OneSequenceGestureRecognizer>[],
   ///   ),
   /// )
   /// ```
   /// To get the [AndroidView] to claim the vertical drag gestures we can pass a vertical drag
-  /// gesture recognizer in [gestureRecognizers] e.g:
+  /// gesture recognizer factory in [gestureRecognizers] e.g:
   /// ```dart
   /// GestureDetector(
   ///   onVerticalDragStart: (DragStartDetails d) {},
@@ -118,19 +119,29 @@ class AndroidView extends StatefulWidget {
   ///     height: 100.0,
   ///     child: AndroidView(
   ///       viewType: 'webview',
-  ///       gestureRecognizers: <OneSequenceGestureRecognizer>[ new VerticalDragGestureRecognizer() ],
+  ///       gestureRecognizers: <Factory<OneSequenceGestureRecognizer>>[
+  ///         new Factory<OneSequenceGestureRecognizer>(
+  ///           () => new EagerGestureRecognizer(),
+  ///         ),
+  ///       ].toSet(),
   ///     ),
   ///   ),
   /// )
   /// ```
   ///
   /// An [AndroidView] can be configured to consume all pointers that were put down in its bounds
-  /// by passing an [EagerGestureRecognizer] in [gestureRecognizers]. [EagerGestureRecognizer] is a
-  /// special gesture recognizer that immediately claims the gesture after a pointer down event.
+  /// by passing a factory for an [EagerGestureRecognizer] in [gestureRecognizers].
+  /// [EagerGestureRecognizer] is a special gesture recognizer that immediately claims the gesture
+  /// after a pointer down event.
+  ///
+  /// The `gestureRecognizers` property must not contain more than one factory with the same [Factory.type].
+  ///
+  /// Changing `gestureRecognizers` results in rejection of any active gesture arenas (if the
+  /// Android view is actively participating in an arena).
   // We use OneSequenceGestureRecognizers as they support gesture arena teams.
   // TODO(amirh): get a list of GestureRecognizers here.
   // https://github.com/flutter/flutter/issues/20953
-  final List<OneSequenceGestureRecognizer> gestureRecognizers;
+  final Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizers;
 
   /// Passed as the args argument of [PlatformViewFactory#create](/javadoc/io/flutter/plugin/platform/PlatformViewFactory.html#create-android.content.Context-int-java.lang.Object-)
   ///
@@ -155,12 +166,15 @@ class _AndroidViewState extends State<AndroidView> {
   TextDirection _layoutDirection;
   bool _initialized = false;
 
+  static final Set<Factory<OneSequenceGestureRecognizer>> _emptyRecognizersSet =
+    Set<Factory<OneSequenceGestureRecognizer>>();
+
   @override
   Widget build(BuildContext context) {
     return _AndroidPlatformView(
         controller: _controller,
         hitTestBehavior: widget.hitTestBehavior,
-        gestureRecognizers: widget.gestureRecognizers,
+        gestureRecognizers: widget.gestureRecognizers ?? _emptyRecognizersSet,
     );
   }
 
@@ -245,7 +259,7 @@ class _AndroidPlatformView extends LeafRenderObjectWidget {
 
   final AndroidViewController controller;
   final PlatformViewHitTestBehavior hitTestBehavior;
-  final List<OneSequenceGestureRecognizer> gestureRecognizers;
+  final Set<Factory<OneSequenceGestureRecognizer>> gestureRecognizers;
 
   @override
   RenderObject createRenderObject(BuildContext context) =>
@@ -259,6 +273,6 @@ class _AndroidPlatformView extends LeafRenderObjectWidget {
   void updateRenderObject(BuildContext context, RenderAndroidView renderObject) {
     renderObject.viewController = controller;
     renderObject.hitTestBehavior = hitTestBehavior;
-    renderObject.gestureRecognizers = gestureRecognizers;
+    renderObject.updateGestureRecognizers(gestureRecognizers);
   }
 }

packages/flutter/test/widgets/platform_view_test.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 import 'dart:typed_data';
 
+import 'package:flutter/foundation.dart';
 import 'package:flutter/gestures.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter/services.dart';
@@ -260,9 +261,9 @@ void main() {
 
     expect(
       viewsController.motionEvents[currentViewId + 1],
-      orderedEquals(<FakeMotionEvent> [
-        const FakeMotionEvent(AndroidViewController.kActionDown, <int> [0], <Offset> [Offset(50.0, 50.0)]),
-        const FakeMotionEvent(AndroidViewController.kActionUp, <int> [0], <Offset> [Offset(50.0, 50.0)]),
+      orderedEquals(<FakeMotionEvent>[
+        const FakeMotionEvent(AndroidViewController.kActionDown, <int>[0], <Offset>[Offset(50.0, 50.0)]),
+        const FakeMotionEvent(AndroidViewController.kActionUp, <int>[0], <Offset>[Offset(50.0, 50.0)]),
       ]),
     );
   });
@@ -277,7 +278,7 @@ void main() {
       Directionality(
         textDirection: TextDirection.ltr,
         child: Stack(
-          children: <Widget> [
+          children: <Widget>[
             Listener(
               behavior: HitTestBehavior.opaque,
               onPointerDown: (PointerDownEvent e) { numPointerDownsOnParent++; },
@@ -320,7 +321,7 @@ void main() {
       Directionality(
         textDirection: TextDirection.ltr,
         child: Stack(
-          children: <Widget> [
+          children: <Widget>[
             Listener(
               behavior: HitTestBehavior.opaque,
               onPointerDown: (PointerDownEvent e) { numPointerDownsOnParent++; },
@@ -345,8 +346,8 @@ void main() {
 
     expect(
       viewsController.motionEvents[currentViewId + 1],
-      orderedEquals(<FakeMotionEvent> [
-        const FakeMotionEvent(AndroidViewController.kActionDown, <int> [0], <Offset> [Offset(50.0, 50.0)]),
+      orderedEquals(<FakeMotionEvent>[
+        const FakeMotionEvent(AndroidViewController.kActionDown, <int>[0], <Offset>[Offset(50.0, 50.0)]),
       ]),
     );
     expect(
@@ -365,7 +366,7 @@ void main() {
       Directionality(
         textDirection: TextDirection.ltr,
         child: Stack(
-          children: <Widget> [
+          children: <Widget>[
             Listener(
               behavior: HitTestBehavior.opaque,
               onPointerDown: (PointerDownEvent e) { numPointerDownsOnParent++; },
@@ -390,8 +391,8 @@ void main() {
 
     expect(
       viewsController.motionEvents[currentViewId + 1],
-      orderedEquals(<FakeMotionEvent> [
-        const FakeMotionEvent(AndroidViewController.kActionDown, <int> [0], <Offset> [Offset(50.0, 50.0)]),
+      orderedEquals(<FakeMotionEvent>[
+        const FakeMotionEvent(AndroidViewController.kActionDown, <int>[0], <Offset>[Offset(50.0, 50.0)]),
       ]),
     );
     expect(
@@ -423,9 +424,9 @@ void main() {
 
     expect(
       viewsController.motionEvents[currentViewId + 1],
-      orderedEquals(<FakeMotionEvent> [
-        const FakeMotionEvent(AndroidViewController.kActionDown, <int> [0], <Offset> [Offset(40.0, 40.0)]),
-        const FakeMotionEvent(AndroidViewController.kActionUp, <int> [0], <Offset> [Offset(40.0, 40.0)]),
+      orderedEquals(<FakeMotionEvent>[
+        const FakeMotionEvent(AndroidViewController.kActionDown, <int>[0], <Offset>[Offset(40.0, 40.0)]),
+        const FakeMotionEvent(AndroidViewController.kActionUp, <int>[0], <Offset>[Offset(40.0, 40.0)]),
       ]),
     );
   });
@@ -562,7 +563,11 @@ void main() {
             height: 100.0,
             child: AndroidView(
               viewType: 'webview',
-              gestureRecognizers: <OneSequenceGestureRecognizer> [VerticalDragGestureRecognizer()],
+              gestureRecognizers: <Factory<OneSequenceGestureRecognizer>>[
+                Factory<VerticalDragGestureRecognizer>(
+                  () => VerticalDragGestureRecognizer(),
+                ),
+              ].toSet(),
               layoutDirection: TextDirection.ltr,
             ),
           ),
@@ -577,10 +582,10 @@ void main() {
     expect(verticalDragAcceptedByParent, false);
     expect(
       viewsController.motionEvents[currentViewId + 1],
-      orderedEquals(<FakeMotionEvent> [
-        const FakeMotionEvent(AndroidViewController.kActionDown, <int> [0], <Offset> [Offset(50.0, 50.0)]),
-        const FakeMotionEvent(AndroidViewController.kActionMove, <int> [0], <Offset> [Offset(50.0, 150.0)]),
-        const FakeMotionEvent(AndroidViewController.kActionUp, <int> [0], <Offset> [Offset(50.0, 150.0)]),
+      orderedEquals(<FakeMotionEvent>[
+        const FakeMotionEvent(AndroidViewController.kActionDown, <int>[0], <Offset>[Offset(50.0, 50.0)]),
+        const FakeMotionEvent(AndroidViewController.kActionMove, <int>[0], <Offset>[Offset(50.0, 150.0)]),
+        const FakeMotionEvent(AndroidViewController.kActionUp, <int>[0], <Offset>[Offset(50.0, 150.0)]),
       ]),
     );
   });
@@ -616,9 +621,9 @@ void main() {
     expect(verticalDragAcceptedByParent, false);
     expect(
       viewsController.motionEvents[currentViewId + 1],
-      orderedEquals(<FakeMotionEvent> [
-        const FakeMotionEvent(AndroidViewController.kActionDown, <int> [0], <Offset> [Offset(50.0, 50.0)]),
-        const FakeMotionEvent(AndroidViewController.kActionUp, <int> [0], <Offset> [Offset(50.0, 50.0)]),
+      orderedEquals(<FakeMotionEvent>[
+        const FakeMotionEvent(AndroidViewController.kActionDown, <int>[0], <Offset>[Offset(50.0, 50.0)]),
+        const FakeMotionEvent(AndroidViewController.kActionUp, <int>[0], <Offset>[Offset(50.0, 50.0)]),
       ]),
     );
   });
@@ -662,10 +667,10 @@ void main() {
 
     expect(
       viewsController.motionEvents[currentViewId + 1],
-      orderedEquals(<FakeMotionEvent> [
-        const FakeMotionEvent(AndroidViewController.kActionDown, <int> [0], <Offset> [Offset(50.0, 50.0)]),
-        const FakeMotionEvent(AndroidViewController.kActionMove, <int> [0], <Offset> [Offset(50.0, 150.0)]),
-        const FakeMotionEvent(AndroidViewController.kActionUp, <int> [0], <Offset> [Offset(50.0, 150.0)]),
+      orderedEquals(<FakeMotionEvent>[
+        const FakeMotionEvent(AndroidViewController.kActionDown, <int>[0], <Offset>[Offset(50.0, 50.0)]),
+        const FakeMotionEvent(AndroidViewController.kActionMove, <int>[0], <Offset>[Offset(50.0, 150.0)]),
+        const FakeMotionEvent(AndroidViewController.kActionUp, <int>[0], <Offset>[Offset(50.0, 150.0)]),
       ]),
     );
   });
@@ -684,7 +689,11 @@ void main() {
             height: 100.0,
             child: AndroidView(
               viewType: 'webview',
-              gestureRecognizers: <OneSequenceGestureRecognizer>[ EagerGestureRecognizer() ],
+              gestureRecognizers: <Factory<OneSequenceGestureRecognizer>>[
+                Factory<OneSequenceGestureRecognizer>(
+                  () => EagerGestureRecognizer(),
+                ),
+              ].toSet(),
               layoutDirection: TextDirection.ltr,
             ),
           ),
@@ -700,9 +709,62 @@ void main() {
     // pointer down event is immediately dispatched.
     expect(
       viewsController.motionEvents[currentViewId + 1],
-      orderedEquals(<FakeMotionEvent> [
-        const FakeMotionEvent(AndroidViewController.kActionDown, <int> [0], <Offset> [Offset(50.0, 50.0)]),
+      orderedEquals(<FakeMotionEvent>[
+        const FakeMotionEvent(AndroidViewController.kActionDown, <int>[0], <Offset>[Offset(50.0, 50.0)]),
       ]),
     );
   });
+
+  testWidgets('RenderAndroidView reconstructed with same gestureRecognizers', (WidgetTester tester) async {
+    final FakePlatformViewsController viewsController = FakePlatformViewsController(TargetPlatform.android);
+    viewsController.registerViewType('webview');
+
+    final AndroidView androidView = AndroidView(
+      viewType: 'webview',
+      gestureRecognizers: <Factory<OneSequenceGestureRecognizer>>[
+        Factory<EagerGestureRecognizer>(
+          () => EagerGestureRecognizer(),
+        ),
+      ].toSet(),
+      layoutDirection: TextDirection.ltr,
+    );
+
+    await tester.pumpWidget(androidView);
+    await tester.pumpWidget(const SizedBox.shrink());
+    await tester.pumpWidget(androidView);
+  });
+
+  testWidgets('AndroidView rebuilt with same gestureRecognizers', (WidgetTester tester) async {
+    final FakePlatformViewsController viewsController = FakePlatformViewsController(TargetPlatform.android);
+    viewsController.registerViewType('webview');
+
+    int factoryInvocationCount = 0;
+    final ValueGetter<EagerGestureRecognizer> constructRecognizer = () {
+        factoryInvocationCount += 1;
+        return EagerGestureRecognizer();
+      };
+
+    await tester.pumpWidget(
+      AndroidView(
+        viewType: 'webview',
+        gestureRecognizers: <Factory<OneSequenceGestureRecognizer>>[
+          Factory<EagerGestureRecognizer>(constructRecognizer),
+        ].toSet(),
+        layoutDirection: TextDirection.ltr,
+      ),
+    );
+
+    await tester.pumpWidget(
+      AndroidView(
+        viewType: 'webview',
+        hitTestBehavior: PlatformViewHitTestBehavior.translucent,
+        gestureRecognizers: <Factory<OneSequenceGestureRecognizer>>[
+          Factory<EagerGestureRecognizer>(constructRecognizer),
+        ].toSet(),
+        layoutDirection: TextDirection.ltr,
+      ),
+    );
+
+    expect(factoryInvocationCount, 1);
+  });
 }