Bug 77686 – [Chromium] WebCore::toV8Context crashes if DomWindow::frame() returns null

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Dmitry Lomov]

http://crbug.com/112110

[Dmitry Lomov]

The break is introduced in http://trac.webkit.org/changeset/99311

[Dmitry Lomov]

Created attachment 125195 [details]
Fix

[Nate Chapin]

Interesting. In what case is this happening?

[Adam Barth]

Comment on attachment 125195 [details]
Fix

DOMWindow::frame() is null after the Frame has been destroyed.

[WebKit Review Bot]

Comment on attachment 125195 [details]
Fix

Attachment 125195 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11394945

[Dmitry Lomov]

Created attachment 125203 [details]
Typo fixed.

[Adam Barth]

Comment on attachment 125203 [details]
Typo fixed.

Can we write a test using the test plugin?

[Nate Chapin]

Comment on attachment 125203 [details]
Typo fixed.

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

> Source/WebCore/bindings/v8/V8Helpers.cpp:45
> +    if (!domWindow || !domWindow->frame() || domWindow != domWindow->frame()->domWindow())

Do we have any idea when this happens? In theory, we shouldn't find ourselves in this state.

Is there any hope of a test?

[Dmitry Lomov]

(In reply to comment #8)
> (From update of attachment 125203 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=125203&action=review
> 
> > Source/WebCore/bindings/v8/V8Helpers.cpp:45
> > +    if (!domWindow || !domWindow->frame() || domWindow != domWindow->frame()->domWindow())
> 
> Do we have any idea when this happens? In theory, we shouldn't find ourselves in this state.

Why? What guarantees it?

> 
> Is there any hope of a test?

[Dmitry Lomov]

(In reply to comment #7)
> (From update of attachment 125203 [details])
> Can we write a test using the test plugin?

Maybe - do we have any examples? Thanks

[Adam Barth]

The DOMWindow's frame pointer is null after the Frame associated with the DOMWindow has been destroyed (typically by being removed from its document).  I'm not sure how this function can get called if the frame no longer exists.

[Adam Barth]

Take a look in LayoutTests/plugins for a bunch of tests that use the test plugin.

[Dmitry Lomov]

(In reply to comment #11)
> The DOMWindow's frame pointer is null after the Frame associated with the DOMWindow has been destroyed (typically by being removed from its document).  I'm not sure how this function can get called if the frame no longer exists.

Looks like NPObject is still alive and receives messages: 
Here is the call stack:
0012f26c 0207b583 chrome_1c30000!WebCore::Frame::domWindow+0x3
0012f278 0207b7f4 chrome_1c30000!WebCore::toV8Context+0x15
0012f2c4 02ccdfc2 chrome_1c30000!_NPN_GetProperty+0x3a
0012f2f0 02ccf1cd chrome_1c30000!NPObjectStub::OnGetProperty+0x6c
0012f3f4 02ccf73b chrome_1c30000!IPC::SyncMessageSchema<Tuple1<NPIdentifier_Param>,Tuple2<NPVariant_Param &,bool &> >::DispatchWithSendParams<NPObjectStub,NPObjectStub,void (__thiscall NPObjectStub::*)(NPIdentifier_Param const &,NPVariant_Param *,bool *)>+0x87
0012f42c 02ccfb87 chrome_1c30000!NPObjectMsg_GetProperty::Dispatch<NPObjectStub,NPObjectStub,void (__thiscall NPObjectStub::*)(NPIdentifier_Param const &,NPVariant_Param *,bool *)>+0x45
0012f5ac 01f2c969 chrome_1c30000!NPObjectStub::OnMessageReceived+0x1f1
0012f5bc 02cc8ddb chrome_1c30000!MessageRouter::RouteMessage+0x30
0012f5d4 01c75c80 chrome_1c30000!NPChannelBase::OnMessageReceived+0xd2
0012f5e0 01c6eae6 chrome_1c30000!base::internal::Invoker<2,base::internal::BindState<base::internal::RunnableAdapter<void (__thiscall media::FFmpegAudioDecoder::*)(base::Callback<void __cdecl(scoped_refptr<media::Buffer>)> const &)>,void __cdecl(media::FFmpegAudioDecoder *,base::Callback<void __cdecl(scoped_refptr<media::Buffer>)> const &),void __cdecl(media::FFmpegAudioDecoder *,base::Callback<void __cdecl(scoped_refptr<media::Buffer>)>)>,void __cdecl(media::FFmpegAudioDecoder *,base::Callback<void __cdecl(scoped_refptr<media::Buffer>)> const &)>::Run+0x16
0012f6b8 01c6e7a1 chrome_1c30000!MessageLoop::RunTask+0x203
0012f708 01c75b70 chrome_1c30000!MessageLoop::DoWork+0x22c
0012f7cc 01c6e47c chrome_1c30000!base::MessagePumpDefault::Run+0x122

[Adam Barth]

That looks like a message from the plugin to the page.  Maybe this is a shutdown race whereby the plugin is getting torn down but it doesn't know it yet.

[Adam Barth]

That seems consistent with this crash being caused by <http://trac.webkit.org/changeset/99311>.  That might make it hard to test in DRT, which is single process.

[Adam Barth]

Comment on attachment 125203 [details]
Typo fixed.

Ok.  I don't think this is testable in DumpRenderTree.  We likely need a separate plug-in process around to test this shutdown race.

Jochen is working on getting the layout tests running in content_shell, which will provide a nice framework for testing these kinds of bugs.

[WebKit Review Bot]

Comment on attachment 125203 [details]
Typo fixed.

Clearing flags on attachment: 125203

Committed r106722: <http://trac.webkit.org/changeset/106722>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.