Reland "Smooth out iOS irregular input events delivery (#12280)" (#12385)

This reverts commit c2879ca.

Additionally, we fix flutter/flutter#40863 by adding a secondary VSYNC callback.

Unit tests are updated to provide VSYNC mocking and check the fix of flutter/flutter#40863.

The root cause of having flutter/flutter#40863 is the false assumption that each input event must trigger a new frame. That was true in the framework PR flutter/flutter#36616 because the input events there are all scrolling move events. When the PR was ported to the engine, we can no longer distinguish different types of events, and tap events may no longer trigger a new frame.

Therefore, this PR directly hooks into the `VsyncWaiter` and uses its (newly added) secondary callback to dispatch the pending input event.

Author: liyuqian

ci/licenses_golden/licenses_flutter

@@ -487,6 +487,7 @@ FILE: ../../../flutter/shell/common/canvas_spy_unittests.cc
 FILE: ../../../flutter/shell/common/engine.cc
 FILE: ../../../flutter/shell/common/engine.h
 FILE: ../../../flutter/shell/common/fixtures/shell_test.dart
+FILE: ../../../flutter/shell/common/input_events_unittests.cc
 FILE: ../../../flutter/shell/common/isolate_configuration.cc
 FILE: ../../../flutter/shell/common/isolate_configuration.h
 FILE: ../../../flutter/shell/common/persistent_cache.cc
@@ -496,6 +497,8 @@ FILE: ../../../flutter/shell/common/pipeline.h
 FILE: ../../../flutter/shell/common/pipeline_unittests.cc
 FILE: ../../../flutter/shell/common/platform_view.cc
 FILE: ../../../flutter/shell/common/platform_view.h
+FILE: ../../../flutter/shell/common/pointer_data_dispatcher.cc
+FILE: ../../../flutter/shell/common/pointer_data_dispatcher.h
 FILE: ../../../flutter/shell/common/rasterizer.cc
 FILE: ../../../flutter/shell/common/rasterizer.h
 FILE: ../../../flutter/shell/common/run_configuration.cc

shell/common/BUILD.gn

@@ -74,6 +74,8 @@ source_set("common") {
     "pipeline.h",
     "platform_view.cc",
     "platform_view.h",
+    "pointer_data_dispatcher.cc",
+    "pointer_data_dispatcher.h",
     "rasterizer.cc",
     "rasterizer.h",
     "run_configuration.cc",
@@ -156,6 +158,7 @@ if (current_toolchain == host_toolchain) {
   shell_host_executable("shell_unittests") {
     sources = [
       "canvas_spy_unittests.cc",
+      "input_events_unittests.cc",
       "pipeline_unittests.cc",
       "shell_test.cc",
       "shell_test.h",

shell/common/animator.cc

@@ -244,4 +244,8 @@ void Animator::AwaitVSync() {
   delegate_.OnAnimatorNotifyIdle(dart_frame_deadline_);
 }
 
+void Animator::ScheduleSecondaryVsyncCallback(fml::closure callback) {
+  waiter_->ScheduleSecondaryCallback(std::move(callback));
+}
+
 }  // namespace flutter

shell/common/animator.h

@@ -52,6 +52,23 @@ class Animator final {
 
   void Render(std::unique_ptr<flutter::LayerTree> layer_tree);
 
+  //--------------------------------------------------------------------------
+  /// @brief    Schedule a secondary callback to be executed right after the
+  ///           main `VsyncWaiter::AsyncWaitForVsync` callback (which is added
+  ///           by `Animator::RequestFrame`).
+  ///
+  ///           Like the callback in `AsyncWaitForVsync`, this callback is
+  ///           only scheduled to be called once, and it's supposed to be
+  ///           called in the UI thread. If there is no AsyncWaitForVsync
+  ///           callback (`Animator::RequestFrame` is not called), this
+  ///           secondary callback will still be executed at vsync.
+  ///
+  ///           This callback is used to provide the vsync signal needed by
+  ///           `SmoothPointerDataDispatcher`.
+  ///
+  /// @see      `PointerDataDispatcher::ScheduleSecondaryVsyncCallback`.
+  void ScheduleSecondaryVsyncCallback(fml::closure callback);
+
   void Start();
 
   void Stop();

shell/common/engine.cc

@@ -36,6 +36,7 @@ static constexpr char kSettingsChannel[] = "flutter/settings";
 static constexpr char kIsolateChannel[] = "flutter/isolate";
 
 Engine::Engine(Delegate& delegate,
+               const PointerDataDispatcherMaker& dispatcher_maker,
                DartVM& vm,
                fml::RefPtr<const DartSnapshot> isolate_snapshot,
                fml::RefPtr<const DartSnapshot> shared_snapshot,
@@ -51,6 +52,7 @@ Engine::Engine(Delegate& delegate,
       image_decoder_(task_runners,
                      vm.GetConcurrentWorkerTaskRunner(),
                      io_manager),
+      task_runners_(std::move(task_runners)),
       weak_factory_(this) {
   // Runtime controller is initialized here because it takes a reference to this
   // object as its delegate. The delegate may be called in the constructor and
@@ -60,7 +62,7 @@ Engine::Engine(Delegate& delegate,
       &vm,                                   // VM
       std::move(isolate_snapshot),           // isolate snapshot
       std::move(shared_snapshot),            // shared snapshot
-      std::move(task_runners),               // task runners
+      task_runners_,                         // task runners
       std::move(io_manager),                 // io manager
       image_decoder_.GetWeakPtr(),           // image decoder
       settings_.advisory_script_uri,         // advisory script uri
@@ -69,6 +71,8 @@ Engine::Engine(Delegate& delegate,
       settings_.isolate_create_callback,     // isolate create callback
       settings_.isolate_shutdown_callback    // isolate shutdown callback
   );
+
+  pointer_data_dispatcher_ = dispatcher_maker(*this);
 }
 
 Engine::~Engine() = default;
@@ -381,12 +385,12 @@ void Engine::HandleSettingsPlatformMessage(PlatformMessage* message) {
   }
 }
 
-void Engine::DispatchPointerDataPacket(const PointerDataPacket& packet,
-                                       uint64_t trace_flow_id) {
+void Engine::DispatchPointerDataPacket(
+    std::unique_ptr<PointerDataPacket> packet,
+    uint64_t trace_flow_id) {
   TRACE_EVENT0("flutter", "Engine::DispatchPointerDataPacket");
   TRACE_FLOW_STEP("flutter", "PointerEvent", trace_flow_id);
-  animator_->EnqueueTraceFlowId(trace_flow_id);
-  runtime_controller_->DispatchPointerDataPacket(packet);
+  pointer_data_dispatcher_->DispatchPacket(std::move(packet), trace_flow_id);
 }
 
 void Engine::DispatchSemanticsAction(int id,
@@ -462,6 +466,18 @@ FontCollection& Engine::GetFontCollection() {
   return font_collection_;
 }
 
+void Engine::DoDispatchPacket(std::unique_ptr<PointerDataPacket> packet,
+                              uint64_t trace_flow_id) {
+  animator_->EnqueueTraceFlowId(trace_flow_id);
+  if (runtime_controller_) {
+    runtime_controller_->DispatchPointerDataPacket(*packet);
+  }
+}
+
+void Engine::ScheduleSecondaryVsyncCallback(fml::closure callback) {
+  animator_->ScheduleSecondaryVsyncCallback(std::move(callback));
+}
+
 void Engine::HandleAssetPlatformMessage(fml::RefPtr<PlatformMessage> message) {
   fml::RefPtr<PlatformMessageResponse> response = message->response();
   if (!response) {

shell/common/engine.h

@@ -22,6 +22,8 @@
 #include "flutter/runtime/runtime_controller.h"
 #include "flutter/runtime/runtime_delegate.h"
 #include "flutter/shell/common/animator.h"
+#include "flutter/shell/common/platform_view.h"
+#include "flutter/shell/common/pointer_data_dispatcher.h"
 #include "flutter/shell/common/rasterizer.h"
 #include "flutter/shell/common/run_configuration.h"
 #include "flutter/shell/common/shell_io_manager.h"
@@ -65,7 +67,7 @@ namespace flutter {
 ///           name and it does happen to be one of the older classes in the
 ///           repository.
 ///
-class Engine final : public RuntimeDelegate {
+class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
  public:
   //----------------------------------------------------------------------------
   /// @brief      Indicates the result of the call to `Engine::Run`.
@@ -234,6 +236,12 @@ class Engine final : public RuntimeDelegate {
   ///                                tasks that require access to components
   ///                                that cannot be safely accessed by the
   ///                                engine. This is the shell.
+  /// @param      dispatcher_maker   The callback provided by `PlatformView` for
+  ///                                engine to create the pointer data
+  ///                                dispatcher. Similar to other engine
+  ///                                resources, this dispatcher_maker and its
+  ///                                returned dispatcher is only safe to be
+  ///                                called from the UI thread.
   /// @param      vm                 An instance of the running Dart VM.
   /// @param[in]  isolate_snapshot   The snapshot used to create the root
   ///                                isolate. Even though the isolate is not
@@ -265,6 +273,7 @@ class Engine final : public RuntimeDelegate {
   ///                                GPU.
   ///
   Engine(Delegate& delegate,
+         const PointerDataDispatcherMaker& dispatcher_maker,
          DartVM& vm,
          fml::RefPtr<const DartSnapshot> isolate_snapshot,
          fml::RefPtr<const DartSnapshot> shared_snapshot,
@@ -649,7 +658,7 @@ class Engine final : public RuntimeDelegate {
   ///                            timeline and allow grouping frames and input
   ///                            events into logical chunks.
   ///
-  void DispatchPointerDataPacket(const PointerDataPacket& packet,
+  void DispatchPointerDataPacket(std::unique_ptr<PointerDataPacket> packet,
                                  uint64_t trace_flow_id);
 
   //----------------------------------------------------------------------------
@@ -700,18 +709,32 @@ class Engine final : public RuntimeDelegate {
   // |RuntimeDelegate|
   FontCollection& GetFontCollection() override;
 
+  // |PointerDataDispatcher::Delegate|
+  void DoDispatchPacket(std::unique_ptr<PointerDataPacket> packet,
+                        uint64_t trace_flow_id) override;
+
+  // |PointerDataDispatcher::Delegate|
+  void ScheduleSecondaryVsyncCallback(fml::closure callback) override;
+
  private:
   Engine::Delegate& delegate_;
   const Settings settings_;
   std::unique_ptr<Animator> animator_;
   std::unique_ptr<RuntimeController> runtime_controller_;
+
+  // The pointer_data_dispatcher_ depends on animator_ and runtime_controller_.
+  // So it should be defined after them to ensure that pointer_data_dispatcher_
+  // is destructed first.
+  std::unique_ptr<PointerDataDispatcher> pointer_data_dispatcher_;
+
   std::string initial_route_;
   ViewportMetrics viewport_metrics_;
   std::shared_ptr<AssetManager> asset_manager_;
   bool activity_running_;
   bool have_surface_;
   FontCollection font_collection_;
   ImageDecoder image_decoder_;
+  TaskRunners task_runners_;
   fml::WeakPtrFactory<Engine> weak_factory_;
 
   // |RuntimeDelegate|

shell/common/fixtures/shell_test.dart

@@ -11,6 +11,7 @@ void main() {}
 
 void nativeReportTimingsCallback(List<int> timings) native 'NativeReportTimingsCallback';
 void nativeOnBeginFrame(int microseconds) native 'NativeOnBeginFrame';
+void nativeOnPointerDataPacket() native 'NativeOnPointerDataPacket';
 
 @pragma('vm:entry-point')
 void reportTimingsMain() {
@@ -32,6 +33,13 @@ void onBeginFrameMain() {
   };
 }
 
+@pragma('vm:entry-point')
+void onPointerDataPacketMain() {
+  window.onPointerDataPacket = (PointerDataPacket packet) {
+    nativeOnPointerDataPacket();
+  };
+}
+
 @pragma('vm:entry-point')
 void emptyMain() {}