Bug 1053321 - Correctly pass info about 'defer' and 'async' attributes to HTML5 parser invoked script preload. r=bkelly, a=lizzard
☠☠ backed out by e47ca72b39cf ☠ ☠
authorHonza Bambas <honzab.moz@firemni.cz>
Tue, 01 Aug 2017 21:21:09 +0200
changeset 423449 4e42008873358d82bec211fb12a8e29596bf4317
parent 423448 0aed982018af1ee322be7d30e723732c34a379b7
child 423450 e648c321f8d348dc6dc7acddce8b10a257aab8c4
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly, lizzard
bugs1053321
milestone56.0
Bug 1053321 - Correctly pass info about 'defer' and 'async' attributes to HTML5 parser invoked script preload. r=bkelly, a=lizzard
dom/script/ScriptLoadRequest.cpp
dom/script/ScriptLoadRequest.h
dom/script/ScriptLoader.cpp
dom/script/ScriptLoader.h
dom/tests/mochitest/script/file_blocked_script.sjs
dom/tests/mochitest/script/mochitest.ini
dom/tests/mochitest/script/test_bug1053321.html
dom/tests/moz.build
parser/html/nsHtml5SpeculativeLoad.cpp
parser/html/nsHtml5SpeculativeLoad.h
parser/html/nsHtml5TreeBuilderCppSupplement.h
parser/html/nsHtml5TreeOpExecutor.cpp
parser/html/nsHtml5TreeOpExecutor.h
--- a/dom/script/ScriptLoadRequest.cpp
+++ b/dom/script/ScriptLoadRequest.cpp
@@ -47,16 +47,18 @@ ScriptLoadRequest::ScriptLoadRequest(Scr
   , mElement(aElement)
   , mScriptFromHead(false)
   , mProgress(Progress::Loading)
   , mDataType(DataType::Unknown)
   , mIsInline(true)
   , mHasSourceMapURL(false)
   , mIsDefer(false)
   , mIsAsync(false)
+  , mPreloadAsAsync(false)
+  , mPreloadAsDefer(false)
   , mIsNonAsyncScriptInserted(false)
   , mIsXSLT(false)
   , mIsCanceled(false)
   , mWasCompiledOMT(false)
   , mIsTracking(false)
   , mOffThreadToken(nullptr)
   , mScriptText()
   , mScriptBytecode()
--- a/dom/script/ScriptLoadRequest.h
+++ b/dom/script/ScriptLoadRequest.h
@@ -150,16 +150,18 @@ public:
   nsCOMPtr<nsIScriptElement> mElement;
   bool mScriptFromHead;   // Synchronous head script block loading of other non js/css content.
   Progress mProgress;     // Are we still waiting for a load to complete?
   DataType mDataType;     // Does this contain Source or Bytecode?
   bool mIsInline;         // Is the script inline or loaded?
   bool mHasSourceMapURL;  // Does the HTTP header have a source map url?
   bool mIsDefer;          // True if we live in mDeferRequests.
   bool mIsAsync;          // True if we live in mLoadingAsyncRequests or mLoadedAsyncRequests.
+  bool mPreloadAsAsync;   // True if this is a preload request and the script is async
+  bool mPreloadAsDefer;   // True if this is a preload request and the script is defer
   bool mIsNonAsyncScriptInserted; // True if we live in mNonAsyncExternalScriptInsertedRequests
   bool mIsXSLT;           // True if we live in mXSLTRequests.
   bool mIsCanceled;       // True if we have been explicitly canceled.
   bool mWasCompiledOMT;   // True if the script has been compiled off main thread.
   bool mIsTracking;       // True if the script comes from a source on our tracking protection list.
   void* mOffThreadToken;  // Off-thread parsing token.
   nsString mSourceMapURL; // Holds source map url for loaded scripts
 
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -967,25 +967,26 @@ ScriptLoader::StartLoad(ScriptLoadReques
       // does not exist, such that we can later save the bytecode with a
       // different alternative data type.
       LOG(("ScriptLoadRequest (%p): Request saving bytecode later", aRequest));
       cic->PreferAlternativeDataType(kNullMimeType);
     }
   }
 
   nsIScriptElement* script = aRequest->mElement;
+  bool async = script ? script->GetScriptAsync() : aRequest->mPreloadAsAsync;
+  bool defer = script ? script->GetScriptDeferred() : aRequest->mPreloadAsDefer;
+
   nsCOMPtr<nsIClassOfService> cos(do_QueryInterface(channel));
-
   if (cos) {
-    if (aRequest->mScriptFromHead &&
-        !(script && (script->GetScriptAsync() || script->GetScriptDeferred()))) {
+    if (aRequest->mScriptFromHead && !async && !defer) {
       // synchronous head scripts block loading of most other non js/css
       // content such as images
       cos->AddClassFlags(nsIClassOfService::Leader);
-    } else if (!(script && script->GetScriptDeferred())) {
+    } else if (!defer) {
       // other scripts are neither blocked nor prioritized unless marked deferred
       cos->AddClassFlags(nsIClassOfService::Unblocked);
     }
   }
 
   nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(channel));
   if (httpChannel) {
     // HTTP content negotation has little value in this context.
@@ -2920,17 +2921,17 @@ ScriptLoader::ParsingComplete(bool aTerm
   ProcessPendingRequests();
 }
 
 void
 ScriptLoader::PreloadURI(nsIURI* aURI, const nsAString& aCharset,
                          const nsAString& aType,
                          const nsAString& aCrossOrigin,
                          const nsAString& aIntegrity,
-                         bool aScriptFromHead,
+                         bool aScriptFromHead, bool aAsync, bool aDefer,
                          const mozilla::net::ReferrerPolicy aReferrerPolicy)
 {
   NS_ENSURE_TRUE_VOID(mDocument);
   // Check to see if scripts has been turned off.
   if (!mEnabled || !mDocument->IsScriptEnabled()) {
     return;
   }
 
@@ -2953,16 +2954,18 @@ ScriptLoader::PreloadURI(nsIURI* aURI, c
 
   RefPtr<ScriptLoadRequest> request =
     CreateLoadRequest(ScriptKind::Classic, nullptr, 0,
                       Element::StringToCORSMode(aCrossOrigin), sriMetadata);
   request->mURI = aURI;
   request->mIsInline = false;
   request->mReferrerPolicy = aReferrerPolicy;
   request->mScriptFromHead = aScriptFromHead;
+  request->mPreloadAsAsync = aAsync;
+  request->mPreloadAsDefer = aDefer;
 
   nsresult rv = StartLoad(request);
   if (NS_FAILED(rv)) {
     return;
   }
 
   PreloadInfo* pi = mPreloads.AppendElement();
   pi->mRequest = request;
--- a/dom/script/ScriptLoader.h
+++ b/dom/script/ScriptLoader.h
@@ -295,17 +295,17 @@ public:
    *                     Void if not present.
    * @param aIntegrity The expect hash url, if avail, of the request
    * @param aScriptFromHead Whether or not the script was a child of head
    */
   virtual void PreloadURI(nsIURI* aURI, const nsAString& aCharset,
                           const nsAString& aType,
                           const nsAString& aCrossOrigin,
                           const nsAString& aIntegrity,
-                          bool aScriptFromHead,
+                          bool aScriptFromHead, bool aAsync, bool aDefer,
                           const mozilla::net::ReferrerPolicy aReferrerPolicy);
 
   /**
    * Process a request that was deferred so that the script could be compiled
    * off thread.
    */
   nsresult ProcessOffThreadRequest(ScriptLoadRequest* aRequest);
 
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/script/file_blocked_script.sjs
@@ -0,0 +1,50 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+function setGlobalState(data, key)
+{
+  x = { data: data, QueryInterface: function(iid) { return this } };
+  x.wrappedJSObject = x;
+  setObjectState(key, x);
+}
+
+function getGlobalState(key)
+{
+  var data;
+  getObjectState(key, function(x) {
+    data = x.wrappedJSObject.data;
+  });
+  return data;
+}
+
+function handleRequest(request, response)
+{
+  var query = request.queryString.split('&');
+  switch (query[0]) {
+    case "blocked":
+      setGlobalState(response, query[1]);
+      response.processAsync();
+      break;
+
+    case "unblock":
+      response.setStatusLine(request.httpVersion, 200, "OK");
+      response.setHeader("Cache-Control", "no-cache", false);
+      response.setHeader("Content-Type", "image/png", false);
+      response.write("\x89PNG"); // just a broken image is enough for our purpose
+
+      var blockedResponse = getGlobalState(query[1]);
+      blockedResponse.setStatusLine(request.httpVersion, 200, "OK");
+      blockedResponse.setHeader("Cache-Control", "no-cache", false);
+      blockedResponse.setHeader("Content-Type", "application/javascript", false);
+      blockedResponse.write("window.script_executed_" + query[1] + " = true; ok(true, 'Script " + query[1] + " executed');");
+      blockedResponse.finish();
+      break;
+
+    default:
+      response.setStatusLine(request.httpVersion, 400, "Bad request");
+      break;
+  }
+}
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/script/mochitest.ini
@@ -0,0 +1,6 @@
+[DEFAULT]
+support-files =
+  file_blocked_script.sjs
+
+[test_bug1053321.html]
+skip-if = os == 'android' # bug 1386644
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/script/test_bug1053321.html
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<!--
+This test confirms we don't block body referred sub-resources by head-referenced defer and async scripts.
+If this test times out, the two image requests, that unblock the two script requests, never happen, hence
+are unexpectedly blocked.
+-->
+
+<head>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+
+  <!-- this script is not loaded until file_blocked_script.sjs?unblock&defer request is made,
+       when this script is executed, it sets window.script_executed_defer to true
+   -->
+  <script defer src="file_blocked_script.sjs?blocked&defer"></script>
+
+  <!-- this script is not loaded until file_blocked_script.sjs?unblock&async request is made,
+       when this script is executed, it sets window.script_executed_async to true
+   -->
+  <script async src="file_blocked_script.sjs?blocked&async"></script>
+</head>
+
+<body>
+  <script>
+    // No need for an async test, we make it all before window.onload.
+    //
+    // We can't test whether the two scripts have not been executed here, since
+    // preloads of the two images below (that unblock the two tested <head>
+    // scripts) may happen sooner than this script executes.
+    document.addEventListener("DOMContentLoaded", function() {
+      ok(window.script_executed_defer, "Deferred script executed before DOMContentLoaded");
+    });
+    window.addEventListener("load", function() {
+      ok(window.script_executed_async, "Async script executed before onload");
+    }, true);
+  </script>
+  <img src="file_blocked_script.sjs?unblock&defer"/>
+  <img src="file_blocked_script.sjs?unblock&async"/>
+</body>
--- a/dom/tests/moz.build
+++ b/dom/tests/moz.build
@@ -92,16 +92,19 @@ with Files("mochitest/orientation/**"):
     BUG_COMPONENT = ("Core", "DOM: Device Interfaces")
 
 with Files("mochitest/orientation/*507902*"):
     BUG_COMPONENT = ("Core", "Layout: Images")
 
 with Files("mochitest/pointerlock/**"):
     BUG_COMPONENT = ("Core", "DOM")
 
+with Files("mochitest/script/**"):
+    BUG_COMPONENT = ("Core", "DOM: Core & HTML")
+
 with Files("mochitest/sessionstorage/**"):
     BUG_COMPONENT = ("Core", "DOM: Core & HTML")
 
 with Files("mochitest/storageevent/**"):
     BUG_COMPONENT = ("Core", "DOM")
 
 with Files("mochitest/webcomponents/**"):
     BUG_COMPONENT = ("Core", "DOM")
@@ -164,16 +167,17 @@ MOCHITEST_MANIFESTS += [
     'mochitest/fetch/mochitest.ini',
     'mochitest/gamepad/mochitest.ini',
     'mochitest/general/mochitest.ini',
     'mochitest/geolocation/mochitest.ini',
     'mochitest/localstorage/mochitest.ini',
     'mochitest/notification/mochitest.ini',
     'mochitest/orientation/mochitest.ini',
     'mochitest/pointerlock/mochitest.ini',
+    'mochitest/script/mochitest.ini',
     'mochitest/sessionstorage/mochitest.ini',
     'mochitest/storageevent/mochitest.ini',
     'mochitest/webcomponents/mochitest.ini',
     'mochitest/whatwg/mochitest.ini',
 ]
 
 MOCHITEST_CHROME_MANIFESTS += [
     'mochitest/beacon/chrome.ini',
--- a/parser/html/nsHtml5SpeculativeLoad.cpp
+++ b/parser/html/nsHtml5SpeculativeLoad.cpp
@@ -1,19 +1,22 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsHtml5SpeculativeLoad.h"
 #include "nsHtml5TreeOpExecutor.h"
 
 nsHtml5SpeculativeLoad::nsHtml5SpeculativeLoad()
+  :
 #ifdef DEBUG
- : mOpCode(eSpeculativeLoadUninitialized)
+  mOpCode(eSpeculativeLoadUninitialized),
 #endif
+  mIsAsync(false),
+  mIsDefer(false)
 {
   MOZ_COUNT_CTOR(nsHtml5SpeculativeLoad);
 }
 
 nsHtml5SpeculativeLoad::~nsHtml5SpeculativeLoad()
 {
   MOZ_COUNT_DTOR(nsHtml5SpeculativeLoad);
   NS_ASSERTION(mOpCode != eSpeculativeLoadUninitialized,
@@ -43,21 +46,21 @@ nsHtml5SpeculativeLoad::Perform(nsHtml5T
       aExecutor->PreloadEndPicture();
       break;
     case eSpeculativeLoadPictureSource:
       aExecutor->PreloadPictureSource(mSrcset, mSizes, mTypeOrCharsetSourceOrDocumentMode,
                                       mMedia);
       break;
     case eSpeculativeLoadScript:
       aExecutor->PreloadScript(mUrl, mCharset, mTypeOrCharsetSourceOrDocumentMode,
-                               mCrossOrigin, mIntegrity, false);
+                               mCrossOrigin, mIntegrity, false, mIsAsync, mIsDefer);
       break;
     case eSpeculativeLoadScriptFromHead:
       aExecutor->PreloadScript(mUrl, mCharset, mTypeOrCharsetSourceOrDocumentMode,
-                               mCrossOrigin, mIntegrity, true);
+                               mCrossOrigin, mIntegrity, true, mIsAsync, mIsDefer);
       break;
     case eSpeculativeLoadStyle:
       aExecutor->PreloadStyle(mUrl, mCharset, mCrossOrigin, mIntegrity);
       break;
     case eSpeculativeLoadManifest:  
       aExecutor->ProcessOfflineManifest(mUrl);
       break;
     case eSpeculativeLoadSetDocumentCharset: {
--- a/parser/html/nsHtml5SpeculativeLoad.h
+++ b/parser/html/nsHtml5SpeculativeLoad.h
@@ -123,27 +123,31 @@ class nsHtml5SpeculativeLoad {
       aMedia.ToString(mMedia);
     }
 
     inline void InitScript(nsHtml5String aUrl,
                            nsHtml5String aCharset,
                            nsHtml5String aType,
                            nsHtml5String aCrossOrigin,
                            nsHtml5String aIntegrity,
-                           bool aParserInHead)
+                           bool aParserInHead,
+                           bool aAsync,
+                           bool aDefer)
     {
       NS_PRECONDITION(mOpCode == eSpeculativeLoadUninitialized,
                       "Trying to reinitialize a speculative load!");
       mOpCode = aParserInHead ?
           eSpeculativeLoadScriptFromHead : eSpeculativeLoadScript;
       aUrl.ToString(mUrl);
       aCharset.ToString(mCharset);
       aType.ToString(mTypeOrCharsetSourceOrDocumentMode);
       aCrossOrigin.ToString(mCrossOrigin);
       aIntegrity.ToString(mIntegrity);
+      mIsAsync = aAsync;
+      mIsDefer = aDefer;
     }
 
     inline void InitStyle(nsHtml5String aUrl,
                           nsHtml5String aCharset,
                           nsHtml5String aCrossOrigin,
                           nsHtml5String aIntegrity)
     {
       NS_PRECONDITION(mOpCode == eSpeculativeLoadUninitialized,
@@ -216,16 +220,23 @@ class nsHtml5SpeculativeLoad {
       aUrl.ToString(mUrl);
       aCrossOrigin.ToString(mCrossOrigin);
     }
 
     void Perform(nsHtml5TreeOpExecutor* aExecutor);
 
   private:
     eHtml5SpeculativeLoad mOpCode;
+
+    /**
+     * Whether the refering element has async and/or defer attributes.
+     */
+    bool mIsAsync;
+    bool mIsDefer;
+
     nsString mUrl;
     nsString mReferrerPolicy;
     nsString mMetaCSP;
 
     /**
      * If mOpCode is eSpeculativeLoadStyle or eSpeculativeLoadScript[FromHead]
      * then this is the value of the "charset" attribute. For
      * eSpeculativeLoadSetDocumentCharset it is the charset that the
--- a/parser/html/nsHtml5TreeBuilderCppSupplement.h
+++ b/parser/html/nsHtml5TreeBuilderCppSupplement.h
@@ -162,26 +162,30 @@ nsHtml5TreeBuilder::createElement(int32_
             nsHtml5String charset =
               aAttributes->getValue(nsHtml5AttributeName::ATTR_CHARSET);
             nsHtml5String type =
               aAttributes->getValue(nsHtml5AttributeName::ATTR_TYPE);
             nsHtml5String crossOrigin =
               aAttributes->getValue(nsHtml5AttributeName::ATTR_CROSSORIGIN);
             nsHtml5String integrity =
               aAttributes->getValue(nsHtml5AttributeName::ATTR_INTEGRITY);
+            bool async =
+              aAttributes->contains(nsHtml5AttributeName::ATTR_ASYNC);
+            bool defer =
+              aAttributes->contains(nsHtml5AttributeName::ATTR_DEFER);
             mSpeculativeLoadQueue.AppendElement()->InitScript(
               url,
               charset,
               type,
               crossOrigin,
               integrity,
-              mode == nsHtml5TreeBuilder::IN_HEAD);
-            mCurrentHtmlScriptIsAsyncOrDefer =
-              aAttributes->contains(nsHtml5AttributeName::ATTR_ASYNC) ||
-              aAttributes->contains(nsHtml5AttributeName::ATTR_DEFER);
+              mode == nsHtml5TreeBuilder::IN_HEAD,
+              async,
+              defer);
+            mCurrentHtmlScriptIsAsyncOrDefer = async || defer;
           }
         } else if (nsGkAtoms::link == aName) {
           nsHtml5String rel =
             aAttributes->getValue(nsHtml5AttributeName::ATTR_REL);
           // Not splitting on space here is bogus but the old parser didn't even
           // do a case-insensitive check.
           if (rel) {
             if (rel.LowerCaseEqualsASCII("stylesheet")) {
@@ -274,17 +278,19 @@ nsHtml5TreeBuilder::createElement(int32_
             nsHtml5String integrity =
               aAttributes->getValue(nsHtml5AttributeName::ATTR_INTEGRITY);
             mSpeculativeLoadQueue.AppendElement()->InitScript(
               url,
               nullptr,
               type,
               crossOrigin,
               integrity,
-              mode == nsHtml5TreeBuilder::IN_HEAD);
+              mode == nsHtml5TreeBuilder::IN_HEAD,
+              false,
+              false);
           }
         } else if (nsGkAtoms::style == aName) {
           nsHtml5TreeOperation* treeOp = mOpQueue.AppendElement();
           NS_ASSERTION(treeOp, "Tree op allocation failed.");
           treeOp->Init(eTreeOpSetStyleLineNumber, content, tokenizer->getLineNumber());
 
           nsHtml5String url =
             aAttributes->getValue(nsHtml5AttributeName::ATTR_XLINK_HREF);
--- a/parser/html/nsHtml5TreeOpExecutor.cpp
+++ b/parser/html/nsHtml5TreeOpExecutor.cpp
@@ -942,24 +942,26 @@ nsHtml5TreeOpExecutor::ShouldPreloadURI(
 }
 
 void
 nsHtml5TreeOpExecutor::PreloadScript(const nsAString& aURL,
                                      const nsAString& aCharset,
                                      const nsAString& aType,
                                      const nsAString& aCrossOrigin,
                                      const nsAString& aIntegrity,
-                                     bool aScriptFromHead)
+                                     bool aScriptFromHead,
+                                     bool aAsync,
+                                     bool aDefer)
 {
   nsCOMPtr<nsIURI> uri = ConvertIfNotPreloadedYet(aURL);
   if (!uri) {
     return;
   }
   mDocument->ScriptLoader()->PreloadURI(uri, aCharset, aType, aCrossOrigin,
-                                           aIntegrity, aScriptFromHead,
+                                           aIntegrity, aScriptFromHead, aAsync, aDefer,
                                            mSpeculationReferrerPolicy);
 }
 
 void
 nsHtml5TreeOpExecutor::PreloadStyle(const nsAString& aURL,
                                     const nsAString& aCharset,
                                     const nsAString& aCrossOrigin,
                                     const nsAString& aIntegrity)
--- a/parser/html/nsHtml5TreeOpExecutor.h
+++ b/parser/html/nsHtml5TreeOpExecutor.h
@@ -247,17 +247,19 @@ class nsHtml5TreeOpExecutor final : publ
 
     nsIURI* GetViewSourceBaseURI();
 
     void PreloadScript(const nsAString& aURL,
                        const nsAString& aCharset,
                        const nsAString& aType,
                        const nsAString& aCrossOrigin,
                        const nsAString& aIntegrity,
-                       bool aScriptFromHead);
+                       bool aScriptFromHead,
+                       bool aAsync,
+                       bool aDefer);
 
     void PreloadStyle(const nsAString& aURL, const nsAString& aCharset,
                       const nsAString& aCrossOrigin,
                       const nsAString& aIntegrity);
 
     void PreloadImage(const nsAString& aURL,
                       const nsAString& aCrossOrigin,
                       const nsAString& aSrcset,