Closed Bug 1221856 Opened 9 years ago Closed 8 years ago

Assertion failure: GetReadyStateEnum() == nsIDocument::READYSTATE_LOADING (Bad readyState), at dom/html/MediaDocument.cpp:205


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

Not set



Tracking Status
firefox49 --- fixed


(Reporter: bc, Assigned: hsivonen)




(Keywords: assertion, reproducible)

Crash Data


(4 files)

Attached file Linux crash report

2. Assertion failure: GetReadyStateEnum() == nsIDocument::READYSTATE_LOADING (Bad readyState), at /builds/slave/m-cen-l64-d-000000000000000000/build/src/dom/html/MediaDocument.cpp:205

Linux, OSX, Windows 7 Debug Beta/43, Aurora/44, Nightly/45

I can reproduced using a saved version of the page on OSX 10.9 with a Nightly. YMMV.

This is a long standing problem. If you wait too long you may lose the ability to reproduce it.
Attached file lodenfrey.tar.bz2
still reproducible with a win 7 trunk debug build based on todays m-c tip

bz: is this something for you ?
Flags: needinfo?(bzbarsky)
No, probably for someone familiar with our mediadocument code.
Flags: needinfo?(bzbarsky)
data:text/html,<iframe src='data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==' onload='setTimeout(function(){;}, 0)'>

is a minimal testcase for this. Looks like the code added in bug 775467 doesn't deal with
Blocks: 775467
Flags: needinfo?(hsivonen)
data:text/html,<iframe src='data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==' onload=';'>

seems to work too, and giving closer to the original stack. But same issue as with previous data-url.
(In reply to Olli Pettay [:smaug] from comment #4)
> data:text/html,<iframe
> src='data:image/gif;base64,
> onload='setTimeout(function(){;}, 0)'>
> is a minimal testcase for this. Looks like the code added in bug 775467
> doesn't deal with

Indeed. Looking at the patch for that bug and thinking what I must have been thinking, it seems to me that SetScriptGlobalObject() was a suitable opportunity in the call sequence of methods of the synthetic document to trigger the transition from loading to interactive. There doesn't seem to be any inherent reason for that transition to be coupled with the global object getting set per se.

And, indeed, it seems that I failed to take into account

smaug, do you have ideas for a non-terrible way of detecting whether we have the case from within SetScriptGlobalObject()? As the comments say, if our readyState code wasn't so bogus to begin with, we could look at readyState itself there instead of asserting on it...
Flags: needinfo?(hsivonen) → needinfo?(bugs)
I'm not too familiar with readyState being bogus, usually assertions like in this bug have shown up to be bugs in other code.
So I wonder if it was ok to just check readyState here?

But another option is to check in nsGlobalWindow::SetNewDocument whether a new inner window is actually created (and not just reusing some old one, or reusing inner window from bfcached thing), and add a flag to SetScriptGlobalObject. Hmm, though, if a document has ever had a scriptglobal, 
mScopeObject should be a WeakPtr to that global. So I guess you could 
do_QueryReferent mScopeObject and compare to the aScriptGlobalObject in ::SetScriptGlobalObject.
If it is the same, we're coming out from bfcache or something. If do_QueryReferent'ed mScopeObject is null and aScriptGlobalObject is not, we're setting the initial global, and if
do_QueryReferent'ed mScopeObject isn't the same as aScriptGlobalObject but both are non-null, 
we should be dealing with case.
Flags: needinfo?(bugs)
Henri, want to write a patch?
Flags: needinfo?(hsivonen)
Attached patch PatchSplinter Review
Seems complicated. Let's just do what the original comment suggested we could do if we trusted our readyState code to be less bogus.
Assignee: nobody → hsivonen
Flags: needinfo?(hsivonen)
Comment on attachment 8743331 [details] [diff] [review]

Took a bit time to remember this stuff, and figure out that the comment isn't just about bfcache, but that with we enter this code with
nsIDocument::READYSTATE_COMPLETE and then in nsHTMLDocument::Open explicitly call SetReadyStateInternal(nsIDocument::READYSTATE_LOADING);

Could you mention case in the comment too.
Attachment #8743331 - Flags: review?(bugs) → review+
Surely we can't a MediaDocument?
(In reply to Henri Sivonen (:hsivonen) from comment #13)
> Surely we can't a MediaDocument?

Of course we can.
That is what my testcase is doing, see comment 5.
And, "class MediaDocument : public nsHTMLDocument"
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.