Bug 94220 – REGRESSION (r125592): Crash in Console::addMessage, under InjectedBundle::reportException

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Kevin M. Dean]

Seems to crash relatively quickly just loading pages.

Process:         WebProcess [14057]
Path:            /Applications/WebKit.app/Contents/Frameworks/10.8/WebKit2.framework/WebProcess.app/Contents/MacOS/WebProcess
Identifier:      com.apple.WebProcess
Version:         537+ (537.6+)
Code Type:       X86-64 (Native)
Parent Process:  ??? [1]
User ID:         501

Date/Time:       2012-08-16 02:54:08.334 -0400
OS Version:      Mac OS X 10.8 (12A269)
Report Version:  10

Crashed Thread:  0  Dispatch queue: com.apple.main-thread

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

VM Regions Near 0x8:
--> 
    __TEXT                 0000000104323000-0000000104324000 [    4K] r-x/rwx SM=COW  /Applications/WebKit.app/Contents/Frameworks/10.8/WebKit2.framework/WebProcess.app/Contents/MacOS/WebProcess

Application Specific Information:
Bundle controller class:
BrowserBundleController
 

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x0000000104b381ae WebCore::Console::addMessage(WebCore::MessageSource, WebCore::MessageType, WebCore::MessageLevel, WTF::String const&, WTF::String const&, unsigned int, WTF::PassRefPtr<WebCore::ScriptCallStack>) + 46
1   com.apple.WebCore             	0x0000000104c24a68 WebCore::Document::addMessage(WebCore::MessageSource, WebCore::MessageType, WebCore::MessageLevel, WTF::String const&, WTF::String const&, unsigned int, WTF::PassRefPtr<WebCore::ScriptCallStack>) + 232
2   com.apple.WebCore             	0x00000001054c6849 WebCore::ScriptExecutionContext::addConsoleMessage(WebCore::MessageSource, WebCore::MessageType, WebCore::MessageLevel, WTF::String const&, WTF::String const&, unsigned int, WTF::PassRefPtr<WebCore::ScriptCallStack>) + 57
3   com.apple.WebCore             	0x0000000104c1d2f0 non-virtual thunk to WebCore::Document::logExceptionToConsole(WTF::String const&, WTF::String const&, int, WTF::PassRefPtr<WebCore::ScriptCallStack>) + 64
4   com.apple.WebCore             	0x00000001054c757b WebCore::ScriptExecutionContext::reportException(WTF::String const&, int, WTF::String const&, WTF::PassRefPtr<WebCore::ScriptCallStack>) + 395
5   com.apple.WebCore             	0x000000010501e9e1 WebCore::reportException(JSC::ExecState*, JSC::JSValue) + 1153
6   com.apple.WebKit2             	0x000000010437f566 WebKit::InjectedBundle::reportException(OpaqueJSContext const*, OpaqueJSValue const*) + 68
7   com.apple.Safari.framework    	0x00007fff8a5b3fe4 Safari::EventTarget::handleEvent(Safari::Event*, Safari::RegisteredEventListener const&) + 134
8   com.apple.Safari.framework    	0x00007fff8a5b3eda Safari::EventTarget::fireEventListeners(Safari::Event*, Safari::EventTargetTracker*) + 312
9   com.apple.Safari.framework    	0x00007fff8a5b3aff Safari::EventTarget::fireEventListeners(Safari::Event*) + 173
10  com.apple.Safari.framework    	0x00007fff8a5b3a0e Safari::EventTarget::dispatchEvent(Safari::Event*) + 370
11  com.apple.Safari.framework    	0x00007fff8a57c736 Safari::ContentExtension::dispatchMessageToPage(Safari::WK::String const&, Safari::WK::SerializedScriptValue const&, Safari::WK::BundlePage const&) + 128
12  com.apple.Safari.framework    	0x00007fff8a57f70c Safari::ContentExtension::handleMessage(Safari::WK::String const&, Safari::WK::Type const&) + 140
13  com.apple.Safari.framework    	0x00007fff8a4e318a Safari::BrowserBundleController::dispatchMessage(Safari::WK::String const&, Safari::WK::Type const&) + 62
14  com.apple.Safari.framework    	0x00007fff8a4e18ca Safari::BrowserBundleController::didReceiveMessage(Safari::WK::Bundle const&, Safari::WK::String const&, Safari::WK::Type const&) + 40
15  com.apple.Safari.framework    	0x00007fff8a565983 Safari::WK::didReceiveMessage(OpaqueWKBundle const*, OpaqueWKString const*, void const*, void const*) + 91
16  com.apple.WebKit2             	0x0000000104382918 WebKit::InjectedBundleClient::didReceiveMessage(WebKit::InjectedBundle*, WTF::String const&, WebKit::APIObject*) + 134
17  com.apple.WebKit2             	0x000000010437f627 WebKit::InjectedBundle::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 97
18  com.apple.WebKit2             	0x00000001043cfa4f WebKit::WebConnectionToUIProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 179
19  com.apple.WebKit2             	0x000000010436b5d7 CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::ArgumentDecoder>&) + 175
20  com.apple.WebKit2             	0x000000010436cb0b CoreIPC::Connection::dispatchOneMessage() + 139
21  com.apple.WebCore             	0x00000001054b24c8 WebCore::RunLoop::performWork() + 312
22  com.apple.WebCore             	0x00000001054b2b45 WebCore::RunLoop::performWork(void*) + 53
23  com.apple.CoreFoundation      	0x00007fff95029841 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
24  com.apple.CoreFoundation      	0x00007fff9502922d __CFRunLoopDoSources0 + 445
25  com.apple.CoreFoundation      	0x00007fff9504c4e5 __CFRunLoopRun + 789
26  com.apple.CoreFoundation      	0x00007fff9504bdd2 CFRunLoopRunSpecific + 290
27  com.apple.HIToolbox           	0x00007fff8dbf2774 RunCurrentEventLoopInMode + 209
28  com.apple.HIToolbox           	0x00007fff8dbf2512 ReceiveNextEventCommon + 356
29  com.apple.HIToolbox           	0x00007fff8dbf23a3 BlockUntilNextEventMatchingListInMode + 62
30  com.apple.AppKit              	0x00007fff8cd83fa3 _DPSNextEvent + 685
31  com.apple.AppKit              	0x00007fff8cd83862 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128
32  com.apple.AppKit              	0x00007fff8cd7ac03 -[NSApplication run] + 517
33  com.apple.WebCore             	0x00000001054b3123 WebCore::RunLoop::run() + 67
34  com.apple.WebKit2             	0x000000010444f895 WebKit::WebProcessMain(WebKit::CommandLine const&) + 2565
35  com.apple.WebKit2             	0x00000001043fc527 WebKitMain + 271
36  com.apple.WebProcess          	0x0000000104323e7b main + 214
37  libdyld.dylib                 	0x00007fff8ffe57e1 start + 1

[Alexey Proskuryakov]

Could you please find out which extension triggers this?

[Kevin M. Dean]

Franker 1.3.1

https://code.google.com/p/franker/

If I open Webkit with Just Franker active, it'll load the webkit start page. Then if I select my Macworld bookmark it'll mostly load then crash.

[Alexey Proskuryakov]

Perfect, this is immediately reproducible when opening macworld.com with the Franker extension installed.

Adam, my bisecting shows that this started with DOMWindow::document() refactoring in r125592, would you be willing to take a look?

[Adam Barth]

Sure.  How to I install the Franker extension?

[Adam Barth]

I went to https://extensions.apple.com and https://ssl.apple.com/search/?q=fanker&section=global&geo=us but couldn't find anything named Franker.  Do you have a link?

[Adam Barth]

https://code.google.com/p/franker/downloads/list similarly doesn't have an extension listed.

[Adam Barth]

Found it http://franker.googlecode.com/svn/release/Franker.safariextz

[Kevin M. Dean]

There appears to be another extension exploiting the same bug since I'm periodically having crashes when just opening links from the the bug report emails or from this page. Franker is disabled.

[Adam Barth]

It's very likely that the null-check on line 165 of JSDOMBinding.cpp should instead check whether the DOMWindow isCurrentlyDisplayedInFrame.  /me is verifying.

[Kevin M. Dean]

Ok,  the other extension and different test is:

Maximieren 0.95
http://www.apfelquak.de/2010/06/12/maximize-safari-extension/

With this extension active, I open Webkit close the window and then go to my bug report email and click the link to open the bug page. The window starts to appear transparent and zooming to size but crashes before it completes.

[Adam Barth]

Created attachment 158886 [details]
fixes the problem

[Adam Barth]

Created attachment 158887 [details]
fixes the problem

[Adam Barth]

What's the recommended way to test bugs that only reproduce because of Safari's extension system?

[Adam Barth]

By the way, I believe the remaining null check on the ScriptExecutionContext isn't needed.  I'm just slightly hesitant to change it since we don't appear to have much test coverage of this function.

[Alexey Proskuryakov]

Comment on attachment 158887 [details]
fixes the problem

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

So, extensions run scripts in frameless windows? I still don't know a lot about how extensions work.

I wonder if we should add a special mode for tests then, where scripts would not be blocked in frameless windows, as they normally are.

> Source/WebCore/bindings/js/JSDOMBinding.cpp:167
>      // scriptExecutionContext can be null when the relevant global object is a stale inner window object.
>      // It's harmless to return here without reporting the exception to the log and the debugger in this case.

Is this still true? As we're starting with activeDOMWindow, it sounds like it can not be stale.

[Alexey Proskuryakov]

> Is this still true? As we're starting with activeDOMWindow, it sounds like it can not be stale.

I now see that you said the same.

[Adam Barth]

> So, extensions run scripts in frameless windows? I still don't know a lot about how extensions work.

There shouldn't be any behavior change with after this patch.  The console messages for inactive frames were getting dropped on the floor before this patch and will continue to be dropped after this patch.

In more detail, DOMWindow->scriptExecutionContext used to return 0 when the DOMWindow wasn't being displayed in a Frame.  r125592 changed things so that it returns a real scriptExecutionContext in these cases.  Now the way you check whether the DOMWindow is being displayed in a Frame is by calling DOMWindow->isCurrentlyDisplayedInFrame.

> I wonder if we should add a special mode for tests then, where scripts would not be blocked in frameless windows, as they normally are.

I think there's some confusion here.  Nothing has changed about whether or not scripts can run in frameless windows.

> > Source/WebCore/bindings/js/JSDOMBinding.cpp:167
> >      // scriptExecutionContext can be null when the relevant global object is a stale inner window object.
> >      // It's harmless to return here without reporting the exception to the log and the debugger in this case.
> 
> Is this still true? As we're starting with activeDOMWindow, it sounds like it can not be stale.

"stale" isn't a useful term to use in this discussion.  The script that is running is associated with a particular DOMWindow.  That's the DOMWindow that is currently active (i.e., running script right now).  There's a separate question of whether that DOMWindow is currently displayed in a Frame.  It's possible for DOMWindows that are not displayed in Frames to execute script.  It's just not possible for them to add messages to the Console.

[Alexey Proskuryakov]

> I think there's some confusion here.  Nothing has changed about whether or not scripts can run in frameless windows.

That's understood.

> It's possible for DOMWindows that are not displayed in Frames to execute script.

This is what I'm confused about. When is this possible? Looking at this patch, I thought that extensions were creating a frameless window of their own, and running scripts there. What are the cases when DOMWindows that are not displayed in Frames execute scripts?

[Adam Barth]

> This is what I'm confused about. When is this possible? Looking at this patch, I thought that extensions were creating a frameless window of their own, and running scripts there. What are the cases when DOMWindows that are not displayed in Frames execute scripts?

The easiest way is to have an iframe that defines a function.  The parent page takes a reference to that function and then destroys the iframe.  Afterwards, the parent page can still call the function from the destroyed iframe.

Another way (and this one is fairly common to do by accident) is to be executing some script that causes the iframe you're in to be destroyed (e.g., by mutating the DOM of your parent frame or by triggering a synchronous load of about:blank).  After your frame is destroyed, your script continues to execute.

It's hard for me to tell what's happening in this case because Safari.framework is just calling reportException directly (i.e., there isn't any JavaScript on the stack above WebCore::reportException).

[Adam Barth]

Created attachment 159034 [details]
Patch

[WebKit Review Bot]

Comment on attachment 159034 [details]
Patch

Clearing flags on attachment: 159034

Committed r125912: <http://trac.webkit.org/changeset/125912>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.