Commit b4d125cc authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

Fix a potential crash in TouchActionFilter

In the case when a GestureScrollBegin comes without a GestureTapDown,
and that when the both |allowed_touch_action_| and |active_touch_action_|
have no value, it could crash at line 139 which tries to access the value
of |active_touch_action_|.

This CL fixes that and adds unit testing.

Bug: 904648
Change-Id: Id4bc2a79f8716ed43981a77278d82d3539fc9a5e
Reviewed-on: https://chromium-review.googlesource.com/c/1495354Reviewed-by: default avatarNavid Zolghadr <nzolghadr@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636901}
parent 32a64276
......@@ -134,19 +134,20 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
active_touch_action_ = allowed_touch_action_;
touch_action = allowed_touch_action_.value();
} else {
touch_action = compositor_touch_action_enabled_
? white_listed_touch_action_
: active_touch_action_.value();
if (compositor_touch_action_enabled_) {
touch_action = white_listed_touch_action_;
} else {
gesture_sequence_.append("B");
static auto* crash_key = base::debug::AllocateCrashKeyString(
"scrollbegin-gestures", base::debug::CrashKeySize::Size256);
base::debug::SetCrashKeyString(crash_key, gesture_sequence_);
base::debug::DumpWithoutCrashing();
gesture_sequence_.clear();
SetTouchAction(cc::kTouchActionAuto);
touch_action = cc::kTouchActionAuto;
}
}
}
gesture_sequence_.append("B");
if (!compositor_touch_action_enabled_ &&
!active_touch_action_.has_value()) {
static auto* crash_key = base::debug::AllocateCrashKeyString(
"scrollbegin-gestures", base::debug::CrashKeySize::Size256);
base::debug::SetCrashKeyString(crash_key, gesture_sequence_);
gesture_sequence_.clear();
}
drop_scroll_events_ =
ShouldSuppressScrolling(*gesture_event, touch_action);
FilterGestureEventResult res;
......
......@@ -50,6 +50,9 @@ class TouchActionFilterTest : public testing::Test,
void SetGestureSequenceInProgress() {
filter_.gesture_sequence_in_progress_ = true;
}
void ResetGestureSequenceInProgress() {
filter_.gesture_sequence_in_progress_ = false;
}
void PanTest(cc::TouchAction action,
float scroll_x,
float scroll_y,
......@@ -1427,6 +1430,23 @@ TEST_P(TouchActionFilterTest, ScrollBeginWithoutTapDown) {
EXPECT_EQ(ActiveTouchAction().value(), cc::kTouchActionPan);
EXPECT_EQ(filter_.allowed_touch_action().value(), cc::kTouchActionPan);
}
ResetTouchAction();
ResetActiveTouchAction();
ResetGestureSequenceInProgress();
EXPECT_FALSE(ActiveTouchAction().has_value());
EXPECT_FALSE(filter_.allowed_touch_action().has_value());
// Ensure that there is no crash at GSB if both |allowed_| and |active_|
// touch action have no value.
if (compositor_touch_action_enabled_)
filter_.OnSetWhiteListedTouchAction(cc::kTouchActionPan);
EXPECT_EQ(filter_.FilterGestureEvent(&scroll_begin),
FilterGestureEventResult::kFilterGestureEventAllowed);
if (!compositor_touch_action_enabled_) {
EXPECT_EQ(filter_.allowed_touch_action().value(), cc::kTouchActionAuto);
EXPECT_EQ(ActiveTouchAction().value(), cc::kTouchActionAuto);
}
}
// This tests a gesture tap down with |num_of_active_touches_| == 0
......
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