From 07a9cc758a019913ecf98260d9783debf28b8b6d Mon Sep 17 00:00:00 2001
From: Adam Barth <abarth@chromium.org>
Date: Sun, 10 Jan 2016 23:12:37 -0800
Subject: [PATCH] DoubleTap gesture asserts when rejected

The pointer router was using an iteration pattern that always delivers
handleEvent calls even if you remove a route during the iteration.
That's awkward to program against and causes trouble for the double-tap
gesture.

This patch switches PointerRouter to using a re-entrant iteration
pattern that supports removing routes (but not adding routes) during the
iteration.
---
 .../flutter/lib/src/gestures/pointer_router.dart  | 12 ++++++++----
 .../test/gestures/pointer_router_test.dart        | 15 +++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/packages/flutter/lib/src/gestures/pointer_router.dart b/packages/flutter/lib/src/gestures/pointer_router.dart
index 32fff2e708..5985187f38 100644
--- a/packages/flutter/lib/src/gestures/pointer_router.dart
+++ b/packages/flutter/lib/src/gestures/pointer_router.dart
@@ -2,6 +2,8 @@
 // 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:flutter/services.dart';
 
 import 'events.dart';
@@ -13,14 +15,14 @@ typedef void PointerExceptionHandler(PointerRouter source, PointerEvent event, P
 
 /// A routing table for [PointerEvent] events.
 class PointerRouter {
-  final Map<int, List<PointerRoute>> _routeMap = new Map<int, List<PointerRoute>>();
+  final Map<int, LinkedHashSet<PointerRoute>> _routeMap = new Map<int, LinkedHashSet<PointerRoute>>();
 
   /// Adds a route to the routing table.
   ///
   /// Whenever this object routes a [PointerEvent] corresponding to
   /// pointer, call route.
   void addRoute(int pointer, PointerRoute route) {
-    List<PointerRoute> routes = _routeMap.putIfAbsent(pointer, () => new List<PointerRoute>());
+    LinkedHashSet<PointerRoute> routes = _routeMap.putIfAbsent(pointer, () => new LinkedHashSet<PointerRoute>());
     assert(!routes.contains(route));
     routes.add(route);
   }
@@ -31,7 +33,7 @@ class PointerRouter {
   /// pointer. Requires that this route was previously added to the router.
   void removeRoute(int pointer, PointerRoute route) {
     assert(_routeMap.containsKey(pointer));
-    List<PointerRoute> routes = _routeMap[pointer];
+    LinkedHashSet<PointerRoute> routes = _routeMap[pointer];
     assert(routes.contains(route));
     routes.remove(route);
     if (routes.isEmpty)
@@ -53,10 +55,12 @@ class PointerRouter {
   /// Routes are called in the order in which they were added to the
   /// PointerRouter object.
   void route(PointerEvent event) {
-    List<PointerRoute> routes = _routeMap[event.pointer];
+    LinkedHashSet<PointerRoute> routes = _routeMap[event.pointer];
     if (routes == null)
       return;
     for (PointerRoute route in new List<PointerRoute>.from(routes)) {
+      if (!routes.contains(route))
+        continue;
       try {
         route(event);
       } catch (exception, stack) {
diff --git a/packages/flutter/test/gestures/pointer_router_test.dart b/packages/flutter/test/gestures/pointer_router_test.dart
index 5328908db1..c5c0582bfe 100644
--- a/packages/flutter/test/gestures/pointer_router_test.dart
+++ b/packages/flutter/test/gestures/pointer_router_test.dart
@@ -27,4 +27,19 @@ void main() {
     router.route(pointer3.up());
     expect(callbackRan, isFalse);
   });
+
+  test('Supports re-entrant cancellation', () {
+    bool callbackRan = false;
+    void callback(PointerEvent event) {
+      callbackRan = true;
+    }
+    PointerRouter router = new PointerRouter();
+    router.addRoute(2, (PointerEvent event) {
+      router.removeRoute(2, callback);
+    });
+    router.addRoute(2, callback);
+    TestPointer pointer2 = new TestPointer(2);
+    router.route(pointer2.down(Point.origin));
+    expect(callbackRan, isFalse);
+  });
 }