New issue
Advanced search Search tips

Issue 622201 link

Starred by 2 users
Status: Fixed
Owner:
karandeepb@chromium.org
Closed: Jun 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

MacViews: Use after free occurs if a BridgedNativeWidget with capture is destroyed.

Project Member Reported by karandeepb@chromium.org, Jun 22 2016 
<b>Version: <Kenneth, what is the frequency?></b>
OS: 53.0.2775.0

What steps will reproduce the problem?
The following unit test leads to a crash. 

TEST_F(WidgetTest, SetWindowCapture) {
  Widget* widget = CreateTopLevelNativeWidget();
  widget->Show();
  widget->native_widget_private()->SetCapture();
  EXPECT_TRUE(widget->HasCapture());
  widget->CloseNow();
}

The crash occurs at the closeNow() call. Here is how the flow progresses.

NativeWidgetMac::CloseNow()->~BridgedNativeWidget()->~CocoaMouseCapture()->~ActiveEventTap()->BridgedNativeWidget::OnMouseCaptureLost. 

This accessess native_widget_mac_, which has already been destroyed via ~BridgedNativeWidget()->BridgedNativeWidget::OnWindowWillClose().

Comment 1 by tapted@chromium.org, Jun 22 2016 

nice find! maybe you've already done this, but it's worth checking out what aura does in this case - there might be handling for this that we can mimic (and there should be a cross-platform test for it). Otherwise BridgedNativeWidget::OnWindowWillClose() probably just needs to call ReleaseCapture()

(and I had a quick look: e.g. void DesktopNativeWidgetAura::OnHostClosed() calls ReleaseCapture() -- that's probably the thing we need, but we should see if there's an existing test for the code in there that we should have failed already!)
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc6e0034dc928d3c10ee923284d0f10781975280

commit dc6e0034dc928d3c10ee923284d0f10781975280
Author: karandeepb <karandeepb@chromium.org>
Date: Tue Jun 28 02:49:10 2016

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

Currently, if a BridgedNativeWidget with mouse capture is destroyed, it gets a
OnMouseCaptureLost callback from its CocoaMouseCapture member instance.
BridgedNativeWidget::OnMouseCaptureLost accesses native_widget_mac_ which has
already been destroyed, leading to a use after crash.

This CL resolves the crash by explicitly releasing mouse capture in
BridgedNativeWidget::OnWindowWillClose(). Also added tests which crash on
MacViews for the current master, to test the various codepaths possible when a
window is closed.

BUG= 622201 

Review-Url: https://codereview.chromium.org/2090003002
Cr-Commit-Position: refs/heads/master@{#402385}

[modify] https://crrev.com/dc6e0034dc928d3c10ee923284d0f10781975280/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/dc6e0034dc928d3c10ee923284d0f10781975280/ui/views/widget/widget_interactive_uitest.cc

Comment 3 by karandeepb@chromium.org, Jun 28 2016

Status: Fixed (was: Started)
►

Sign in to add a comment