Commit 2c73c7f3 authored by Jun Mukai's avatar Jun Mukai Committed by Commit Bot

Don't check mismatches in content::responsiveness::Watcher

content::responsiveness::Watcher can cause crashes of DCHECK
for checking mismatched events when tab-dragging is involved.

This is happening because Watcher watches both MessageLoopUI
and WindowEventDispatcher (through NativeEventObserver), but
OnWindowEventDispatcherFinishedProcessingEvent can be called
unexpectedly. When the dragged tab is attached to another
window, it means its WindowTreeHost and WindowEventDispatcher
is destructed, which causes FinishedProcessingEvent in a way
that Watcher thinks unmatched. More specifically:
. MessageLoopUI processes an event, Watcher::WillRunTaskOnUIThread
   is called.
2. WindowEventDispatcher::OnWindowEventDispatcherStartedProcessing
3. the event is to start dragging, creates a nested message loop.
4. MessageLoopUI processes another event, Watcher::WillRunTaskOnUIThread
5. if this causes attaching of the dragged tab, it closes the
   dragged browser window
6. close causes WindowEventDispatcher::OnWindowEventDispatcherFinishedProcessing
   on remaining events (for 2., in this case)
7. On Watcher::DidRunEventOnUIThread, it receives the identifier
   from 6 (i.e. the event for 2), but the last metadata_ui is
   from 4 (i.e. the task of messageloopUI), so mismatch happens

Absolutely the problem is the timing for 6, but it can happen
on other scenarios. This CL simply ignore mismatches on ChromeOS
with WindowService, and clears currently_running_metadata_ui_.
This will lose some data points for a short period of time, but
that can be acceptable for now.

Bug: 929813
Test: manually
Change-Id: I5f98d084bcbf7674ed62e790ccfbb804cd9cd3e7
Reviewed-on: https://chromium-review.googlesource.com/c/1474688Reviewed-by: default avatarErik Chen <erikchen@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632748}
parent fa57546b
......@@ -13,6 +13,10 @@
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#if defined(OS_CHROMEOS)
#include "ui/base/ui_base_features.h"
#endif
namespace content {
namespace responsiveness {
......@@ -187,6 +191,16 @@ void Watcher::DidRunTask(const base::PendingTask* task,
if (UNLIKELY(currently_running_metadata->empty() ||
(task != currently_running_metadata->back().identifier))) {
*mismatched_task_identifiers += 1;
#if defined(OS_CHROMEOS)
// Mismatches can happen often on ChromeOS with window service when
// tab-dragging is involved. Simply ignore the mismatches for now. See
// https://crbug.com/929813 for the details of why the mismatch happens.
// TODO(mukai): fix the event order issue.
if (features::IsUsingWindowService()) {
currently_running_metadata_ui_.clear();
return;
}
#endif
DCHECK_LE(*mismatched_task_identifiers, 1);
return;
}
......@@ -242,6 +256,16 @@ void Watcher::DidRunEventOnUIThread(const void* opaque_identifier) {
(opaque_identifier !=
currently_running_metadata_ui_.back().identifier))) {
mismatched_event_identifiers_ui_ += 1;
#if defined(OS_CHROMEOS)
// Mismatches can happen often on ChromeOS with window service when
// tab-dragging is involved. Simply ignore the mismatches for now. See
// https://crbug.com/929813 for the details of why the mismatch happens.
// TODO(mukai): fix the event order issue.
if (features::IsUsingWindowService()) {
currently_running_metadata_ui_.clear();
return;
}
#endif
DCHECK_LE(mismatched_event_identifiers_ui_, 1);
return;
}
......
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