Commit 3fa60286 authored by Leonard Grey's avatar Leonard Grey Committed by Commit Bot

Mac: Add CHECKs and crash keys to find lifetime issues in CocoaWindowMoveLoop

- Changes a DCHECK to a CHECK to crash if a move loop starts when one is already
in progress.
- Adds a CHECK to ensure the event monitor's handler is running on the main thread.
- Adds crash keys in CocoaWindowMoveLoop's destructor to capture its address and
a stack trace, to test the hypothesis that crashes may be occurring on objects
that have already been freed.

Bug: 876493
Change-Id: Ic36703f08138daff88e46f02e7af3de61acedc4e
Reviewed-on: https://chromium-review.googlesource.com/1205012
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589243}
parent ac94dafa
......@@ -609,7 +609,8 @@ bool BridgedNativeWidgetImpl::HasCapture() {
Widget::MoveLoopResult BridgedNativeWidgetImpl::RunMoveLoop(
const gfx::Vector2d& drag_offset) {
DCHECK(!HasCapture());
DCHECK(!window_move_loop_);
// https://crbug.com/876493
CHECK(!window_move_loop_);
// RunMoveLoop caller is responsible for updating the window to be under the
// mouse, but it does this using possibly outdated coordinate from the mouse
......
......@@ -4,7 +4,10 @@
#include "ui/views/cocoa/cocoa_window_move_loop.h"
#include "base/debug/stack_trace.h"
#include "base/run_loop.h"
#include "base/strings/stringprintf.h"
#include "components/crash/core/common/crash_key.h"
#include "ui/display/screen.h"
#import "ui/gfx/mac/coordinate_conversion.h"
#import "ui/views/cocoa/bridged_native_widget.h"
......@@ -46,6 +49,13 @@ CocoaWindowMoveLoop::CocoaWindowMoveLoop(BridgedNativeWidgetImpl* owner,
weak_factory_(this) {}
CocoaWindowMoveLoop::~CocoaWindowMoveLoop() {
// Record the address and stack to help catch https://crbug.com/876493.
static crash_reporter::CrashKeyString<19> address_key("move_loop_address");
address_key.Set(base::StringPrintf("%p", this));
static crash_reporter::CrashKeyString<1024> stack_key("move_loop_stack");
crash_reporter::SetCrashKeyStringToStackTrace(&stack_key,
base::debug::StackTrace());
// Handle the pathological case, where |this| is destroyed while running.
if (exit_reason_ref_) {
*exit_reason_ref_ = WINDOW_DESTROYED;
......@@ -73,6 +83,10 @@ Widget::MoveLoopResult CocoaWindowMoveLoop::Run() {
// TabDragController.
NSEventMask mask = NSLeftMouseUpMask | NSLeftMouseDraggedMask;
auto handler = ^NSEvent*(NSEvent* event) {
// 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.
CHECK_EQ(CFRunLoopGetMain(), CFRunLoopGetCurrent());
CocoaWindowMoveLoop* strong = [weak_cocoa_window_move_loop weak].get();
if (!strong || !strong->exit_reason_ref_) {
// By this point CocoaWindowMoveLoop was deleted while processing this
......
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