Commit 5444194b authored by jonross's avatar jonross Committed by Commit bot

Prevent TouchEvent Crash on X

When TouchEvents are copied the base::NativeEvent of the copy is set as null for
GenericEvent types, such as touchs.

Touch Release and Cancel events are set to remove a native touch id mapping upon
destruction.

This causes copied Touch Release/Cancel events to attempt to remove a mapping on
a null native event. Leading to a crash.

I've explicitly defined a copy constructor on TouchEvent to stop copied events
from attempting to remove this mapping.

TEST=EventsXTest.CopiedTouchEventNotRemovingFromNativeMapping
BUG=467102

Review URL: https://codereview.chromium.org/1026573002

Cr-Commit-Position: refs/heads/master@{#321563}
parent a77f0444
...@@ -582,6 +582,21 @@ TouchEvent::TouchEvent(EventType type, ...@@ -582,6 +582,21 @@ TouchEvent::TouchEvent(EventType type,
FixRotationAngle(); FixRotationAngle();
} }
TouchEvent::TouchEvent(const TouchEvent& copy)
: LocatedEvent(copy),
touch_id_(copy.touch_id_),
unique_event_id_(copy.unique_event_id_),
radius_x_(copy.radius_x_),
radius_y_(copy.radius_y_),
rotation_angle_(copy.rotation_angle_),
force_(copy.force_),
may_cause_scrolling_(copy.may_cause_scrolling_),
should_remove_native_touch_id_mapping_(false) {
// Copied events should not remove touch id mapping, as this either causes the
// mapping to be lost before the initial event has finished dispatching, or
// the copy to attempt to remove the mapping from a null |native_event_|.
}
TouchEvent::~TouchEvent() { TouchEvent::~TouchEvent() {
// In ctor TouchEvent(native_event) we call GetTouchId() which in X11 // In ctor TouchEvent(native_event) we call GetTouchId() which in X11
// platform setups the tracking_id to slot mapping. So in dtor here, // platform setups the tracking_id to slot mapping. So in dtor here,
......
...@@ -515,6 +515,8 @@ class EVENTS_EXPORT TouchEvent : public LocatedEvent { ...@@ -515,6 +515,8 @@ class EVENTS_EXPORT TouchEvent : public LocatedEvent {
float angle, float angle,
float force); float force);
TouchEvent(const TouchEvent& copy);
~TouchEvent() override; ~TouchEvent() override;
// The id of the pointer this event modifies. // The id of the pointer this event modifies.
......
...@@ -353,6 +353,26 @@ TEST_F(EventsXTest, TouchEventNotRemovingFromNativeMapping) { ...@@ -353,6 +353,26 @@ TEST_F(EventsXTest, TouchEventNotRemovingFromNativeMapping) {
EXPECT_EQ(-1, GetTouchIdForTrackingId(kTrackingId)); EXPECT_EQ(-1, GetTouchIdForTrackingId(kTrackingId));
} }
// Copied events should not remove native touch id mappings, as this causes a
// crash (crbug.com/467102). Copied events do not contain a proper
// base::NativeEvent and should not attempt to access it.
TEST_F(EventsXTest, CopiedTouchEventNotRemovingFromNativeMapping) {
std::vector<unsigned int> devices;
devices.push_back(0);
ui::SetUpTouchDevicesForTest(devices);
std::vector<Valuator> valuators;
// Create a release event which has a native touch id mapping.
ui::ScopedXI2Event xrelease0;
xrelease0.InitTouchEvent(0, XI_TouchEnd, 0, gfx::Point(10, 10), valuators);
ui::TouchEvent urelease0(xrelease0);
{
// When the copy is destructed it should not attempt to remove the mapping.
// Exiting this scope should not cause a crash.
ui::TouchEvent copy = urelease0;
}
}
TEST_F(EventsXTest, NumpadKeyEvents) { TEST_F(EventsXTest, NumpadKeyEvents) {
XEvent event; XEvent event;
Display* display = gfx::GetXDisplay(); Display* display = gfx::GetXDisplay();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment