MacViews: Explicitly release mouse capture in BridgedNativeWidget::OnWindowWillClose().

ui/views/cocoa/bridged_native_widget.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ui/views/cocoa/bridged_native_widget.h"
 
 #import <objc/runtime.h>
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 348 matching lines @@
     // For other teardown flows (e.g. Widget::WIDGET_OWNS_NATIVE_WIDGET or
     // Widget::CloseNow()) the delegate must first be cleared to avoid AppKit
     // calling back into the bridge. This means OnWindowWillClose() needs to be
     // invoked manually, which is done below.
     // Note that if the window has children it can't be closed until the
     // children are gone, but removing child windows calls into AppKit for the
     // parent window, so the delegate must be cleared first.
     [window_ setDelegate:nil];
     close_window = true;
   }
-
+  // If we have capture, we'll get a OnMouseCaptureLost callback, which can

[tapted, 2016/06/23 02:26:28]

nit: it's generally good to avoid "we". (there's even go/avoidwe). Here "we"
actually refers to specific things so it's better to say what those things are.
E.g. you can say

// Ensure BridgedNativeWidget does not have capture, otherwise
OnMouseCaptureLost() will reference a deleted Widget when called via
~CocoaMouseCapture() upon the destruction of |mouse_capture_|.

[karandeepb, 2016/06/23 07:58:32]

Done.

+  // lead to a use after free crash. Hence explicitly release capture. See
+  // crbug.com/622201.
+  ReleaseCapture();

[tapted, 2016/06/23 02:26:28]

DesktopNativeWidgetAura does it in OnHostClosed, which is closer to our
OnWindowWillClose - is there a reason to be different?

(maybe this is better, but there are lots of codepaths)

In particular, we need to see if Aura invokes Widget::OnMouseCaptureLost during
shutdown-with-capture, and try to be consistent with that. There's a
CaptureLostTrackingWidget in widget_interactive_uitest.cc which can help with
this.

[karandeepb, 2016/06/23 07:58:32]

Yes Widget::OnMouseCaptureLost is invoked for aura during shutdown-with-capture.
However I am not sure if testing that is relevant to this issue. 
Various code-paths are possible depending on Close/CloseNow or whether it is a
DesktopNativeWidgetAura or NativeWidgetAura. For example, for
DesktopNativeWidgetAura, for a normal CloseNow call, the flow is
DesktopNativeWidgetAura::CloseNow() -> DesktopWindowTreeHostX11::CloseNow() ->
DesktopWindowTreeHostX11::ReleaseCapture(). 

Have moved this to OnWindowWillClose. Both should be fine, we just need to
ensure that we release capture before the native_widget_mac_ is destroyed.

[karandeepb, 2016/06/24 05:34:36]

Done. 

   RemoveOrDestroyChildren();
   DCHECK(child_windows_.empty());
   SetFocusManager(nullptr);
   SetRootView(nullptr);
   DestroyCompositor();
 
   if (close_window) {
     OnWindowWillClose();
     [window_ close];
   }
@@ skipping 965 matching lines @@
   [bridged_view_ setMouseDownCanMoveWindow:draggable];
   // AppKit will not update its cache of mouseDownCanMoveWindow unless something
   // changes. Previously we tried adding an NSView and removing it, but for some
   // reason it required reposting the mouse-down event, and didn't always work.
   // Calling the below seems to be an effective solution.
   [window_ setMovableByWindowBackground:NO];
   [window_ setMovableByWindowBackground:YES];
 }
 
 }  // namespace views