Closed Bug 515610 Opened 12 years ago Closed 12 years ago

[HTML5][Patch] Make defer and async work with the speculating HTML5 parser

Categories

(Core :: DOM: HTML Parser, defect, P1)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: hsivonen, Assigned: hsivonen)

References

Details

Attachments

(1 file)

When a script has the defer attribute (or async if async support is added to Gecko) and the src attribute set, the script end shouldn't cause the creation of a new speculative future.
Can you elaborate on what you mean by "speculative future"?
I mean creating a speculatively parsed tree operation queue that may or may not get executed depending on how the main thread document.writes().

In the defer and async cases the main thread doesn't have an opportunity to document.write(), so it's pointless to prepare for it.
Depends on: 527896
Morphing this into a more general defer and async brokenness fix bug, because both features are broken and it doesn't make sense to fix them independently of the issue of avoiding speculations for them.
Summary: [HTML5] defer (and async) should not create a speculative future → [HTML5] Make defer and async work with the speculating HTML5 parser
Some notes:

 * It's not necessary to flush the tree ops from off the main thread immediately upon seeing the async script, because speculative loading will take care of starting the load ASAP anyway.

 * When the tree op for an async script is performed, the script will have to wait until the end of the flush until it gets a chance to run. If this turns out to be a real performance problem, let's deal with it then instead of prematurely optimizing now.

 * With this patch the tests from bug 503481 and bug 518104 pass (hence, no tests in this patch).

 * Bug 527896 takes care of the script loader and the parser having the same idea about which scripts scripts are defer/async.

 * It's not possible to nest HTML scripts, so the defer/async status can be a simple flag instead of being a stack. However, it is possible to nest SVG scripts or to nest an HTML script inside an SVG script, but SVG scripts don't support async/defer.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Attachment #412227 - Flags: review?(bnewman)
Comment on attachment 412227 [details] [diff] [review]
Make async and defer special

Makes sens to me.

You ought to be able to assert here instead of testing the namespace again, I think:

diff --git a/parser/html/nsHtml5TreeBuilderCppSupplement.h b/parser/html/nsHtml5TreeBuilderCppSupplement.h
--- a/parser/html/nsHtml5TreeBuilderCppSupplement.h
+++ b/parser/html/nsHtml5TreeBuilderCppSupplement.h
@@ -463,9 +463,10 @@ nsHtml5TreeBuilder::elementPopped(PRInt3
     return;
   }
   // we now have only SVG and HTML
   if (aName == nsHtml5Atoms::script) {
-    if (mCurrentHtmlScriptIsAsyncOrDefer && aNamespace == kNameSpaceID_XHTML) {
+    if (mCurrentHtmlScriptIsAsyncOrDefer) {
+      NS_ASSERTION(aNamespace == kNameSpaceID_XHTML, "Only HTML scripts may be async/defer.");
       nsHtml5TreeOperation* treeOp = mOpQueue.AppendElement();
       NS_ASSERTION(treeOp, "Tree op allocation failed.");
       treeOp->Init(eTreeOpRunScriptAsyncDefer, aElement);      
       mCurrentHtmlScriptIsAsyncOrDefer = PR_FALSE;
Attachment #412227 - Flags: review?(bnewman) → review+
(In reply to comment #5)
> (From update of attachment 412227 [details] [diff] [review])
> Makes sens to me.

Thanks.

> You ought to be able to assert here instead of testing the namespace again, I
> think:

Good point. Changed accordingly.
Priority: -- → P1
Summary: [HTML5] Make defer and async work with the speculating HTML5 parser → [HTML5][Patch] Make defer and async work with the speculating HTML5 parser
http://hg.mozilla.org/mozilla-central/rev/cec37057986c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
A piece of the patch didn't get landed due to a bad merge. Landed now:
http://hg.mozilla.org/mozilla-central/rev/a732c6d3c078
You need to log in before you can comment on or make changes to this bug.