553924 - Crash [@ nsDocumentViewer.cpp][@ DocumentViewerImpl::LoadComplete]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

window is null when DispatchSyncPopState is called.

#0  0x000000327b2a4d1d in nanosleep () from /lib64/libc.so.6
#1  0x000000327b2a4b90 in sleep () from /lib64/libc.so.6
#2  0x00007f27f37c494e in ah_crap_handler (signum=11) at /home/smaug/mozilla/mozilla_cvs/hg/e10s/toolkit/xre/nsSigHandlers.cpp:164
#3  <signal handler called>
#4  DocumentViewerImpl::LoadComplete (this=0x7f27ec3c5320, aStatus=<value optimized out>) at /home/smaug/mozilla/mozilla_cvs/hg/e10s/layout/base/nsDocumentViewer.cpp:1090
#5  0x00007f27f406c66b in nsDocShell::EndPageLoad (this=0x7f27ec2344e0, aProgress=<value optimized out>, aChannel=0x7f27ec3be310, aStatus=0)
    at /home/smaug/mozilla/mozilla_cvs/hg/e10s/docshell/base/nsDocShell.cpp:5746
#6  0x00007f27f4060990 in nsDocShell::OnStateChange (this=0x7f27ec2344e0, aProgress=0x7f27ec234508, aRequest=0x7f27ec3be310, aStateFlags=<value optimized out>, 
    aStatus=<value optimized out>) at /home/smaug/mozilla/mozilla_cvs/hg/e10s/docshell/base/nsDocShell.cpp:5624
#7  0x00007f27f407bb77 in nsDocLoader::FireOnStateChange (this=0x7f27ec2344e0, aProgress=0x7f27ec234508, aRequest=0x7f27ec3be310, aStateFlags=131088, aStatus=<value optimized out>)
    at /home/smaug/mozilla/mozilla_cvs/hg/e10s/uriloader/base/nsDocLoader.cpp:1314
#8  0x00007f27f407bd03 in nsDocLoader::doStopDocumentLoad (this=0x7f27ec2344e0, request=0x7f27ec3be310, aStatus=0)
    at /home/smaug/mozilla/mozilla_cvs/hg/e10s/uriloader/base/nsDocLoader.cpp:926
#9  0x00007f27f407c09d in nsDocLoader::DocLoaderIsEmpty (this=0x7f27ec2344e0, aFlushLayout=<value optimized out>)
    at /home/smaug/mozilla/mozilla_cvs/hg/e10s/uriloader/base/nsDocLoader.cpp:802
#10 0x00007f27f407c628 in nsDocLoader::OnStopRequest (this=0x7f27ec2344e0, aRequest=0x7f27ec55dc30, aCtxt=<value optimized out>, aStatus=0)
    at /home/smaug/mozilla/mozilla_cvs/hg/e10s/uriloader/base/nsDocLoader.cpp:697
#11 0x00007f27f387807d in nsLoadGroup::RemoveRequest (this=0x7f27ec234fc0, request=0x7f27ec55dc30, ctxt=0x0, aStatus=0)
    at /home/smaug/mozilla/mozilla_cvs/hg/e10s/netwerk/base/src/nsLoadGroup.cpp:680
#12 0x00007f27f39abb14 in imgRequestProxy::RemoveFromLoadGroup (this=0x7f27ec55dc30, releaseLoadGroup=1)
    at /home/smaug/mozilla/mozilla_cvs/hg/e10s/modules/libpr0n/src/imgRequestProxy.cpp:194

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Other if()s in the method have the null check for window.

I don't have a testcase for this atm.

Comment 2

[Justin Lebar (not reading bugmail)]

Yeah, it definitely looks like we can't assume that we have a window there.  Can we fix it without a testcase?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

It is just a simple null check. Doesn't change functionality, only prevents a crash. So IMO, this could be pushed without a testcase.

But I'll try to come up with a testcase.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Justin, could you review this, so that we don't forget this.

I know, testcase would be great, but I don't have it yet.

Comment 5

[Justin Lebar (not reading bugmail)]

> Justin, could you review this, so that we don't forget this.
Yes, no problem.  I wasn't sure what the protocol was regarding giving something an r+ when we're waiting for a testcase.

The change looks fine to me.  Maybe in lieu of a testcase right now, we could add a comment to the function indicating that we shouldn't assume that window is non-null?  The perponderance of nullchecks in that function apparently isn't warning enough.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/e6d08c87089e
I pushed the patch as is.