Bug 527896 - Freeze script deferredness and asyncness upon insertion to tree by HTML5 parser. r&sr=sicking.

--HG--
extra : rebase_source : a070df9af738e1716929cecf6163f82b74ab4e1d

content/base/public/nsIScriptElement.h

@@ -47,8 +47,8 @@
 #include "nsIParser.h"
 
 #define NS_ISCRIPTELEMENT_IID \
-{ 0xa28c198e, 0x14f0, 0x42b1, \
-{ 0x8f, 0x6b, 0x0e, 0x7f, 0xca, 0xb4, 0xf4, 0xe8 } }
+{ 0xa9d5732a, 0x8c1f, 0x439d, \
+{ 0x83, 0x75, 0x3d, 0xf6, 0xa9, 0xba, 0xa3, 0x7d } }
 
 /**
  * Internal interface implemented by script elements
@@ -62,6 +62,9 @@ public:
       mIsEvaluated(PR_FALSE),
       mMalformed(PR_FALSE),
       mDoneAddingChildren(PR_TRUE),
+      mFrozen(PR_FALSE),
+      mDefer(PR_FALSE),
+      mAsync(PR_FALSE),
       mCreatorParser(nsnull)
   {
   }
@@ -76,7 +79,11 @@ public:
    * Location of script source text. Can return null, in which case
    * this is assumed to be an inline script element.
    */
-  virtual already_AddRefed<nsIURI> GetScriptURI() = 0;
+  nsIURI* GetScriptURI()
+  {
+    NS_PRECONDITION(mFrozen, "Not ready for this call yet!");
+    return mUri;
+  }
   
   /**
    * Script source text for inline script elements.
@@ -85,15 +92,30 @@ public:
 
   virtual void GetScriptCharset(nsAString& charset) = 0;
 
+  /**
+   * Freezes the return values of GetScriptDeferred(), GetScriptAsync() and
+   * GetScriptURI() so that subsequent modifications to the attributes don't
+   * change execution behavior.
+   */
+  virtual void FreezeUriAsyncDefer() = 0;
+
   /**
    * Is the script deferred. Currently only supported by HTML scripts.
    */
-  virtual PRBool GetScriptDeferred() = 0;
+  PRBool GetScriptDeferred()
+  {
+    NS_PRECONDITION(mFrozen, "Not ready for this call yet!");
+    return mDefer;
+  }
 
   /**
    * Is the script async. Currently only supported by HTML scripts.
    */
-  virtual PRBool GetScriptAsync() = 0;
+  PRBool GetScriptAsync()
+  {
+    NS_PRECONDITION(mFrozen, "Not ready for this call yet!");
+    return mAsync;  
+  }
 
   void SetScriptLineNumber(PRUint32 aLineNumber)
   {
@@ -134,8 +156,6 @@ public:
    */
   void BeginEvaluating()
   {
-    // Once the async attribute is supported, don't do this if this is an
-    // async script.
     nsCOMPtr<nsIParser> parser = do_QueryReferent(mCreatorParser);
     if (parser) {
       parser->BeginEvaluatingParserInsertedScript();
@@ -147,8 +167,6 @@ public:
    */
   void EndEvaluating()
   {
-    // Once the async attribute is supported, don't do this if this is an
-    // async script.
     nsCOMPtr<nsIParser> parser = do_QueryReferent(mCreatorParser);
     if (parser) {
       parser->EndEvaluatingParserInsertedScript();
@@ -165,11 +183,50 @@ public:
   }
 
 protected:
+  /**
+   * The start line number of the script.
+   */
   PRUint32 mLineNumber;
+  
+  /**
+   * The "already started" flag per HTML5.
+   */
   PRPackedBool mIsEvaluated;
+  
+  /**
+   * The script didn't have an end tag.
+   */
   PRPackedBool mMalformed;
+  
+  /**
+   * False if parser-inserted but the parser hasn't triggered running yet.
+   */
   PRPackedBool mDoneAddingChildren;
-  nsWeakPtr    mCreatorParser;
+
+  /**
+   * Whether src, defer and async are frozen.
+   */
+  PRPackedBool mFrozen;
+  
+  /**
+   * The effective deferredness.
+   */
+  PRPackedBool mDefer;
+  
+  /**
+   * The effective asyncness.
+   */
+  PRPackedBool mAsync;
+  
+  /**
+   * The effective src (or null if no src).
+   */
+  nsCOMPtr<nsIURI> mUri;
+  
+  /**
+   * The creator parser of a non-defer, non-async parser-inserted script.
+   */
+  nsWeakPtr mCreatorParser;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIScriptElement, NS_ISCRIPTELEMENT_IID)

content/base/src/nsScriptElement.cpp

@@ -180,6 +180,8 @@ nsScriptElement::MaybeProcessScript()
     return NS_OK;
   }
 
+  FreezeUriAsyncDefer();
+
   if (InNonScriptingContainer(cont)) {
     // Make sure to flag ourselves as evaluated
     mIsEvaluated = PR_TRUE;

content/base/test/Makefile.in

@@ -154,6 +154,7 @@ _TEST_FILES = 	test_bug5141.html \
 		test_bug415860.html \
 		test_bug414190.html \
 		test_bug414796.html \
+		test_bug527896.html \
 		test_bug416317-1.html \
 		test_bug416317-2.html \
 		test_XHRSendData.html \

content/base/test/test_bug527896.html

@@ -0,0 +1,72 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=527896
+-->
+<head>
+  <title>Test for Bug 527896</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body onload='done();'>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=527896">Mozilla Bug 527896</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <iframe></iframe>
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 527896 **/
+
+netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+var prefs = Components.classes["@mozilla.org/preferences-service;1"]
+          .getService(Components.interfaces.nsIPrefBranch);
+var gOriginalHtml5Pref = prefs.getBoolPref("html5.enable");
+prefs.setBoolPref("html5.enable", true);
+
+SimpleTest.waitForExplicitFinish();
+
+var docWrittenSrcExecuted = false;
+var scriptInsertedSrcExecuted = false;
+
+// the iframe test runs with the HTML5 parser
+
+var iframe = document.getElementsByTagName('iframe')[0];
+iframe.contentWindow.document.open();
+iframe.contentWindow.document.write("<!DOCTYPE html>");
+iframe.contentWindow.document.write("<body><script id =\"thescript\" src=\"data:text/javascript,parent.docWrittenSrcExecuted = true;\">");
+
+// now remove the src attribute before the end tag is parsed
+iframe.contentWindow.document.getElementById('thescript').removeAttribute('src');
+
+iframe.contentWindow.document.write("parent.ok(false, \"Content executed.\");");
+iframe.contentWindow.document.write("<\/script>");
+iframe.contentWindow.document.close();
+
+// the insertion test runs with the default HTML parser since it's in this document itself!
+
+var div = document.getElementById('content');
+var script = document.createElement('script');
+div.appendChild(script); // this shouldn't yet freeze the script node nor run it
+script.setAttribute("src", "data:text/javascript,scriptInsertedSrcExecuted = true;");
+
+todo(false, "Add SVG tests after bug 528442.");
+
+function done() {
+  ok(docWrittenSrcExecuted, "document.written src didn't execute");
+  ok(scriptInsertedSrcExecuted, "script-inserted src didn't execute");
+
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+  var prefs = Components.classes["@mozilla.org/preferences-service;1"]
+            .getService(Components.interfaces.nsIPrefBranch);
+  prefs.setBoolPref("html5.enable", gOriginalHtml5Pref);
+
+  SimpleTest.finish();
+}
+
+</script>
+</pre>
+</body>
+</html>

content/html/content/src/nsHTMLScriptElement.cpp

@@ -329,11 +329,9 @@ public:
 
   // nsIScriptElement
   virtual void GetScriptType(nsAString& type);
-  virtual already_AddRefed<nsIURI> GetScriptURI();
   virtual void GetScriptText(nsAString& text);
   virtual void GetScriptCharset(nsAString& charset);
-  virtual PRBool GetScriptDeferred();
-  virtual PRBool GetScriptAsync();
+  virtual void FreezeUriAsyncDefer();
 
   // nsIContent
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
@@ -477,7 +475,14 @@ nsresult
 nsHTMLScriptElement::DoneAddingChildren(PRBool aHaveNotified)
 {
   mDoneAddingChildren = PR_TRUE;
-  return MaybeProcessScript();
+  nsresult rv = MaybeProcessScript();
+  if (!mIsEvaluated) {
+    // Need to thaw the script uri here to allow another script to cause
+    // execution later.
+    mFrozen = PR_FALSE;
+    mUri = nsnull;
+  }
+  return rv;
 }
 
 PRBool
@@ -495,20 +500,6 @@ nsHTMLScriptElement::GetScriptType(nsAString& type)
   GetType(type);
 }
 
-// variation of this code in nsSVGScriptElement - check if changes
-// need to be transfered when modifying
-
-already_AddRefed<nsIURI>
-nsHTMLScriptElement::GetScriptURI()
-{
-  nsIURI *uri = nsnull;
-  nsAutoString src;
-  GetSrc(src);
-  if (!src.IsEmpty())
-    NS_NewURI(&uri, src);
-  return uri;
-}
-
 void
 nsHTMLScriptElement::GetScriptText(nsAString& text)
 {
@@ -521,31 +512,35 @@ nsHTMLScriptElement::GetScriptCharset(nsAString& charset)
   GetCharset(charset);
 }
 
-PRBool
-nsHTMLScriptElement::GetScriptDeferred()
+void
+nsHTMLScriptElement::FreezeUriAsyncDefer()
 {
-  PRBool defer, async;
-  GetAsync(&async);
-  GetDefer(&defer);
-  nsCOMPtr<nsIURI> uri = GetScriptURI();
+  if (mFrozen) {
+    return;
+  }
+  
+  // variation of this code in nsSVGScriptElement - check if changes
+  // need to be transfered when modifying
+  if (HasAttr(kNameSpaceID_None, nsGkAtoms::src)) {
+    nsAutoString src;
+    GetSrc(src);
+    NS_NewURI(getter_AddRefs(mUri), src);
 
-  return !async && defer && uri;
-}
+    PRBool defer, async;
+    GetAsync(&async);
+    GetDefer(&defer);
 
-PRBool
-nsHTMLScriptElement::GetScriptAsync()
-{
-  PRBool async;
-  GetAsync(&async);
-  nsCOMPtr<nsIURI> uri = GetScriptURI();
-
-  return async && uri;
+    mDefer = !async && defer;
+    mAsync = async;
+  }
+  
+  mFrozen = PR_TRUE;
 }
 
 PRBool
 nsHTMLScriptElement::HasScriptContent()
 {
-  return HasAttr(kNameSpaceID_None, nsGkAtoms::src) ||
+  return (mFrozen ? !!mUri : HasAttr(kNameSpaceID_None, nsGkAtoms::src)) ||
          nsContentUtils::HasNonEmptyTextContent(this);
 }
 

content/svg/content/src/nsSVGScriptElement.cpp

@@ -76,12 +76,10 @@ public:
 
   // nsIScriptElement
   virtual void GetScriptType(nsAString& type);
-  virtual already_AddRefed<nsIURI> GetScriptURI();
   virtual void GetScriptText(nsAString& text);
   virtual void GetScriptCharset(nsAString& charset);
-  virtual PRBool GetScriptDeferred();
-  virtual PRBool GetScriptAsync();
-
+  virtual void FreezeUriAsyncDefer();
+  
   // nsScriptElement
   virtual PRBool HasScriptContent();
 
@@ -199,22 +197,6 @@ nsSVGScriptElement::GetScriptType(nsAString& type)
   GetType(type);
 }
 
-// variation of this code in nsHTMLScriptElement - check if changes
-// need to be transfered when modifying
-
-already_AddRefed<nsIURI>
-nsSVGScriptElement::GetScriptURI()
-{
-  nsIURI *uri = nsnull;
-  nsAutoString src;
-  mStringAttributes[HREF].GetAnimValue(src, this);
-  if (!src.IsEmpty()) {
-    nsCOMPtr<nsIURI> baseURI = GetBaseURI();
-    NS_NewURI(&uri, src, nsnull, baseURI);
-  }
-  return uri;
-}
-
 void
 nsSVGScriptElement::GetScriptText(nsAString& text)
 {
@@ -227,16 +209,24 @@ nsSVGScriptElement::GetScriptCharset(nsAString& charset)
   charset.Truncate();
 }
 
-PRBool
-nsSVGScriptElement::GetScriptDeferred()
+void
+nsSVGScriptElement::FreezeUriAsyncDefer()
 {
-  return PR_FALSE;
-}
+  if (mFrozen) {
+    return;
+  }
 
-PRBool
-nsSVGScriptElement::GetScriptAsync()
-{
-  return PR_FALSE;
+  // variation of this code in nsHTMLScriptElement - check if changes
+  // need to be transfered when modifying
+  nsAutoString src;
+  mStringAttributes[HREF].GetAnimValue(src, this);
+  // preserving bug 528444 here due to being unsure how to fix correctly
+  if (!src.IsEmpty()) {
+    nsCOMPtr<nsIURI> baseURI = GetBaseURI();
+    NS_NewURI(getter_AddRefs(mUri), src, nsnull, baseURI);
+  }
+  
+  mFrozen = PR_TRUE;
 }
 
 //----------------------------------------------------------------------
@@ -245,9 +235,10 @@ nsSVGScriptElement::GetScriptAsync()
 PRBool
 nsSVGScriptElement::HasScriptContent()
 {
-  nsAutoString str;
-  mStringAttributes[HREF].GetAnimValue(str, this);
-  return !str.IsEmpty() ||
+  nsAutoString src;
+  mStringAttributes[HREF].GetAnimValue(src, this);
+  // preserving bug 528444 here due to being unsure how to fix correctly
+  return (mFrozen ? !!mUri : !src.IsEmpty()) ||
          nsContentUtils::HasNonEmptyTextContent(this);
 }
 
@@ -278,7 +269,14 @@ nsresult
 nsSVGScriptElement::DoneAddingChildren(PRBool aHaveNotified)
 {
   mDoneAddingChildren = PR_TRUE;
-  return MaybeProcessScript();
+  nsresult rv = MaybeProcessScript();
+  if (!mIsEvaluated) {
+    // Need to thaw the script uri here to allow another script to cause
+    // execution later.
+    mFrozen = PR_FALSE;
+    mUri = nsnull;
+  }
+  return rv;
 }
 
 nsresult

parser/html/nsHtml5TreeBuilderCppSupplement.h

@@ -164,7 +164,7 @@ nsHtml5TreeBuilder::createElement(PRInt32 aNamespace, nsIAtom* aName, nsHtml5Htm
         } else if (nsHtml5Atoms::script == aName) {
           nsHtml5TreeOperation* treeOp = mOpQueue.AppendElement();
           NS_ASSERTION(treeOp, "Tree op allocation failed.");
-          treeOp->Init(eTreeOpSetScriptLineNumber, content, tokenizer->getLineNumber());
+          treeOp->Init(eTreeOpSetScriptLineNumberAndFreeze, content, tokenizer->getLineNumber());
 
           nsString* url = aAttributes->getValue(nsHtml5AttributeName::ATTR_SRC);
           if (url) {
@@ -208,7 +208,7 @@ nsHtml5TreeBuilder::createElement(PRInt32 aNamespace, nsIAtom* aName, nsHtml5Htm
         } else if (nsHtml5Atoms::script == aName) {
           nsHtml5TreeOperation* treeOp = mOpQueue.AppendElement();
           NS_ASSERTION(treeOp, "Tree op allocation failed.");
-          treeOp->Init(eTreeOpSetScriptLineNumber, content, tokenizer->getLineNumber());
+          treeOp->Init(eTreeOpSetScriptLineNumberAndFreeze, content, tokenizer->getLineNumber());
 
           nsString* url = aAttributes->getValue(nsHtml5AttributeName::ATTR_XLINK_HREF);
           if (url) {
@@ -241,7 +241,7 @@ nsHtml5TreeBuilder::createElement(PRInt32 aNamespace, nsIAtom* aName, nsHtml5Htm
     } else if (nsHtml5Atoms::script == aName) {
       nsHtml5TreeOperation* treeOp = mOpQueue.AppendElement();
       NS_ASSERTION(treeOp, "Tree op allocation failed.");
-      treeOp->Init(eTreeOpSetScriptLineNumber, content, tokenizer->getLineNumber());
+      treeOp->Init(eTreeOpSetScriptLineNumberAndFreeze, content, tokenizer->getLineNumber());
     }
   }
 

parser/html/nsHtml5TreeOperation.cpp

@@ -582,11 +582,12 @@ nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor* aBuilder,
       ssle->SetLineNumber(mInt);
       return rv;
     }
-    case eTreeOpSetScriptLineNumber: {
+    case eTreeOpSetScriptLineNumberAndFreeze: {
       nsIContent* node = *(mOne.node);
       nsCOMPtr<nsIScriptElement> sele = do_QueryInterface(node);
       NS_ASSERTION(sele, "Node didn't QI to script.");
       sele->SetScriptLineNumber(mInt);
+      sele->FreezeUriAsyncDefer();
       return rv;
     }
     default: {

parser/html/nsHtml5TreeOperation.h

@@ -78,7 +78,7 @@ enum eHtml5TreeOperation {
   eTreeOpMarkMalformedIfScript,
   eTreeOpStreamEnded,
   eTreeOpSetStyleLineNumber,
-  eTreeOpSetScriptLineNumber,
+  eTreeOpSetScriptLineNumberAndFreeze,
   eTreeOpStartLayout
 };