Make pointer events ignore invalid views (flutter/engine#51925)

With this PR, Flutter will now ignore pointer events for an invalid view, i.e. views that have not been add or have been removed.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

engine/src/flutter/lib/ui/window/pointer_data_packet_converter.cc

@@ -11,7 +11,8 @@
 
 namespace flutter {
 
-PointerDataPacketConverter::PointerDataPacketConverter() {}
+PointerDataPacketConverter::PointerDataPacketConverter(const Delegate& delegate)
+    : delegate_(delegate) {}
 
 PointerDataPacketConverter::~PointerDataPacketConverter() = default;
 
@@ -39,6 +40,10 @@ std::unique_ptr<PointerDataPacket> PointerDataPacketConverter::Convert(
 void PointerDataPacketConverter::ConvertPointerData(
     PointerData pointer_data,
     std::vector<PointerData>& converted_pointers) {
+  // Ignores pointer events with an invalid view ID.
+  if (!delegate_.ViewExists(pointer_data.view_id)) {
+    return;
+  }
   if (pointer_data.signal_kind == PointerData::SignalKind::kNone) {
     switch (pointer_data.change) {
       case PointerData::Change::kCancel: {

engine/src/flutter/lib/ui/window/pointer_data_packet_converter.h

@@ -75,7 +75,26 @@ struct PointerState {
 ///
 class PointerDataPacketConverter {
  public:
-  PointerDataPacketConverter();
+  // Used by PointerDataPacketConverter to query the system status.
+  //
+  // Typically RuntimeController.
+  class Delegate {
+   public:
+    Delegate() = default;
+
+    virtual ~Delegate() = default;
+
+    // Returns true if the specified view exists.
+    virtual bool ViewExists(int64_t view_id) const = 0;
+  };
+
+  //----------------------------------------------------------------------------
+  /// @brief      Create a PointerDataPacketConverter.
+  ///
+  /// @param[in]  delegate   A delegate to fulfill the query to the app state.
+  ///                        The delegate must exist throughout the lifetime
+  ///                        of this class. Typically `RuntimeController`.
+  explicit PointerDataPacketConverter(const Delegate& delegate);
   ~PointerDataPacketConverter();
 
   //----------------------------------------------------------------------------
@@ -85,6 +104,8 @@ class PointerDataPacketConverter {
   ///             pointer transitions. This method will fill out that
   ///             information and attempt to correct pointer transitions.
   ///
+  ///             Pointer data with invalid view IDs will be ignored.
+  ///
   /// @param[in]  packet                   The raw pointer packet sent from
   ///                                      embedding.
   ///
@@ -95,6 +116,9 @@ class PointerDataPacketConverter {
   std::unique_ptr<PointerDataPacket> Convert(const PointerDataPacket& packet);
 
  private:
+  const Delegate& delegate_;
+
+  // A map from pointer device ID to the state of the pointer.
   std::map<int64_t, PointerState> states_;
 
   int64_t pointer_ = 0;

engine/src/flutter/lib/ui/window/pointer_data_packet_converter_unittests.cc

@@ -5,12 +5,34 @@
 #include "flutter/lib/ui/window/pointer_data_packet_converter.h"
 
 #include <cstring>
+#include <unordered_set>
 
 #include "gtest/gtest.h"
 
 namespace flutter {
 namespace testing {
 
+namespace {
+
+constexpr int64_t kImplicitViewId = 0;
+
+}
+
+class TestDelegate : public PointerDataPacketConverter::Delegate {
+ public:
+  // |PointerDataPacketConverter::Delegate|
+  bool ViewExists(int64_t view_id) const override {
+    return views_.count(view_id) != 0;
+  }
+
+  void AddView(int64_t view_id) { views_.insert(view_id); }
+
+  void RemoveView(int64_t view_id) { views_.erase(view_id); }
+
+ private:
+  std::unordered_set<int64_t> views_;
+};
+
 void CreateSimulatedPointerData(PointerData& data,  // NOLINT
                                 PointerData::Change change,
                                 int64_t device,
@@ -45,7 +67,7 @@ void CreateSimulatedPointerData(PointerData& data,  // NOLINT
   data.platformData = 0;
   data.scroll_delta_x = 0.0;
   data.scroll_delta_y = 0.0;
-  data.view_id = 0;
+  data.view_id = kImplicitViewId;
 }
 
 void CreateSimulatedMousePointerData(PointerData& data,  // NOLINT
@@ -85,7 +107,7 @@ void CreateSimulatedMousePointerData(PointerData& data,  // NOLINT
   data.platformData = 0;
   data.scroll_delta_x = scroll_delta_x;
   data.scroll_delta_y = scroll_delta_y;
-  data.view_id = 0;
+  data.view_id = kImplicitViewId;
 }
 
 void CreateSimulatedTrackpadGestureData(PointerData& data,  // NOLINT
@@ -144,7 +166,9 @@ void UnpackPointerPacket(std::vector<PointerData>& output,  // NOLINT
 }
 
 TEST(PointerDataPacketConverterTest, CanConvertPointerDataPacket) {
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(kImplicitViewId);
+  PointerDataPacketConverter converter(delegate);
   auto packet = std::make_unique<PointerDataPacket>(6);
   PointerData data;
   CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);
@@ -193,7 +217,9 @@ TEST(PointerDataPacketConverterTest, CanConvertPointerDataPacket) {
 }
 
 TEST(PointerDataPacketConverterTest, CanSynthesizeDownAndUp) {
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(kImplicitViewId);
+  PointerDataPacketConverter converter(delegate);
   auto packet = std::make_unique<PointerDataPacket>(4);
   PointerData data;
   CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);
@@ -244,7 +270,9 @@ TEST(PointerDataPacketConverterTest, CanSynthesizeDownAndUp) {
 }
 
 TEST(PointerDataPacketConverterTest, CanUpdatePointerIdentifier) {
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(kImplicitViewId);
+  PointerDataPacketConverter converter(delegate);
   auto packet = std::make_unique<PointerDataPacket>(7);
   PointerData data;
   CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);
@@ -299,7 +327,9 @@ TEST(PointerDataPacketConverterTest, CanUpdatePointerIdentifier) {
 }
 
 TEST(PointerDataPacketConverterTest, AlwaysForwardMoveEvent) {
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(kImplicitViewId);
+  PointerDataPacketConverter converter(delegate);
   auto packet = std::make_unique<PointerDataPacket>(4);
   PointerData data;
   CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);
@@ -336,7 +366,9 @@ TEST(PointerDataPacketConverterTest, AlwaysForwardMoveEvent) {
 }
 
 TEST(PointerDataPacketConverterTest, CanWorkWithDifferentDevices) {
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(kImplicitViewId);
+  PointerDataPacketConverter converter(delegate);
   auto packet = std::make_unique<PointerDataPacket>(12);
   PointerData data;
   CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);
@@ -433,7 +465,9 @@ TEST(PointerDataPacketConverterTest, CanWorkWithDifferentDevices) {
 }
 
 TEST(PointerDataPacketConverterTest, CanSynthesizeAdd) {
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(kImplicitViewId);
+  PointerDataPacketConverter converter(delegate);
   auto packet = std::make_unique<PointerDataPacket>(2);
   PointerData data;
   CreateSimulatedPointerData(data, PointerData::Change::kDown, 0, 330.0, 450.0,
@@ -478,7 +512,9 @@ TEST(PointerDataPacketConverterTest, CanSynthesizeAdd) {
 
 TEST(PointerDataPacketConverterTest, CanHandleThreeFingerGesture) {
   // Regression test https://github.com/flutter/flutter/issues/20517.
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(kImplicitViewId);
+  PointerDataPacketConverter converter(delegate);
   PointerData data;
   std::vector<PointerData> result;
   // First finger down.
@@ -558,7 +594,9 @@ TEST(PointerDataPacketConverterTest, CanConvertPointerSignals) {
       PointerData::SignalKind::kScale,
   };
   for (const PointerData::SignalKind& kind : signal_kinds) {
-    PointerDataPacketConverter converter;
+    TestDelegate delegate;
+    delegate.AddView(kImplicitViewId);
+    PointerDataPacketConverter converter(delegate);
     auto packet = std::make_unique<PointerDataPacket>(6);
     PointerData data;
     CreateSimulatedMousePointerData(data, PointerData::Change::kAdd,
@@ -666,7 +704,9 @@ TEST(PointerDataPacketConverterTest, CanConvertPointerSignals) {
 }
 
 TEST(PointerDataPacketConverterTest, CanConvertTrackpadGesture) {
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(kImplicitViewId);
+  PointerDataPacketConverter converter(delegate);
   auto packet = std::make_unique<PointerDataPacket>(3);
   PointerData data;
   CreateSimulatedTrackpadGestureData(data, PointerData::Change::kPanZoomStart,
@@ -717,7 +757,10 @@ TEST(PointerDataPacketConverterTest, CanConvertTrackpadGesture) {
 }
 
 TEST(PointerDataPacketConverterTest, CanConvertViewId) {
-  PointerDataPacketConverter converter;
+  TestDelegate delegate;
+  delegate.AddView(100);
+  delegate.AddView(200);
+  PointerDataPacketConverter converter(delegate);
   auto packet = std::make_unique<PointerDataPacket>(2);
   PointerData data;
   CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);

engine/src/flutter/runtime/runtime_controller.cc

@@ -23,7 +23,10 @@ namespace flutter {
 
 RuntimeController::RuntimeController(RuntimeDelegate& p_client,
                                      const TaskRunners& task_runners)
-    : client_(p_client), vm_(nullptr), context_(task_runners) {}
+    : client_(p_client),
+      vm_(nullptr),
+      context_(task_runners),
+      pointer_data_packet_converter_(*this) {}
 
 RuntimeController::RuntimeController(
     RuntimeDelegate& p_client,
@@ -43,7 +46,8 @@ RuntimeController::RuntimeController(
       isolate_create_callback_(p_isolate_create_callback),
       isolate_shutdown_callback_(p_isolate_shutdown_callback),
       persistent_isolate_data_(std::move(p_persistent_isolate_data)),
-      context_(p_context) {}
+      context_(p_context),
+      pointer_data_packet_converter_(*this) {}
 
 std::unique_ptr<RuntimeController> RuntimeController::Spawn(
     RuntimeDelegate& p_client,
@@ -207,6 +211,10 @@ bool RuntimeController::RemoveView(int64_t view_id) {
   return platform_configuration->RemoveView(view_id);
 }
 
+bool RuntimeController::ViewExists(int64_t view_id) const {
+  return platform_data_.viewport_metrics_for_views.count(view_id) != 0;
+}
+
 bool RuntimeController::SetViewportMetrics(int64_t view_id,
                                            const ViewportMetrics& metrics) {
   TRACE_EVENT0("flutter", "SetViewportMetrics");
@@ -360,7 +368,9 @@ bool RuntimeController::DispatchPointerDataPacket(
     TRACE_EVENT0("flutter", "RuntimeController::DispatchPointerDataPacket");
     std::unique_ptr<PointerDataPacket> converted_packet =
         pointer_data_packet_converter_.Convert(packet);
-    platform_configuration->DispatchPointerDataPacket(*converted_packet);
+    if (converted_packet->GetLength() != 0) {
+      platform_configuration->DispatchPointerDataPacket(*converted_packet);
+    }
     return true;
   }
 

engine/src/flutter/runtime/runtime_controller.h

@@ -49,7 +49,8 @@ class Window;
 /// `RuntimeController` and flushed to the Dart VM when the isolate becomes
 /// ready before the entrypoint function. See `PlatformData`.
 ///
-class RuntimeController : public PlatformConfigurationClient {
+class RuntimeController : public PlatformConfigurationClient,
+                          PointerDataPacketConverter::Delegate {
  public:
   /// A callback that's invoked after this `RuntimeController` attempts to
   /// add a view to the Dart isolate.
@@ -723,6 +724,9 @@ class RuntimeController : public PlatformConfigurationClient {
 
   bool FlushRuntimeStateToIsolate();
 
+  // |PointerDataPacketConverter::Delegate|
+  bool ViewExists(int64_t view_id) const override;
+
   // |PlatformConfigurationClient|
   std::string DefaultRouteName() override;
 

engine/src/flutter/shell/common/input_events_unittests.cc

@@ -19,6 +19,12 @@ using UnitlessTime = int;
 // returns the time of that frame.
 using Generator = std::function<UnitlessTime(int)>;
 
+namespace {
+
+constexpr int64_t kImplicitViewId = 0;
+
+}
+
 //----------------------------------------------------------------------------
 /// Simulate n input events where the i-th one is delivered at delivery_time(i).
 ///
@@ -124,7 +130,9 @@ static void TestSimulatedInputEvents(
     for (int i = 0, j = 0; i < num_events; j += 1) {
       double t = j * frame_time;
       while (i < num_events && delivery_time(i) <= t) {
-        ShellTest::DispatchFakePointerData(shell.get());
+        // Use a different x every time for the pointer data converter to
+        // generate non-empty events.
+        ShellTest::DispatchFakePointerData(shell.get(), /*x=*/i);
         i += 1;
       }
       ShellTest::VSyncFlush(shell.get(), &will_draw_new_frame);
@@ -182,6 +190,7 @@ void CreateSimulatedPointerData(PointerData& data,
   data.platformData = 0;
   data.scroll_delta_x = 0.0;
   data.scroll_delta_y = 0.0;
+  data.view_id = kImplicitViewId;
 }
 
 TEST_F(ShellTest, MissAtMostOneFrameForIrregularInputEvents) {

engine/src/flutter/shell/common/shell_test.cc

@@ -258,8 +258,12 @@ void ShellTest::PumpOneFrame(Shell* shell, FrameContent frame_content) {
   latch.Wait();
 }
 
-void ShellTest::DispatchFakePointerData(Shell* shell) {
+void ShellTest::DispatchFakePointerData(Shell* shell, double x) {
   auto packet = std::make_unique<PointerDataPacket>(1);
+  packet->SetPointerData(0, PointerData{
+                                .change = PointerData::Change::kHover,
+                                .physical_x = x,
+                            });
   DispatchPointerData(shell, std::move(packet));
 }
 

engine/src/flutter/shell/common/shell_test.h

@@ -110,7 +110,9 @@ class ShellTest : public FixtureTest {
 
   static void PumpOneFrame(Shell* shell);
   static void PumpOneFrame(Shell* shell, FrameContent frame_content);
-  static void DispatchFakePointerData(Shell* shell);
+  // Dispatch a PointerHoverEvent with the specified `x` as the pointer
+  // position.
+  static void DispatchFakePointerData(Shell* shell, double x);
   static void DispatchPointerData(Shell* shell,
                                   std::unique_ptr<PointerDataPacket> packet);
   // Declare |UnreportedTimingsCount|, |GetNeedsReportTimings| and

engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc

@@ -3534,7 +3534,7 @@ TEST_F(EmbedderTest, CanSendPointerEventWithViewId) {
   builder.SetSoftwareRendererConfig();
   builder.SetDartEntrypoint("pointer_data_packet_view_id");
 
-  fml::AutoResetWaitableEvent ready_latch, count_latch, message_latch;
+  fml::AutoResetWaitableEvent ready_latch, add_view_latch, message_latch;
   context.AddNativeCallback(
       "SignalNativeTest",
       CREATE_NATIVE_ENTRY(
@@ -3550,9 +3550,31 @@ TEST_F(EmbedderTest, CanSendPointerEventWithViewId) {
 
   auto engine = builder.LaunchEngine();
   ASSERT_TRUE(engine.is_valid());
-
   ready_latch.Wait();
 
+  // Add view 2
+  FlutterWindowMetricsEvent metrics = {};
+  metrics.struct_size = sizeof(FlutterWindowMetricsEvent);
+  metrics.width = 800;
+  metrics.height = 600;
+  metrics.pixel_ratio = 1.0;
+  metrics.view_id = 2;
+
+  FlutterAddViewInfo info = {};
+  info.struct_size = sizeof(FlutterAddViewInfo);
+  info.view_id = 2;
+  info.view_metrics = &metrics;
+  info.add_view_callback = [](const FlutterAddViewResult* result) {
+    EXPECT_TRUE(result->added);
+    fml::AutoResetWaitableEvent* add_view_latch =
+        reinterpret_cast<fml::AutoResetWaitableEvent*>(result->user_data);
+    add_view_latch->Signal();
+  };
+  info.user_data = &add_view_latch;
+  ASSERT_EQ(FlutterEngineAddView(engine.get(), &info), kSuccess);
+  add_view_latch.Wait();
+
+  // Send a pointer event for view 2
   FlutterPointerEvent pointer_event = {};
   pointer_event.struct_size = sizeof(FlutterPointerEvent);
   pointer_event.phase = FlutterPointerPhase::kAdd;