Commit 266da78a authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

Fix and instrument Mac crash in TabSwitchTimeRecorder.

Crash reports indicate that
TabSwitchTimeRecorder::RecordHistogramsAndTraceEvents is occasionnally
called with |tab_switch_start_state_| being nullopt.

However:

- TabWasShown() sets |tab_switch_start_state_| and issues a
  RecordHistogramsAndTraceEvents() callback.
- RecordHistogramsAndTraceEvents() expects
  |tab_switch_start_state_| to be set, and resets it before returning.
- TabWasHidden() invokes RecordHistogramsAndTraceEvents() directly
  and invalidates previous callbacks issued by TabWasShown().

For the crash to happen, there must be multiple calls to
TabWasShown() without the tab being hidden in between, which
shouldn't happen.

This CL makes the following changes:
- Dump without crashing when TabWasShown() is called and
  |tab_switch_start_state_| is set (indicates that neither
  TabWasHidden() nor the RecordHistogramsAndTraceEvents()
  were invoked since the last call to TabWasShown()).
- Invalidate previous callbacks issued by TabWasShown(), to prevent
  the crash from happening.

A version of this CL without the call to DumpWithoutCrashing()
should be merged to M76 branch before it is promoted to stable.

Bug: 981757
Change-Id: Ic67c696022a73eb21c2ad48577f5e22c176f7987
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1695747Reviewed-by: default avatarJonathan Ross <jonross@chromium.org>
Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676540}
parent 9da5f65b
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/debug/dump_without_crashing.h"
#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
...@@ -62,6 +63,17 @@ TabSwitchTimeRecorder::TabWasShown( ...@@ -62,6 +63,17 @@ TabSwitchTimeRecorder::TabWasShown(
DCHECK(!tab_switch_start_state_); DCHECK(!tab_switch_start_state_);
DCHECK(render_widget_visibility_request_timestamp_.is_null()); DCHECK(render_widget_visibility_request_timestamp_.is_null());
if (tab_switch_start_state_) {
// TabWasShown() is called multiple times without the tab being hidden in
// between. This shouldn't happen per the DCHECK above. Dump without
// crashing to gather more information for https://crbug.com/981757.
//
// TODO(fdoray): This code should be removed no later than August 30, 2019.
// https://crbug.com/981757
base::debug::DumpWithoutCrashing();
weak_ptr_factory_.InvalidateWeakPtrs();
}
has_saved_frames_ = has_saved_frames; has_saved_frames_ = has_saved_frames;
tab_switch_start_state_ = start_state; tab_switch_start_state_ = start_state;
render_widget_visibility_request_timestamp_ = render_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