Bug 139740 – Web Inspector: [Mac] Occasional Crashes Closing Inspector

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

* SUMMARY
I've been able to hit some crashes when closing Inspector Windows and using Guard Malloc.

* STEPS TO REPRODUCE
1. shell> DYLD_INSERT_LIBRARIES=/usr/lib/libgmalloc.dylib run-safari
2. Open http://bogojoker.com
3. Rapidly open/close inspector windows (e.g. hold down Cmd+Shift+I and Cmd+W occasionally when inspector window is focused)
  => Crash

* NOTES
Two frequent crash points:

(1) In WebInspectorProxy::inspectorWindowFrame using destroyed WebInspectorProxy as the client of the WKView's Page. This was because we were clearing our reference to the WKWebInspectorWKView, but it was actually kept alive by a dispatch in WKView, the WebInspectorProxy was destroyed, but still had a weak reference in the UIClient.

    - (void)_updateWindowAndViewFrames
    {
        ...
        dispatch_async(dispatch_get_main_queue(), ^{
            ...
            _data->_page->windowAndViewFramesChanged(viewFrameInWindowCoordinates, accessibilityPosition);
        });
    }

(2) In WebPageProxy::didReceiveEvent. While handling a keypress, the WebPageProxy is destroyed, and we continue and use bad objects.

[Radar WebKit Bug Importer]

<rdar://problem/19282330>

[Joseph Pecoraro]

Created attachment 243451 [details]
[PATCH] Proposed Fix

[Timothy Hatcher]

Comment on attachment 243451 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=243451&action=review

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:296
> +        WebPageProxy* inspectorPage = toImpl(m_inspectorView.get().pageRef);
> +        inspectorPage->close();

WKView dealloc calls close(), but that assumes we hold the last ref here.

[WebKit Commit Bot]

Comment on attachment 243451 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 243451

Committed r177459: <http://trac.webkit.org/changeset/177459>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

(In reply to comment #3)
> Comment on attachment 243451 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=243451&action=review
> 
> > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:296
> > +        WebPageProxy* inspectorPage = toImpl(m_inspectorView.get().pageRef);
> > +        inspectorPage->close();
> 
> WKView dealloc calls close(), but that assumes we hold the last ref here.

Correct, we were not holding the lat ref, because the WKWebInspectorWKView was being retained by a dispatch_async block. =(