Issue #618 - (async) Keep track of script modes in a single mode state.

This simplifies handling of combinations of async/defer by assigning one and
only one state to scripts.
If async then always async > if defer or module then defer > otherwise blocking.

3 files changed, 61 insertions, 17 deletions

diff --git a/dom/script/ModuleLoadRequest.cpp b/dom/script/ModuleLoadRequest.cpp
index 746a355b5d..743f30fb97 100644
--- a/dom/script/ModuleLoadRequest.cpp
+++ b/dom/script/ModuleLoadRequest.cpp
@@ -63,6 +63,7 @@ ModuleLoadRequest::ModuleLoadRequest(nsIURI* aURI,
   MOZ_ASSERT(mVisitedSet->Contains(aURI));
 
   mIsInline = false;
+  mScriptMode = aParent->mScriptMode;
 }
 
 void ModuleLoadRequest::Cancel()
diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp
index 9216be835c..59c4ecae46 100644
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -140,6 +140,18 @@ ScriptLoadRequest::AsModuleRequest()
   return static_cast<ModuleLoadRequest*>(this);
 }
 
+void
+ScriptLoadRequest::SetScriptMode(bool aDeferAttr, bool aAsyncAttr)
+{
+  if (aAsyncAttr) {
+    mScriptMode = ScriptMode::eAsync;
+  } else if (aDeferAttr || IsModuleRequest()) {
+    mScriptMode = ScriptMode::eDeferred;
+  } else {
+    mScriptMode = ScriptMode::eBlocking;
+  }
+}
+
 //////////////////////////////////////////////////////////////
 
 // ScriptLoadRequestList
@@ -1049,18 +1061,14 @@ ScriptLoader::StartLoad(ScriptLoadRequest *aRequest, const nsAString &aType,
 
   NS_ENSURE_SUCCESS(rv, rv);
 
-  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 (aScriptFromHead && !async && !defer) {
+    if (aScriptFromHead && aRequest->IsBlockingScript()) {
       // synchronous head scripts block lading of most other non js/css
       // content such as images
       cos->AddClassFlags(nsIClassOfService::Leader);
-    } else if (!defer) {
+    } else if (!aRequest->IsDeferredScript()) {
       // other scripts are neither blocked nor prioritized unless marked deferred
       cos->AddClassFlags(nsIClassOfService::Unblocked);
     }
@@ -1315,8 +1323,15 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement)
       }
     }
 
-    if (!request) {
-      // no usable preload
+    if (request) {
+      // Use a preload request.
+
+      // It's possible these attributes changed since we started the preload, so
+      // update them here.
+      request->SetScriptMode(aElement->GetScriptDeferred(),
+                             aElement->GetScriptAsync());
+    } else {    
+      // No usable preload found.
 
       SRIMetadata sriMetadata;
       {
@@ -1340,8 +1355,10 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement)
                                   version, ourCORSMode, sriMetadata,
                                   ourRefPolicy);
       request->mIsInline = false;
+      request->SetScriptMode(aElement->GetScriptDeferred(),
+                             aElement->GetScriptAsync());
 
-      // set aScriptFromHead to false so we don't treat non preloaded scripts as
+      // Set aScriptFromHead to false so we don't treat non-preloaded scripts as
       // blockers for full page load. See bug 792438.
       rv = StartLoad(request, type, false);
       if (NS_FAILED(rv)) {
@@ -1359,7 +1376,7 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement)
 
     request->mJSVersion = version;
 
-    if (aElement->GetScriptAsync()) {
+    if (request->IsAsyncScript()) {
       AddAsyncRequest(request);
       if (request->IsReadyToRun()) {
         // The script is available already. Run it ASAP when the event
@@ -1386,7 +1403,7 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement)
     }
     // we now have a parser-inserted request that may or may not be still
     // loading
-    if (aElement->GetScriptDeferred() || request->IsModuleRequest()) {
+    if (request->IsDeferredScript()) {
       // We don't want to run this yet.
       // If we come here, the script is a parser-created script and it has
       // the defer attribute but not the async attribute. Since a
@@ -2605,8 +2622,7 @@ ScriptLoader::PreloadURI(nsIURI *aURI, const nsAString &aCharset,
                       Element::StringToCORSMode(aCrossOrigin), sriMetadata,
                       aReferrerPolicy);
   request->mIsInline = false;
-  request->mPreloadAsAsync = aAsync;
-  request->mPreloadAsDefer = aDefer;
+  request->SetScriptMode(aDefer, aAsync);
 
   nsresult rv = StartLoad(request, aType, aScriptFromHead);
   if (NS_FAILED(rv)) {
@@ -2621,6 +2637,9 @@ ScriptLoader::PreloadURI(nsIURI *aURI, const nsAString &aCharset,
 void
 ScriptLoader::AddDeferRequest(ScriptLoadRequest* aRequest)
 {
+  MOZ_ASSERT(aRequest->IsDeferredScript());
+  MOZ_ASSERT(!aRequest->mInDeferList && !aRequest->mInAsyncList);
+
   aRequest->mInDeferList = true;
   mDeferRequests.AppendElement(aRequest);
   if (mDeferEnabled && aRequest == mDeferRequests.getFirst() &&
@@ -2634,6 +2653,9 @@ ScriptLoader::AddDeferRequest(ScriptLoadRequest* aRequest)
 void
 ScriptLoader::AddAsyncRequest(ScriptLoadRequest* aRequest)
 {
+  MOZ_ASSERT(aRequest->IsAsyncScript());
+  MOZ_ASSERT(!aRequest->mInDeferList && !aRequest->mInAsyncList);
+
   aRequest->mInAsyncList = true;
   if (aRequest->IsReadyToRun()) {
     mLoadedAsyncRequests.AppendElement(aRequest);
diff --git a/dom/script/ScriptLoader.h b/dom/script/ScriptLoader.h
index b7e20c7eee..e9b663acd5 100644
--- a/dom/script/ScriptLoader.h
+++ b/dom/script/ScriptLoader.h
@@ -72,12 +72,11 @@ public:
     : mKind(aKind),
       mElement(aElement),
       mProgress(Progress::Loading),
+      mScriptMode(ScriptMode::eBlocking),
       mIsInline(true),
       mHasSourceMapURL(false),
       mInDeferList(false),
       mInAsyncList(false),
-      mPreloadAsAsync(false),
-      mPreloadAsDefer(false),
       mIsNonAsyncScriptInserted(false),
       mIsXSLT(false),
       mIsCanceled(false),
@@ -151,6 +150,29 @@ public:
            (IsReadyToRun() && mWasCompiledOMT);
   }
 
+  enum class ScriptMode : uint8_t {
+    eBlocking,
+    eDeferred,
+    eAsync
+  };
+
+  void SetScriptMode(bool aDeferAttr, bool aAsyncAttr);
+
+  bool IsBlockingScript() const
+  {
+    return mScriptMode == ScriptMode::eBlocking;
+  }
+
+  bool IsDeferredScript() const
+  {
+    return mScriptMode == ScriptMode::eDeferred;
+  }
+
+  bool IsAsyncScript() const
+  {
+    return mScriptMode == ScriptMode::eAsync;
+  }
+
   void MaybeCancelOffThreadScript();
 
   using super::getNext;
@@ -159,12 +181,11 @@ public:
   const ScriptKind mKind;
   nsCOMPtr<nsIScriptElement> mElement;
   Progress mProgress;     // Are we still waiting for a load to complete?
+  ScriptMode mScriptMode; // Whether this script is blocking, deferred or async.
   bool mIsInline;         // Is the script inline or loaded?
   bool mHasSourceMapURL;  // Does the HTTP header have a source map url?
   bool mInDeferList;      // True if we live in mDeferRequests.
   bool mInAsyncList;      // 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.