Commit 866afbed authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

Remove temp fix for a crash in TouchActionFilter

Previously in order to prevent a crash, we add some temporary fix in
touch_action_filter.cc. With some of the recent committed changes, there
should be no more crashing.

This CL removes the temporary code for suppressing the crash but keeps
DumpWithoutCrashing so that we can get debugging info for the crash.

Bug: 851644
Change-Id: I614ae6d20a24b402141563451c51b36d52302d70
Reviewed-on: https://chromium-review.googlesource.com/1185627
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585803}
parent 939e4717
......@@ -46,15 +46,6 @@ TouchActionFilter::TouchActionFilter()
TouchActionFilter::~TouchActionFilter() {}
// In all the places that has DumpWithCrashing(), generate a report once every
// 100 times.
bool TouchActionFilter::ShouldDump() {
std::uniform_int_distribution<int> uniform_dist(0, 99);
if (uniform_dist(gen_) == 0)
return true;
return false;
}
FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
WebGestureEvent* gesture_event) {
if (gesture_event->SourceDevice() != blink::kWebGestureDeviceTouchscreen)
......@@ -67,16 +58,12 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
DCHECK(!suppress_manipulation_events_);
gesture_sequence_in_progress_ = true;
gesture_sequence_.append("B");
// TODO(https://crbug.com/851644): Make sure the value is properly set.
if (!scrolling_touch_action_.has_value()) {
if (ShouldDump()) {
static auto* crash_key = base::debug::AllocateCrashKeyString(
"scrollbegin-gestures", base::debug::CrashKeySize::Size256);
base::debug::SetCrashKeyString(crash_key, gesture_sequence_);
base::debug::DumpWithoutCrashing();
}
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);
}
suppress_manipulation_events_ =
ShouldSuppressManipulation(*gesture_event);
......@@ -146,16 +133,12 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
case WebInputEvent::kGestureTapUnconfirmed: {
gesture_sequence_.append("C");
DCHECK_EQ(1, gesture_event->data.tap.tap_count);
// TODO(https://crbug.com/851644): Make sure the value is properly set.
if (!scrolling_touch_action_.has_value()) {
if (ShouldDump()) {
static auto* crash_key = base::debug::AllocateCrashKeyString(
"tapunconfirmed-gestures", base::debug::CrashKeySize::Size256);
base::debug::SetCrashKeyString(crash_key, gesture_sequence_);
base::debug::DumpWithoutCrashing();
}
static auto* crash_key = base::debug::AllocateCrashKeyString(
"tapunconfirmed-gestures", base::debug::CrashKeySize::Size256);
base::debug::SetCrashKeyString(crash_key, gesture_sequence_);
base::debug::DumpWithoutCrashing();
gesture_sequence_.clear();
SetTouchAction(cc::kTouchActionAuto);
}
allow_current_double_tap_event_ = (scrolling_touch_action_.value() &
cc::kTouchActionDoubleTapZoom) != 0;
......@@ -184,16 +167,12 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
if (gesture_event->is_source_touch_event_set_non_blocking)
SetTouchAction(cc::kTouchActionAuto);
scrolling_touch_action_ = allowed_touch_action_;
// TODO(https://crbug.com/851644): Make sure the value is properly set.
if (!scrolling_touch_action_.has_value()) {
if (ShouldDump()) {
static auto* crash_key = base::debug::AllocateCrashKeyString(
"tapdown-gestures", base::debug::CrashKeySize::Size256);
base::debug::SetCrashKeyString(crash_key, gesture_sequence_);
base::debug::DumpWithoutCrashing();
}
static auto* crash_key = base::debug::AllocateCrashKeyString(
"tapdown-gestures", base::debug::CrashKeySize::Size256);
base::debug::SetCrashKeyString(crash_key, gesture_sequence_);
base::debug::DumpWithoutCrashing();
gesture_sequence_.clear();
SetTouchAction(cc::kTouchActionAuto);
}
DCHECK(!drop_current_tap_ending_event_);
break;
......
......@@ -5,8 +5,6 @@
#ifndef CONTENT_BROWSER_RENDERER_HOST_INPUT_TOUCH_ACTION_FILTER_H_
#define CONTENT_BROWSER_RENDERER_HOST_INPUT_TOUCH_ACTION_FILTER_H_
#include <random>
#include "base/macros.h"
#include "base/optional.h"
#include "cc/input/touch_action.h"
......@@ -81,9 +79,6 @@ class CONTENT_EXPORT TouchActionFilter {
void ReportTouchAction();
void SetTouchAction(cc::TouchAction touch_action);
// Debugging only.
bool ShouldDump();
// Whether scroll and pinch gestures should be discarded due to touch-action.
bool suppress_manipulation_events_;
......@@ -124,7 +119,6 @@ class CONTENT_EXPORT TouchActionFilter {
// Debugging only.
std::string gesture_sequence_;
std::default_random_engine gen_;
DISALLOW_COPY_AND_ASSIGN(TouchActionFilter);
};
......
......@@ -890,8 +890,8 @@ TEST_F(TouchActionFilterTest, TouchActionNotResetWithinGestureSequence) {
EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
FilterGestureEventResult::kFilterGestureEventAllowed);
EXPECT_TRUE(filter_.allowed_touch_action().has_value());
EXPECT_TRUE(ScrollingTouchAction().has_value());
EXPECT_FALSE(filter_.allowed_touch_action().has_value());
EXPECT_FALSE(ScrollingTouchAction().has_value());
}
// This test ensures that when the IPC message OnHasTouchEventHandlers is
......
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