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

ui/views/cocoa/bridged_native_widget.mm

Index: ui/views/cocoa/bridged_native_widget.mm
diff --git a/ui/views/cocoa/bridged_native_widget.mm b/ui/views/cocoa/bridged_native_widget.mm
index 47373d0d4a973470d3ddc37c590af7672d842173..c54bcc32a1023532bf4634a4486ec89890649b62 100644
--- a/ui/views/cocoa/bridged_native_widget.mm
+++ b/ui/views/cocoa/bridged_native_widget.mm
@@ -366,7 +366,10 @@ BridgedNativeWidget::~BridgedNativeWidget() {
     [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);