Bug 1053321 - Correctly pass info about 'defer' and 'async' attributes to HTML5 parser invoked script preload. r=bkelly
authorHonza Bambas <honzab.moz@firemni.cz>
Sat, 19 Aug 2017 05:35:00 -0400
changeset 427967 b3c22267033406033582a84a6076e3b10a232739
parent 427966 3aeae824965c9cb7a492f7d928ae2831d2faed5e
child 427968 8ccc616a23b3b1422e9e3b59e64d90544b7b23bc
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1053321
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1053321 - Correctly pass info about 'defer' and 'async' attributes to HTML5 parser invoked script preload. r=bkelly
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/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
@@ -999,25 +999,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.
@@ -2948,17 +2949,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;
   }
 
@@ -2981,16 +2982,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/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,
@@ -47,22 +50,24 @@ nsHtml5SpeculativeLoad::Perform(nsHtml5T
     case eSpeculativeLoadPictureSource:
       aExecutor->PreloadPictureSource(mCharsetOrSrcset, mUrlOrSizes,
                                       mTypeOrCharsetSourceOrDocumentModeOrMetaCSPOrSizesOrIntegrity,
                                       mCrossOriginOrMedia);
       break;
     case eSpeculativeLoadScript:
       aExecutor->PreloadScript(mUrlOrSizes, mCharsetOrSrcset,
                                mTypeOrCharsetSourceOrDocumentModeOrMetaCSPOrSizesOrIntegrity,
-                               mCrossOriginOrMedia, mReferrerPolicyOrIntegrity, false);
+                               mCrossOriginOrMedia, mReferrerPolicyOrIntegrity, false,
+                               mIsAsync, mIsDefer);
       break;
     case eSpeculativeLoadScriptFromHead:
       aExecutor->PreloadScript(mUrlOrSizes, mCharsetOrSrcset,
                                mTypeOrCharsetSourceOrDocumentModeOrMetaCSPOrSizesOrIntegrity,
-                               mCrossOriginOrMedia, mReferrerPolicyOrIntegrity, true);
+                               mCrossOriginOrMedia, mReferrerPolicyOrIntegrity, true,
+                               mIsAsync, mIsDefer);
       break;
     case eSpeculativeLoadStyle:
       aExecutor->PreloadStyle(mUrlOrSizes, mCharsetOrSrcset, mCrossOriginOrMedia, mReferrerPolicyOrIntegrity,
                               mTypeOrCharsetSourceOrDocumentModeOrMetaCSPOrSizesOrIntegrity);
       break;
     case eSpeculativeLoadManifest:  
       aExecutor->ProcessOfflineManifest(mUrlOrSizes);
       break;
--- a/parser/html/nsHtml5SpeculativeLoad.h
+++ b/parser/html/nsHtml5SpeculativeLoad.h
@@ -123,27 +123,31 @@ class nsHtml5SpeculativeLoad {
       aMedia.ToString(mCrossOriginOrMedia);
     }
 
     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(mUrlOrSizes);
       aCharset.ToString(mCharsetOrSrcset);
       aType.ToString(mTypeOrCharsetSourceOrDocumentModeOrMetaCSPOrSizesOrIntegrity);
       aCrossOrigin.ToString(mCrossOriginOrMedia);
       aIntegrity.ToString(mReferrerPolicyOrIntegrity);
+      mIsAsync = aAsync;
+      mIsDefer = aDefer;
     }
 
     inline void InitStyle(nsHtml5String aUrl,
                           nsHtml5String aCharset,
                           nsHtml5String aCrossOrigin,
                           nsHtml5String aReferrerPolicy,
                           nsHtml5String aIntegrity)
     {
@@ -219,16 +223,23 @@ class nsHtml5SpeculativeLoad {
       aCrossOrigin.ToString(mCrossOriginOrMedia);
     }
 
     void Perform(nsHtml5TreeOpExecutor* aExecutor);
 
   private:
     eHtml5SpeculativeLoad mOpCode;
 
+
+    /**
+     * Whether the refering element has async and/or defer attributes.
+     */
+    bool mIsAsync;
+    bool mIsDefer;
+
     /* If mOpCode is eSpeculativeLoadPictureSource, this is the value of the
      * "sizes" attribute. If the attribute is not set, this will be a void
      * string. Otherwise it empty or the value of the url.
      */
     nsString mUrlOrSizes;
     /**
      * If mOpCode is eSpeculativeLoadScript[FromHead], this is the value of the
      * "integrity" attribute. If the attribute is not set, this will be a void
--- a/parser/html/nsHtml5TreeBuilderCppSupplement.h
+++ b/parser/html/nsHtml5TreeBuilderCppSupplement.h
@@ -180,26 +180,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")) {
@@ -294,17 +298,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
@@ -946,24 +946,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& aReferrerPolicy,
--- 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& aReferrerPolicy,
                       const nsAString& aIntegrity);
 
     void PreloadImage(const nsAString& aURL,
                       const nsAString& aCrossOrigin,