779430 - "Bad readystate" assertion in DocumentViewerImpl::LoadComplete

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Assertion failure: mDocument->IsXUL() || mDocument->GetReadyStateEnum() == nsIDocument::READYSTATE_INTERACTIVE || (mDocument->GetReadyStateEnum() == nsIDocument::READYSTATE_UNINITIALIZED && NS_IsAboutBlank(mDocument->GetDocumentURI())) (Bad readystate), at layout/base/nsDocumentViewer.cpp:1017


This instance of this assertion was added in:

changeset:   fa6a84e7ba53
user:        Henri Sivonen
date:        Fri Jul 27 16:35:09 2012 +0300
summary:     Bug 775467 - Make readyState progress through all states without duplicate transitions. r=bzbarsky.

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

Does document.documentElement.style.counterReset = "egg"; involve XBL somehow?

Comment 3

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

Note that bug 779100 is a case of bogosity known at the time of landing bug 775467. It appears that our "load" event firing logic is a very broken in interesting edge cases (such as this case here and the case in that networking bug). :-(

Comment 4

[Boris Zbarsky [:bzbarsky]]

There shouldn't be any XBL involved there, no.

And I'm not sure why the new tab part is relevant...

Comment 5

[Bob Clary [:bc] (inactive)]

I see this on 

http://video.qip.ru/live/sexy2012/
http://ziza.qip.ru/category/girls/

for Aurora, Nightly and Windows, Linux, Mac. You may need to load them multiple times to see it.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Happened twice in the last day to me.  I was scrolling through a Google Groups page I think; I honestly don't remember what I was exactly doing other than trying to scroll as the page loaded.

GetReadyStateEnum() is READYSTATE_COMPLETE

IsXUL() is inlined, but: 
(gdb) p ((nsIDocument*) mDocument.mRawPtr)->mIsXUL
$5 = false

I believe I was trying to mousewheel scroll on a document I had just opened, but I'm not 100% sure

I'll attach a GDB log with dumps of the backtrace, document, docshell, etc.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

I *may* have been loading https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.platform/vS877oA_HSA

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Yup, crashed again when leaving that page via "Back" to a bugzilla bug

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Attachment: FIrst gdb log

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Second gdb log

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Attachment: gdb logs

Comment 12

[Jesse Ruderman]

Still happens on trunk:

###!!! ASSERTION: Bad readystate: 'mDocument->IsXUL() || mDocument->GetReadyStateEnum() == nsIDocument::READYSTATE_INTERACTIVE || (mDocument->GetReadyStateEnum() == nsIDocument::READYSTATE_UNINITIALIZED && NS_IsAboutBlank(mDocument->GetDocumentURI()))', file ../../../layout/base/nsDocumentViewer.cpp, line 1029

It was made non-fatal in bug 807442.

Comment 13

[Carsten Book [:Tomcat]]

Attachment: new assertion stack

still reproduced on win 7 debug build based on m-c tip from today

Comment 14

[Carsten Book [:Tomcat]]

smaug: is this something for you ?

Comment 15

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

So nsDocShell::GetRestoringDocument is lying us here. We actually do restore the testcase document as expected, but for some reason DocShell says we're not doing that.
And the issue happens only first time the button is clicked.

Investigating.

Comment 16

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

Attachment: patch

bfcache handling in docshell really doesn't expect layout flushes (which may add onload blockers, like in this case XBL does that) when this code is called at the end of restoration. Adding async onload blockers make bfcache restoration
behave as if normal page was loaded, which means we try to set the readyState again.

I tried to write mochitest for this, but something in the framework affects to the timing so that the assertion
isn't triggered without the patch.

Comment 17

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

(nor with the patch)


https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e01843bdc85

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8708544 [details] [diff] [review]
patch

r=me

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee172b352910

Comment 20

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

One can probably still find ways to trigger the assertion, but please file new bugs for new cases.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ee172b352910