Handle out-of-order add/remove pointer events (#55740)

Fixes [#146251](flutter/flutter#146251).

On Windows, a "pointer add" event to a new view is sometimes sent before the "pointer remove" event from the old view. When `PointerDataPacketConverter` handles the add event, it asserts that the device's pointer state has been cleared. Since the pointer state is cleared only when the remove event is processed, the assertion fails if the events arrive out of order.

The solution proposed here is to synthesize a remove event if an add event is received while the pointer state hasn't been cleared (i.e., if the pointer is added without being removed from the previous view).

To avoid duplicate remove events, the view ID is now tracked in `PointerState`. If the original "remove" arrives after the synthesized one, the state's view ID will differ from the pointer data's view ID, allowing it to be safely ignored.

When synthesizing the remove event, it's possible that the old view has already been destroyed, meaning no remove event will be received later. For example, this occurs when a window is destroyed while the pointer is inside it. However, the framework expects an "add" event to always follow a "remove" event, and "remove" events with invalid views are ignored. To meet the framework's expectations, the view ID of the "add" event will be used for the "remove" event if the old view has been destroyed.

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

lib/ui/window/pointer_data_packet_converter.cc

@@ -74,7 +74,45 @@ void PointerDataPacketConverter::ConvertPointerData(
         break;
       }
       case PointerData::Change::kAdd: {
-        FML_DCHECK(states_.find(pointer_data.device) == states_.end());
+        auto iter = states_.find(pointer_data.device);
+        if (iter != states_.end()) {
+          // Synthesizes a remove event if the pointer was not previously
+          // removed.
+          PointerState state = iter->second;
+          PointerData synthesized_data = pointer_data;
+          synthesized_data.physical_x = state.physical_x;
+          synthesized_data.physical_y = state.physical_y;
+          synthesized_data.pan_x = state.pan_x;
+          synthesized_data.pan_y = state.pan_y;
+          synthesized_data.scale = state.scale;
+          synthesized_data.rotation = state.rotation;
+          synthesized_data.buttons = state.buttons;
+          synthesized_data.synthesized = 1;
+
+          // The framework expects an add event to always follow a remove event,
+          // and remove events with invalid views are ignored.
+          // To meet the framework's expectations, the view ID of the add event
+          // is used for the remove event if the old view has been destroyed.
+          if (delegate_.ViewExists(state.view_id)) {
+            synthesized_data.view_id = state.view_id;
+
+            if (state.is_down) {
+              // Synthesizes cancel event if the pointer is down.
+              PointerData synthesized_cancel_event = synthesized_data;
+              synthesized_cancel_event.change = PointerData::Change::kCancel;
+              UpdatePointerIdentifier(synthesized_cancel_event, state, false);
+
+              state.is_down = false;
+              converted_pointers.push_back(synthesized_cancel_event);
+            }
+          }
+
+          PointerData synthesized_remove_event = synthesized_data;
+          synthesized_remove_event.change = PointerData::Change::kRemove;
+
+          converted_pointers.push_back(synthesized_remove_event);
+        }
+
         EnsurePointerState(pointer_data);
         converted_pointers.push_back(pointer_data);
         break;
@@ -84,6 +122,11 @@ void PointerDataPacketConverter::ConvertPointerData(
         auto iter = states_.find(pointer_data.device);
         FML_DCHECK(iter != states_.end());
         PointerState state = iter->second;
+        if (state.view_id != pointer_data.view_id) {
+          // Ignores remove event if the pointer was previously added to a
+          // different view.
+          break;
+        }
 
         if (state.is_down) {
           // Synthesizes cancel event if the pointer is down.
@@ -362,6 +405,7 @@ PointerState PointerDataPacketConverter::EnsurePointerState(
   state.physical_y = pointer_data.physical_y;
   state.pan_x = 0;
   state.pan_y = 0;
+  state.view_id = pointer_data.view_id;
   states_[pointer_data.device] = state;
   return state;
 }

lib/ui/window/pointer_data_packet_converter.h

@@ -38,6 +38,7 @@ struct PointerState {
   double scale;
   double rotation;
   int64_t buttons;
+  int64_t view_id;
 };
 
 //------------------------------------------------------------------------------

lib/ui/window/pointer_data_packet_converter_unittests.cc

@@ -510,6 +510,108 @@ TEST(PointerDataPacketConverterTest, CanSynthesizeAdd) {
   ASSERT_EQ(result[3].buttons, 0);
 }
 
+TEST(PointerDataPacketConverterTest, CanSynthesizeRemove) {
+  TestDelegate delegate;
+  delegate.AddView(100);
+  delegate.AddView(200);
+  PointerDataPacketConverter converter(delegate);
+  auto packet = std::make_unique<PointerDataPacket>(3);
+
+  PointerData data;
+  CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);
+  data.view_id = 100;
+  packet->SetPointerData(0, data);
+  CreateSimulatedPointerData(data, PointerData::Change::kDown, 0, 3.0, 4.0, 1);
+  data.view_id = 100;
+  packet->SetPointerData(1, data);
+  CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);
+  data.view_id = 200;
+  packet->SetPointerData(2, data);
+  auto converted_packet = converter.Convert(*packet);
+
+  std::vector<PointerData> result;
+  UnpackPointerPacket(result, std::move(converted_packet));
+
+  ASSERT_EQ(result.size(), (size_t)6);
+  ASSERT_EQ(result[0].synthesized, 0);
+  ASSERT_EQ(result[0].view_id, 100);
+
+  // A hover should be synthesized.
+  ASSERT_EQ(result[1].change, PointerData::Change::kHover);
+  ASSERT_EQ(result[1].synthesized, 1);
+  ASSERT_EQ(result[1].physical_delta_x, 3.0);
+  ASSERT_EQ(result[1].physical_delta_y, 4.0);
+  ASSERT_EQ(result[1].buttons, 0);
+
+  ASSERT_EQ(result[2].change, PointerData::Change::kDown);
+  ASSERT_EQ(result[2].pointer_identifier, 1);
+  ASSERT_EQ(result[2].synthesized, 0);
+  ASSERT_EQ(result[2].buttons, 1);
+
+  // A cancel should be synthesized.
+  ASSERT_EQ(result[3].change, PointerData::Change::kCancel);
+  ASSERT_EQ(result[3].pointer_identifier, 1);
+  ASSERT_EQ(result[3].synthesized, 1);
+  ASSERT_EQ(result[3].physical_x, 3.0);
+  ASSERT_EQ(result[3].physical_y, 4.0);
+  ASSERT_EQ(result[3].buttons, 1);
+
+  // A remove should be synthesized.
+  ASSERT_EQ(result[4].physical_x, 3.0);
+  ASSERT_EQ(result[4].physical_y, 4.0);
+  ASSERT_EQ(result[4].synthesized, 1);
+  ASSERT_EQ(result[4].view_id, 100);
+
+  ASSERT_EQ(result[5].synthesized, 0);
+  ASSERT_EQ(result[5].view_id, 200);
+}
+
+TEST(PointerDataPacketConverterTest,
+     CanAvoidDoubleRemoveAfterSynthesizedRemove) {
+  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);
+  data.view_id = 100;
+  packet->SetPointerData(0, data);
+  CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0);
+  data.view_id = 200;
+  packet->SetPointerData(1, data);
+  auto converted_packet = converter.Convert(*packet);
+
+  std::vector<PointerData> result;
+  UnpackPointerPacket(result, std::move(converted_packet));
+
+  ASSERT_EQ(result.size(), (size_t)3);
+  ASSERT_EQ(result[0].synthesized, 0);
+  ASSERT_EQ(result[0].view_id, 100);
+
+  // A remove should be synthesized.
+  ASSERT_EQ(result[1].synthesized, 1);
+  ASSERT_EQ(result[1].view_id, 100);
+
+  ASSERT_EQ(result[2].synthesized, 0);
+  ASSERT_EQ(result[2].view_id, 200);
+
+  // Simulate a double remove.
+  packet = std::make_unique<PointerDataPacket>(1);
+  CreateSimulatedPointerData(data, PointerData::Change::kRemove, 0, 0.0, 0.0,
+                             0);
+  data.view_id = 100;
+  packet->SetPointerData(0, data);
+  converted_packet = converter.Convert(*packet);
+
+  result.clear();
+  UnpackPointerPacket(result, std::move(converted_packet));
+
+  // The double remove should be ignored.
+  ASSERT_EQ(result.size(), (size_t)0);
+}
+
 TEST(PointerDataPacketConverterTest, CanHandleThreeFingerGesture) {
   // Regression test https://github.com/flutter/flutter/issues/20517.
   TestDelegate delegate;