Bug 135569 – Always clear ConsoleClient when Page/WindowShell is destroyed

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Joseph Pecoraro]

* SUMMARY
WebCore::Page's set the ConsoleClient to their PageConsole object. It should always clear this pointer whenever the PageConsole is going away. Otherwise we could crash trying to use it.

Thread 0 Crashed:: main  Dispatch queue: com.apple.main-thread
0   ???                           	000000000000000000 0 + 0
1   com.apple.JavaScriptCore      	0x7fff973a9ca2 JSC::ConsoleClient::logWithLevel
2   com.apple.JavaScriptCore      	0x7fff973a8f3e JSC::consoleLogWithLevel
3   ???                           	0x000042ce8dc01114 0 + 73454908870932
4   com.apple.JavaScriptCore      	0x7fff973a5fbe llint_entry
5   com.apple.JavaScriptCore      	0x7fff973a04c1 callToJavaScript

<rdar://problem/17856494>

[Joseph Pecoraro]

Created attachment 235979 [details]
[PATCH] Proposed Fix

I spent a bit of time trying to create a test for this. I have a manual test case, but it has two setTimeouts of 100ms, which I can't seem to reduce by much. Is that worth adding?

[Mark Lam]

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

r=me

[Geoffrey Garen]

Usually, it is the client's responsibility to clear this pointer, usually in the client object's destructor. Would that approach work here?

(I think this patch is fine too, but it would be a bit cleaner for the client to clear its own pointer.)

[Joseph Pecoraro]

(In reply to comment #3)
> Usually, it is the client's responsibility to clear this pointer, usually in the client object's destructor. Would that approach work here?
> 
> (I think this patch is fine too, but it would be a bit cleaner for the client to clear its own pointer.)

This is a little more complicated than that. During the lifetime of a page, it seems there may be the potential for multiple DOMWindow objects / JSGlobalObjects. As these come and go (page navigation) we want to make sure we configure the ConsoleClient in each of these to be the PageConsole.

Currently this happens in WindowShell initialization and clearing in ScriptController. With one other place in ScriptCachedFrameData::restore when a page is created from the page cache. Here we are fixing an overlooked case in destruction (which doesn't go through the normal clearing path).

I agree, this is messy and should be made clearer somehow.

[WebKit Commit Bot]

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

Clearing flags on attachment: 235979

Committed r172006: <http://trac.webkit.org/changeset/172006>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.