Commit 674fc087 authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

Mitigate redundant calls to ContentToVisibleTimeReporter::TabWasShown().

crbug.com/1121339 indicates that there can be a null-pointer access in
ContentToVisibleTimeReporter::RecordHistogramsAndTraceEvents() on Mac
since 87.0.4241.0. The accessed variable is likely
|tab_switch_start_state_|. Since |tab_switch_start_state_| is set before
issuing RecordHistogramsAndTraceEvents() callback and reset at the end
of that callback, a null-pointer access suggests that TabWasShown() was
invoked twice, without a call to RecordHistogramsAndTraceEvents() or
TabWasHidden() in-between.

To mitigate the crash, this CL invalidates previously issued callbacks
when TabWasShown() is invoked. This mitigation is useful to avoid
crashes in branch M87.

In a separate CL, we will add a DumpWithoutCrashing() to diagnose
cases where TabWasShown() is invoked twice, without a call to
RecordHistogramsAndTraceEvents() or TabWasHidden() in-between.

Bug: 1121339
Change-Id: I6526cf3a001e9647c5f8ec1081f4747f94a160f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414408
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808340}
parent e1175754
...@@ -82,6 +82,14 @@ ContentToVisibleTimeReporter::TabWasShown( ...@@ -82,6 +82,14 @@ ContentToVisibleTimeReporter::TabWasShown(
DCHECK(!tab_switch_start_state_); DCHECK(!tab_switch_start_state_);
DCHECK(widget_visibility_request_timestamp_.is_null()); DCHECK(widget_visibility_request_timestamp_.is_null());
// Invalidate previously issued callbacks, to avoid accessing a null
// |tab_switch_start_state_|.
//
// TODO(https://crbug.com/1121339): Make sure that TabWasShown() is never
// called twice without a call to TabWasHidden() in-between, and remove this
// mitigation.
weak_ptr_factory_.InvalidateWeakPtrs();
has_saved_frames_ = has_saved_frames; has_saved_frames_ = has_saved_frames;
tab_switch_start_state_ = std::move(start_state); tab_switch_start_state_ = std::move(start_state);
widget_visibility_request_timestamp_ = widget_visibility_request_timestamp; widget_visibility_request_timestamp_ = widget_visibility_request_timestamp;
......
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