[WK2] REGRESSION r122966: Crash when closing tab with Web Inspector open in WebKit::PageOverlay

[Kevin M. Dean]

Since July 18th, I received a few crashes when closing a tab with the Web Inspector open in it I believe. I'm not yet sure of the exact circumstance to trigger the crash.

Here's the first one.

Date/Time:       2012-07-18 17:51:52.586 -0400
OS Version:      Mac OS X 10.7.4 (11E53)

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000070

VM Regions Near 0x70:
--> 
    __TEXT                 00000001082a8000-00000001082a9000 [    4K] r-x/rwx SM=COW  /Applications/WebKit.app/Contents/Frameworks/10.7/WebKit2.framework/WebProcess.app/Contents/MacOS/WebProcess

Application Specific Information:
objc[26923]: garbage collection is OFF

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebKit2             	0x00000001084406ac WebKit::PageOverlay::bounds() const + 18
1   com.apple.WebKit2             	0x0000000108440807 WebKit::PageOverlay::setNeedsDisplay() + 17
2   com.apple.WebKit2             	0x00000001084941c8 WebKit::WebInspectorClient::highlight() + 92
3   com.apple.WebCore             	0x0000000108f8ba3f WebCore::InspectorDOMAgent::clearFrontend() + 127
4   com.apple.WebCore             	0x0000000108f78e69 WebCore::InspectorController::disconnectFrontend() + 121
5   com.apple.WebCore             	0x0000000108f78dbe WebCore::InspectorController::inspectedPageDestroyed() + 14
6   com.apple.WebCore             	0x0000000109346d0e WebCore::Page::~Page() + 350
7   com.apple.WebKit2             	0x00000001084a70f8 WTF::OwnPtr<WebCore::Page>::clear() + 36
8   com.apple.WebKit2             	0x00000001084a00db WebKit::WebPage::close() + 297
9   com.apple.WebKit2             	0x000000010846943f WebKit::WebConnectionToUIProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 179
10  com.apple.WebKit2             	0x000000010840cd91 CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::ArgumentDecoder>&) + 175
11  com.apple.WebKit2             	0x000000010840e2ab CoreIPC::Connection::dispatchOneMessage() + 139
12  com.apple.WebCore             	0x00000001094d3f48 WebCore::RunLoop::performWork() + 312
13  com.apple.WebCore             	0x00000001094d45a5 WebCore::RunLoop::performWork(void*) + 53
14  com.apple.CoreFoundation      	0x00007fff8ab6c4f1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
15  com.apple.CoreFoundation      	0x00007fff8ab6bd5d __CFRunLoopDoSources0 + 253
16  com.apple.CoreFoundation      	0x00007fff8ab92b49 __CFRunLoopRun + 905
17  com.apple.CoreFoundation      	0x00007fff8ab92486 CFRunLoopRunSpecific + 230
18  com.apple.HIToolbox           	0x00007fff8a0834d3 RunCurrentEventLoopInMode + 277
19  com.apple.HIToolbox           	0x00007fff8a08a781 ReceiveNextEventCommon + 355
20  com.apple.HIToolbox           	0x00007fff8a08a60e BlockUntilNextEventMatchingListInMode + 62
21  com.apple.AppKit              	0x00007fff92e84e31 _DPSNextEvent + 659
22  com.apple.AppKit              	0x00007fff92e84735 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 135
23  com.apple.AppKit              	0x00007fff92e81071 -[NSApplication run] + 470
24  com.apple.WebCore             	0x00000001094d4b83 WebCore::RunLoop::run() + 67
25  com.apple.WebKit2             	0x00000001084e5c00 WebKit::WebProcessMain(WebKit::CommandLine const&) + 2695
26  com.apple.WebKit2             	0x000000010849889b WebKitMain + 305
27  com.apple.WebProcess          	0x00000001082a8e5e main + 214
28  com.apple.WebProcess          	0x00000001082a8d80 start + 52

[Kevin M. Dean]

Ok, here's how to reproduce.

Open a site. Open the web inspector. I'm using Command-Option-I. Click in the web page portion of the screen so the Inspector portion lightens and becomes inactive. Then click in the Inspector portion again so it darkens and becomes active. Then close the window or tab. Crash.

[Kevin M. Dean]

Clarification - The Web Inspector needs to be opened to the Elements tab when clicking back and forth. The other tabs seem to handle the clicking fine.

[Timothy Hatcher]

More than likely this was caused by: http://trac.webkit.org/changeset/122966

[Andrey Kosyakov]

Created attachment 153665 [details]
Patch

[Pavel Feldman]

Comment on attachment 153665 [details]
Patch

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

> Source/WebCore/inspector/DOMNodeHighlighter.cpp:536
> +        update();

This change will only mask the crash - it will still happen in case there was a highlighted node upon page destruction.

> Source/WebKit2/WebProcess/WebPage/PageOverlay.cpp:70
> +    if (!frameView)

So this seems to be fixing crash, but we should not be getting here from the page destructor. The proper fix would be to mute InspectorOverlay from within InspectorController::inspectedPageDestroyed().

[Pavel Feldman]

Created attachment 153667 [details]
Patch

[Pavel Feldman]

> The proper fix would be to mute InspectorOverlay from within InspectorController::inspectedPageDestroyed().

I take this back - we would still need to report hideHighlight to the client upon destruction. Submitted a proper fix.

[Yury Semikhatsky]

Comment on attachment 153667 [details]
Patch

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

> Source/WebCore/inspector/DOMNodeHighlighter.cpp:567
> +    if ((m_highlightData && (m_highlightData->rect || m_highlightData->node)) || !m_pausedInDebuggerMessage.isNull())

Consider extracting this logic into hasHighlightData() as it is used used in  InspectorOverlay::drawHighlight as well.

[WebKit Review Bot]

Comment on attachment 153667 [details]
Patch

Clearing flags on attachment: 153667

Committed r123328: <http://trac.webkit.org/changeset/123328>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.