Commit 41cf53cb authored by David Bienvenu's avatar David Bienvenu Committed by Commit Bot

Remove CHECK to stop canary from crashing so frequently for some canary users.

This CL attempts to handle the unexpected situation we were CHECKING for.

The windows native window occlusion finch experiment is causing a lot
of check fails with |window_is_moving_| true when we receive a EVENT_SYSTEM_MOVESIZESTART
event. I'm unable to reproduce this, but this CL should prevent us from getting stuck
in the |window_is_moving_| state while still avoiding calculating occlusion during moves.

Also attempt to detect when we've possibly missed an end move event, because we
got an unexpected event while we think we're moving, and set |window_is_moving_| to false.

I've also moved the setting of |window_is_moving_| to false at the start of UnregisterEventHooks
in the unlikely event that UnhookWinEvent is causing the event hook callback to get called
reentrantly. Some crash reports show UnregisterEventHooks on the stack, but looking at the
same crash dump in VS doesn't show UnregisterEventHooks.

Bug: 907365
Change-Id: Iec41dbbe656d0acea0c161b45ec5b7812eb32233
Reviewed-on: https://chromium-review.googlesource.com/c/1348829Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611240}
parent 79c00421
...@@ -417,6 +417,7 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator:: ...@@ -417,6 +417,7 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator:: void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
UnregisterEventHooks() { UnregisterEventHooks() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
window_is_moving_ = false;
for (HWINEVENTHOOK event_hook : global_event_hooks_) for (HWINEVENTHOOK event_hook : global_event_hooks_)
UnhookWinEvent(event_hook); UnhookWinEvent(event_hook);
global_event_hooks_.clear(); global_event_hooks_.clear();
...@@ -426,7 +427,6 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator:: ...@@ -426,7 +427,6 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
process_event_hooks_.clear(); process_event_hooks_.clear();
pids_for_location_change_hook_.clear(); pids_for_location_change_hook_.clear();
window_is_moving_ = false;
} }
bool NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator:: bool NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
...@@ -500,14 +500,18 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator:: ...@@ -500,14 +500,18 @@ void NativeWindowOcclusionTrackerWin::WindowOcclusionCalculator::
// Don't continually calculate occlusion while a window is moving, but rather // Don't continually calculate occlusion while a window is moving, but rather
// once at the beginning and once at the end. // once at the beginning and once at the end.
if (event == EVENT_SYSTEM_MOVESIZESTART) { if (event == EVENT_SYSTEM_MOVESIZESTART) {
// TODO(davidbienvenu): convert to DCHECK once we've confirmed in canary
// that this condition isn't met.
CHECK(!window_is_moving_);
window_is_moving_ = true; window_is_moving_ = true;
} else if (event == EVENT_SYSTEM_MOVESIZEEND) { } else if (event == EVENT_SYSTEM_MOVESIZEEND) {
window_is_moving_ = false; window_is_moving_ = false;
} else if (window_is_moving_) { } else if (window_is_moving_) {
return; if (event == EVENT_OBJECT_LOCATIONCHANGE ||
event == EVENT_OBJECT_STATECHANGE) {
return;
}
// If we get an event that isn't a location/state change, then we probably
// missed the movesizeend notification, or got events out of order. In
// that case, we want to go back to calculating occlusion.
window_is_moving_ = false;
} }
// ProcessEventHookCallback is called from the task_runner's PeekMessage // ProcessEventHookCallback is called from the task_runner's PeekMessage
// call, on the task runner's thread, but before the task_tracker thread sets // call, on the task runner's thread, but before the task_tracker thread sets
......
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