Make pointer events ignore invalid views (#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
diff --git a/lib/ui/window/pointer_data_packet_converter.cc b/lib/ui/window/pointer_data_packet_converter.cc
index 5cf2005..45b05a0 100644
--- a/lib/ui/window/pointer_data_packet_converter.cc
+++ b/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 @@
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: {
diff --git a/lib/ui/window/pointer_data_packet_converter.h b/lib/ui/window/pointer_data_packet_converter.h
index e903e4e..146ade2 100644
--- a/lib/ui/window/pointer_data_packet_converter.h
+++ b/lib/ui/window/pointer_data_packet_converter.h
@@ -75,7 +75,26 @@
///
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 @@
/// 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 @@
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;
diff --git a/lib/ui/window/pointer_data_packet_converter_unittests.cc b/lib/ui/window/pointer_data_packet_converter_unittests.cc
index 69f798d..4f6a37f 100644
--- a/lib/ui/window/pointer_data_packet_converter_unittests.cc
+++ b/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 @@
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 @@
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 @@
}
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, 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, 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, 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, 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, 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, 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 @@
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, 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, 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);
diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc
index ad4fe09..6a607ed 100644
--- a/runtime/runtime_controller.cc
+++ b/runtime/runtime_controller.cc
@@ -23,7 +23,10 @@
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 @@
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 @@
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 @@
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;
}
diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h
index 89f3eaf..93da070 100644
--- a/runtime/runtime_controller.h
+++ b/runtime/runtime_controller.h
@@ -49,7 +49,8 @@
/// `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 @@
bool FlushRuntimeStateToIsolate();
+ // |PointerDataPacketConverter::Delegate|
+ bool ViewExists(int64_t view_id) const override;
+
// |PlatformConfigurationClient|
std::string DefaultRouteName() override;
diff --git a/shell/common/input_events_unittests.cc b/shell/common/input_events_unittests.cc
index d8ee75f..a0a22d1 100644
--- a/shell/common/input_events_unittests.cc
+++ b/shell/common/input_events_unittests.cc
@@ -19,6 +19,12 @@
// 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 @@
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 @@
data.platformData = 0;
data.scroll_delta_x = 0.0;
data.scroll_delta_y = 0.0;
+ data.view_id = kImplicitViewId;
}
TEST_F(ShellTest, MissAtMostOneFrameForIrregularInputEvents) {
diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc
index 90dcf35..6683399 100644
--- a/shell/common/shell_test.cc
+++ b/shell/common/shell_test.cc
@@ -258,8 +258,12 @@
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));
}
diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h
index c11ad11..b5f832a 100644
--- a/shell/common/shell_test.h
+++ b/shell/common/shell_test.h
@@ -110,7 +110,9 @@
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
diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc
index 1e1075f..2b03e51 100644
--- a/shell/platform/embedder/tests/embedder_unittests.cc
+++ b/shell/platform/embedder/tests/embedder_unittests.cc
@@ -3534,7 +3534,7 @@
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 @@
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;
