Commit 6c5b2186 authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

Revert "[Debugging] Add instrumentation in TouchActionFilter"

This reverts commit a0de839e.

Reason for revert:
As it is mentioned in crbug.com/869375:
Know that these are not real crashes but being a top crash this may result in suppression of other crashes

Original change's description:
> [Debugging] Add instrumentation in TouchActionFilter
> 
> From the stack trace of the current crash report, we are not able to
> tell the gesture sequence, which makes it hard to debug.
> 
> This CL adds a CrashKeyString in TouchActionFilter, and push necessary
> events into that string. When it crashes, we should be able to retrieve
> the gesture sequence from that crash key.
> 
> Bug: 869375
> Change-Id: Ieb7cf0839e4a062c23ec48cbaca72bd641f28a30
> Reviewed-on: https://chromium-review.googlesource.com/1165586
> Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
> Commit-Queue: Xida Chen <xidachen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581676}

TBR=dtapuska@chromium.org,xidachen@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 869375
Change-Id: I033b2b1bd646c1310593e612322e4bb2d5c503d2
Reviewed-on: https://chromium-review.googlesource.com/1172522Reviewed-by: default avatarXida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582579}
parent 7266a9a7
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
#include <math.h> #include <math.h>
#include "base/debug/crash_logging.h"
#include "base/debug/dump_without_crashing.h" #include "base/debug/dump_without_crashing.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
...@@ -43,7 +42,7 @@ TouchActionFilter::TouchActionFilter() ...@@ -43,7 +42,7 @@ TouchActionFilter::TouchActionFilter()
force_enable_zoom_(false) {} force_enable_zoom_(false) {}
TouchActionFilter::~TouchActionFilter() { TouchActionFilter::~TouchActionFilter() {
gesture_sequence_.clear(); function_call_sequence_.clear();
} }
FilterGestureEventResult TouchActionFilter::FilterGestureEvent( FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
...@@ -57,7 +56,6 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent( ...@@ -57,7 +56,6 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
case WebInputEvent::kGestureScrollBegin: { case WebInputEvent::kGestureScrollBegin: {
DCHECK(!suppress_manipulation_events_); DCHECK(!suppress_manipulation_events_);
DCHECK(!touchscreen_scroll_in_progress_); DCHECK(!touchscreen_scroll_in_progress_);
gesture_sequence_.append("GSB ");
touchscreen_scroll_in_progress_ = true; touchscreen_scroll_in_progress_ = true;
// TODO(https://crbug.com/851644): Make sure the value is properly set. // TODO(https://crbug.com/851644): Make sure the value is properly set.
if (!scrolling_touch_action_.has_value()) if (!scrolling_touch_action_.has_value())
...@@ -103,7 +101,7 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent( ...@@ -103,7 +101,7 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
break; break;
case WebInputEvent::kGestureScrollEnd: case WebInputEvent::kGestureScrollEnd:
gesture_sequence_.clear(); function_call_sequence_.clear();
DCHECK(touchscreen_scroll_in_progress_); DCHECK(touchscreen_scroll_in_progress_);
touchscreen_scroll_in_progress_ = false; touchscreen_scroll_in_progress_ = false;
ReportGestureEventFiltered(suppress_manipulation_events_); ReportGestureEventFiltered(suppress_manipulation_events_);
...@@ -156,10 +154,17 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent( ...@@ -156,10 +154,17 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
scrolling_touch_action_ = allowed_touch_action_; scrolling_touch_action_ = allowed_touch_action_;
// TODO(https://crbug.com/851644): Make sure the value is properly set. // TODO(https://crbug.com/851644): Make sure the value is properly set.
if (!scrolling_touch_action_.has_value()) { if (!scrolling_touch_action_.has_value()) {
static auto* crash_key = base::debug::AllocateCrashKeyString( auto index_of_set =
"touchaction-gestures", base::debug::CrashKeySize::Size256); std::find(std::begin(function_call_sequence_),
base::debug::SetCrashKeyString(crash_key, gesture_sequence_); std::end(function_call_sequence_), kOnSetTouchActionCall);
base::debug::DumpWithoutCrashing(); auto index_of_reset =
std::find(std::begin(function_call_sequence_),
std::end(function_call_sequence_), kResetTouchActionCall);
if (index_of_set != std::end(function_call_sequence_) &&
index_of_reset != std::end(function_call_sequence_) &&
index_of_reset > index_of_set) {
base::debug::DumpWithoutCrashing();
}
SetTouchAction(cc::kTouchActionAuto); SetTouchAction(cc::kTouchActionAuto);
} }
DCHECK(!drop_current_tap_ending_event_); DCHECK(!drop_current_tap_ending_event_);
...@@ -189,7 +194,7 @@ bool TouchActionFilter::FilterManipulationEventAndResetState() { ...@@ -189,7 +194,7 @@ bool TouchActionFilter::FilterManipulationEventAndResetState() {
void TouchActionFilter::OnSetTouchAction(cc::TouchAction touch_action) { void TouchActionFilter::OnSetTouchAction(cc::TouchAction touch_action) {
if (touch_action != cc::kTouchActionAuto) if (touch_action != cc::kTouchActionAuto)
gesture_sequence_.append("Set "); function_call_sequence_.push_back(kOnSetTouchActionCall);
// TODO(https://crbug.com/849819): add a DCHECK for // TODO(https://crbug.com/849819): add a DCHECK for
// |has_touch_event_handler_|. // |has_touch_event_handler_|.
// For multiple fingers, we take the intersection of the touch actions for // For multiple fingers, we take the intersection of the touch actions for
...@@ -248,7 +253,7 @@ void TouchActionFilter::ResetTouchAction() { ...@@ -248,7 +253,7 @@ void TouchActionFilter::ResetTouchAction() {
// their begin event(s) suppressed will be suppressed until the next // their begin event(s) suppressed will be suppressed until the next
// sequenceo. // sequenceo.
if (has_touch_event_handler_) { if (has_touch_event_handler_) {
gesture_sequence_.append("Reset "); function_call_sequence_.push_back(kResetTouchActionCall);
allowed_touch_action_.reset(); allowed_touch_action_.reset();
white_listed_touch_action_.reset(); white_listed_touch_action_.reset();
} else { } else {
......
...@@ -113,7 +113,14 @@ class CONTENT_EXPORT TouchActionFilter { ...@@ -113,7 +113,14 @@ class CONTENT_EXPORT TouchActionFilter {
// Whitelisted touch action received from the compositor. // Whitelisted touch action received from the compositor.
base::Optional<cc::TouchAction> white_listed_touch_action_; base::Optional<cc::TouchAction> white_listed_touch_action_;
std::string gesture_sequence_; // DEBUG ONLY! Record the sequence of function calls, cleared at GSE. When it
// is GSB and |scrolling_touch_action_| has no value, check this sequence.
enum FunctionCalls {
kOnSetTouchActionCall,
kResetTouchActionCall,
};
std::vector<FunctionCalls> function_call_sequence_;
DISALLOW_COPY_AND_ASSIGN(TouchActionFilter); DISALLOW_COPY_AND_ASSIGN(TouchActionFilter);
}; };
......
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