Prevent event dispatch from happening during reassemble. (#11459)

It was previously possible for event dispatch to occurr during the
brief window where the tree had been marked dirty but before it had
been relaid out by reassemble, which would cause assertions to fire if
someone did a hot reload while touching the device.

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

@@ -149,17 +149,83 @@ abstract class BindingBase {
     assert(() { _debugServiceExtensionsRegistered = true; return true; });
   }
 
-  /// Called when the ext.flutter.reassemble signal is sent by
-  /// development tools.
+  /// Whether [lockEvents] is currently locking events.
   ///
-  /// This is used by development tools when the application code has
-  /// changed, to cause the application to pick up any changed code.
-  /// Bindings are expected to use this method to reregister anything
-  /// that uses closures, so that they do not keep pointing to old
-  /// code, and to flush any caches of previously computed values, in
-  /// case the new code would compute them differently.
+  /// Binding subclasses that fire events should check this first, and if it is
+  /// set, queue events instead of firing them.
+  ///
+  /// Events should be flushed when [unlocked] is called.
+  @protected
+  bool get locked => _lockCount > 0;
+  int _lockCount = 0;
+
+  /// Locks the dispatching of asynchronous events and callbacks until the
+  /// callback's future completes.
+  ///
+  /// This causes input lag and should therefore be avoided when possible. It is
+  /// primarily intended for development features, in particular to allow
+  /// [reassembleApplication] to block input while it walks the tree (which it
+  /// partially does asynchronously).
+  ///
+  /// The [Future] returned by the `callback` argument is returned by [lockEvents].
+  @protected
+  Future<Null> lockEvents(Future<Null> callback()) {
+    assert(callback != null);
+    _lockCount += 1;
+    final Future<Null> future = callback();
+    assert(future != null, 'The lockEvents() callback returned null; it should return a Future<Null> that completes when the lock is to expire.');
+    future.whenComplete(() {
+      _lockCount -= 1;
+      if (!locked)
+        unlocked();
+    });
+    return future;
+  }
+
+  /// Called by [lockEvents] when events get unlocked.
+  ///
+  /// This should flush any events that were queued while [locked] was true.
+  @protected
   @mustCallSuper
+  void unlocked() {
+    assert(!locked);
+  }
+
+  /// Cause the entire application to redraw.
+  ///
+  /// This is used by development tools when the application code has changed,
+  /// to cause the application to pick up any changed code. It can be triggered
+  /// manually by sending the `ext.flutter.reassemble` service extension signal.
+  ///
+  /// This method is very computationally expensive and should not be used in
+  /// production code. There is never a valid reason to cause the entire
+  /// application to repaint in production. All aspects of the Flutter framework
+  /// know how to redraw when necessary. It is only necessary in development
+  /// when the code is literally changed on the fly (e.g. in hot reload) or when
+  /// debug flags are being toggled.
+  ///
+  /// While this method runs, events are locked (e.g. pointer events are not
+  /// dispatched).
+  ///
+  /// Subclasses (binding classes) should override [performReassemble] to react
+  /// to this method being called. This method itself should not be overridden.
   Future<Null> reassembleApplication() {
+    return lockEvents(performReassemble);
+  }
+
+  /// This method is called by [reassembleApplication] to actually cause the
+  /// application to reassemble.
+  ///
+  /// Bindings are expected to use this method to reregister anything that uses
+  /// closures, so that they do not keep pointing to old code, and to flush any
+  /// caches of previously computed values, in case the new code would compute
+  /// them differently. For example, the rendering layer triggers the entire
+  /// application to repaint when this is called.
+  ///
+  /// Do not call this method directly. Instead, use [reassembleApplication].
+  @mustCallSuper
+  @protected
+  Future<Null> performReassemble() {
     FlutterError.resetErrorCount();
     return new Future<Null>.value();
   }

packages/flutter/lib/src/gestures/binding.dart

@@ -28,22 +28,24 @@ abstract class GestureBinding extends BindingBase with HitTestable, HitTestDispa
     ui.window.onPointerDataPacket = _handlePointerDataPacket;
   }
 
+  @override
+  void unlocked() {
+    super.unlocked();
+    _flushPointerEventQueue();
+  }
+
   /// The singleton instance of this object.
   static GestureBinding get instance => _instance;
   static GestureBinding _instance;
 
+  final Queue<PointerEvent> _pendingPointerEvents = new Queue<PointerEvent>();
+
   void _handlePointerDataPacket(ui.PointerDataPacket packet) {
     // We convert pointer data to logical pixels so that e.g. the touch slop can be
     // defined in a device-independent manner.
     _pendingPointerEvents.addAll(PointerEventConverter.expand(packet.data, ui.window.devicePixelRatio));
-    _flushPointerEventQueue();
-  }
-
-  final Queue<PointerEvent> _pendingPointerEvents = new Queue<PointerEvent>();
-
-  void _flushPointerEventQueue() {
-    while (_pendingPointerEvents.isNotEmpty)
-      _handlePointerEvent(_pendingPointerEvents.removeFirst());
+    if (!locked)
+      _flushPointerEventQueue();
   }
 
   /// Dispatch a [PointerCancelEvent] for the given pointer soon.
@@ -51,11 +53,17 @@ abstract class GestureBinding extends BindingBase with HitTestable, HitTestDispa
   /// The pointer event will be dispatch before the next pointer event and
   /// before the end of the microtask but not within this function call.
   void cancelPointer(int pointer) {
-    if (_pendingPointerEvents.isEmpty)
+    if (_pendingPointerEvents.isEmpty && !locked)
       scheduleMicrotask(_flushPointerEventQueue);
     _pendingPointerEvents.addFirst(new PointerCancelEvent(pointer: pointer));
   }
 
+  void _flushPointerEventQueue() {
+    assert(!locked);
+    while (_pendingPointerEvents.isNotEmpty)
+      _handlePointerEvent(_pendingPointerEvents.removeFirst());
+  }
+
   /// A router that routes all pointer events received from the engine.
   final PointerRouter pointerRouter = new PointerRouter();
 
@@ -70,6 +78,7 @@ abstract class GestureBinding extends BindingBase with HitTestable, HitTestDispa
   final Map<int, HitTestResult> _hitTests = <int, HitTestResult>{};
 
   void _handlePointerEvent(PointerEvent event) {
+    assert(!locked);
     HitTestResult result;
     if (event is PointerDownEvent) {
       assert(!_hitTests.containsKey(event.pointer));
@@ -105,6 +114,7 @@ abstract class GestureBinding extends BindingBase with HitTestable, HitTestDispa
   /// the handlers might throw. The `result` argument must not be null.
   @override // from HitTestDispatcher
   void dispatchEvent(PointerEvent event, HitTestResult result) {
+    assert(!locked);
     assert(result != null);
     for (HitTestEntry entry in result.path) {
       try {

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

@@ -276,8 +276,8 @@ abstract class RendererBinding extends BindingBase with SchedulerBinding, Servic
   }
 
   @override
-  Future<Null> reassembleApplication() async {
-    await super.reassembleApplication();
+  Future<Null> performReassemble() async {
+    await super.performReassemble();
     Timeline.startSync('Dirty Render Tree');
     try {
       renderView.reassemble();

packages/flutter/lib/src/scheduler/binding.dart

@@ -207,7 +207,14 @@ abstract class SchedulerBinding extends BindingBase {
   void scheduleTask(VoidCallback task, Priority priority) {
     final bool isFirstTask = _taskQueue.isEmpty;
     _taskQueue.add(new _TaskEntry(task, priority.value));
-    if (isFirstTask)
+    if (isFirstTask && !locked)
+      _ensureEventLoopCallback();
+  }
+
+  @override
+  void unlocked() {
+    super.unlocked();
+    if (_taskQueue.isNotEmpty)
       _ensureEventLoopCallback();
   }
 
@@ -216,6 +223,7 @@ abstract class SchedulerBinding extends BindingBase {
 
   // Ensures that the scheduler is awakened by the event loop.
   void _ensureEventLoopCallback() {
+    assert(!locked);
     if (_hasRequestedAnEventLoopCallback)
       return;
     Timer.run(handleEventLoopCallback);
@@ -230,7 +238,7 @@ abstract class SchedulerBinding extends BindingBase {
 
   // Called when the system wakes up and at the end of each frame.
   void _runTasks() {
-    if (_taskQueue.isEmpty)
+    if (_taskQueue.isEmpty || locked)
       return;
     final _TaskEntry entry = _taskQueue.first;
     // TODO(floitsch): for now we only expose the priority. It might
@@ -285,7 +293,6 @@ abstract class SchedulerBinding extends BindingBase {
   /// [cancelFrameCallbackWithId].
   int scheduleFrameCallback(FrameCallback callback, { bool rescheduling: false }) {
     scheduleFrame();
-
     _nextFrameCallbackId += 1;
     _transientCallbacks[_nextFrameCallbackId] = new _FrameCallbackEntry(callback, rescheduling: rescheduling);
     return _nextFrameCallbackId;

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

@@ -544,12 +544,12 @@ abstract class WidgetsBinding extends BindingBase with GestureBinding, RendererB
   }
 
   @override
-  Future<Null> reassembleApplication() {
+  Future<Null> performReassemble() {
     _needToReportFirstFrame = true;
     preventThisFrameFromBeingReportedAsFirstFrame();
     if (renderViewElement != null)
       buildOwner.reassemble(renderViewElement);
-    return super.reassembleApplication();
+    return super.performReassemble();
   }
 }
 

packages/flutter/test/foundation/reassemble_test.dart

+// Copyright 2017 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 'dart:async';
+
+import 'package:flutter/foundation.dart';
+import 'package:test/test.dart';
+
+class TestFoundationFlutterBinding extends BindingBase {
+  bool wasLocked;
+
+  @override
+  Future<Null> performReassemble() async {
+    wasLocked = locked;
+    return super.performReassemble();
+  }
+}
+
+TestFoundationFlutterBinding binding = new TestFoundationFlutterBinding();
+
+void main() {
+  binding ??= new TestFoundationFlutterBinding();
+
+  test('Pointer events are locked during reassemble', () async {
+    await binding.reassembleApplication();
+    expect(binding.wasLocked, isTrue);
+  });
+}
\ No newline at end of file

packages/flutter/test/foundation/service_extensions_test.dart

@@ -40,9 +40,9 @@ class TestServiceExtensionsBinding extends BindingBase
 
   int reassembled = 0;
   @override
-  Future<Null> reassembleApplication() {
+  Future<Null> performReassemble() {
     reassembled += 1;
-    return super.reassembleApplication();
+    return super.performReassemble();
   }
 
   bool frameScheduled = false;

packages/flutter/test/gestures/locking_test.dart

+// Copyright 2017 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 'dart:async';
+import 'dart:ui' as ui;
+
+import 'package:flutter/foundation.dart';
+import 'package:flutter/gestures.dart';
+import 'package:test/test.dart';
+
+typedef void HandleEventCallback(PointerEvent event);
+
+class TestGestureFlutterBinding extends BindingBase with GestureBinding {
+  HandleEventCallback callback;
+
+  @override
+  void handleEvent(PointerEvent event, HitTestEntry entry) {
+    if (callback != null)
+      callback(event);
+    super.handleEvent(event, entry);
+  }
+
+  static const ui.PointerDataPacket packet = const ui.PointerDataPacket(
+    data: const <ui.PointerData>[
+      const ui.PointerData(change: ui.PointerChange.down),
+      const ui.PointerData(change: ui.PointerChange.up),
+    ]
+  );
+
+  Future<Null> test(VoidCallback callback) {
+    assert(callback != null);
+    return _binding.lockEvents(() async {
+      ui.window.onPointerDataPacket(packet);
+      callback();
+    });
+  }
+}
+
+TestGestureFlutterBinding _binding = new TestGestureFlutterBinding();
+
+void ensureTestGestureBinding() {
+  _binding ??= new TestGestureFlutterBinding();
+  assert(GestureBinding.instance != null);
+}
+
+void main() {
+  setUp(ensureTestGestureBinding);
+
+  test('Pointer events are locked during reassemble', () async {
+    final List<PointerEvent> events = <PointerEvent>[];
+    _binding.callback = events.add;
+    bool tested = false;
+    await _binding.test(() {
+      expect(events.length, 0);
+      tested = true;
+    });
+    expect(tested, isTrue);
+    expect(events.length, 2);
+    expect(events[0].runtimeType, equals(PointerDownEvent));
+    expect(events[1].runtimeType, equals(PointerUpEvent));
+  });
+}
\ No newline at end of file