Commit c5e398c0 authored by Leonard Grey's avatar Leonard Grey Committed by Commit Bot

Mac: End move loop if left mouse is not down

We've had longstanding issues with interrupted tab drags not cancelling
the move loop, often causing a crash when the loop is ended belatedly.

A recent repro (issue 1119714) theorizes that a NSLeftMouseUp event is
not reaching the move loop. Another repro case from a while ago (right
clicking the tab while dragging) had a similar proximate cause.

This change causes the move loop to also look at mouse moves while it's
active. If it receives one (and the left mouse button is not down), it
assumes we missed a mouse up and terminates the loop.

(As a side note, with this change, I can no longer repro the behavior
in 119714 where a single full-screen tab drag can flakily unfullscreen
the tab. I'm thinking this behavior itself was due to move loop
shenanigans.)

Bug: 1119714,1030987
Change-Id: Idd4c765e1abfc87099fa6be82ae2e22f8dc9c55e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2368172
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801003}
parent f2954b6a
...@@ -82,7 +82,8 @@ bool CocoaWindowMoveLoop::Run() { ...@@ -82,7 +82,8 @@ bool CocoaWindowMoveLoop::Run() {
// Esc keypress is handled by EscapeTracker, which is installed by // Esc keypress is handled by EscapeTracker, which is installed by
// TabDragController. // TabDragController.
NSEventMask mask = NSLeftMouseUpMask | NSLeftMouseDraggedMask; NSEventMask mask =
NSLeftMouseUpMask | NSLeftMouseDraggedMask | NSMouseMovedMask;
auto handler = ^NSEvent*(NSEvent* event) { auto handler = ^NSEvent*(NSEvent* event) {
// The docs say this always runs on the main thread, but if it didn't, // The docs say this always runs on the main thread, but if it didn't,
// it would explain https://crbug.com/876493, so let's make sure. // it would explain https://crbug.com/876493, so let's make sure.
...@@ -109,9 +110,15 @@ bool CocoaWindowMoveLoop::Run() { ...@@ -109,9 +110,15 @@ bool CocoaWindowMoveLoop::Run() {
return event; return event;
} }
DCHECK_EQ([event type], NSLeftMouseUp); // In theory, we shouldn't see any kind of NSMouseMoved, but if we see one
*strong->exit_reason_ref_ = MOUSE_UP; // and the left button isn't pressed, we know for a fact that we missed a
std::move(strong->quit_closure_).Run(); // NSLeftMouseUp.
BOOL unexpectedMove = [event type] == NSMouseMoved &&
([NSEvent pressedMouseButtons] & 1) != 1;
if (unexpectedMove || [event type] == NSLeftMouseUp) {
*strong->exit_reason_ref_ = MOUSE_UP;
std::move(strong->quit_closure_).Run();
}
return event; // Process the MouseUp. return event; // Process the MouseUp.
}; };
id monitor = [NSEvent addLocalMonitorForEventsMatchingMask:mask id monitor = [NSEvent addLocalMonitorForEventsMatchingMask:mask
......
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