Bug 1724236 - Move stencil duplication out of JoinOffThread, Decode, Compile r=arai

In InstantiateClassicScriptFromMaybeEncodedSource(), I need to use a local
ErrorResult because calling `aRv.NoteJSContextException(aCx)` must be followed
by an immediate return, but that would avoid adding the compile time to
mMainThreadParseTime.

Differential Revision: https://phabricator.services.mozilla.com/D222301

dom/base/JSExecutionContext.cpp

@@ -99,15 +99,6 @@ void JSExecutionContext::JoinOffThread(JSContext* aCx,
     aRv.NoteJSContextException(aCx);
     return;
   }
-
-  if (mKeepStencil) {
-    mStencil = JS::DuplicateStencil(aCx, aStencil.get());
-    if (!mStencil) {
-      mSkip = true;
-      aRv.NoteJSContextException(aCx);
-      return;
-    }
-  }
 }
 
 template <typename Unit>
@@ -126,15 +117,6 @@ void JSExecutionContext::InternalCompile(JSContext* aCx,
     aRv.NoteJSContextException(aCx);
     return;
   }
-
-  if (mKeepStencil) {
-    mStencil = JS::DuplicateStencil(aCx, aStencil.get());
-    if (!mStencil) {
-      mSkip = true;
-      aRv.NoteJSContextException(aCx);
-      return;
-    }
-  }
 }
 
 void JSExecutionContext::Compile(JSContext* aCx,
@@ -194,15 +176,6 @@ void JSExecutionContext::Decode(JSContext* aCx,
     aRv = NS_ERROR_DOM_JS_DECODING_ERROR;
     return;
   }
-
-  if (mKeepStencil) {
-    mStencil = JS::DuplicateStencil(aCx, aStencil.get());
-    if (!mStencil) {
-      mSkip = true;
-      aRv.NoteJSContextException(aCx);
-      return;
-    }
-  }
 }
 
 void JSExecutionContext::InstantiateStencil(

dom/base/JSExecutionContext.h

@@ -43,14 +43,10 @@ class MOZ_STACK_CLASS JSExecutionContext final {
   JS::Rooted<JS::Value> mDebuggerPrivateValue;
   JS::Rooted<JSScript*> mDebuggerIntroductionScript;
 
-  RefPtr<JS::Stencil> mStencil;
-
   // Used to skip upcoming phases in case of a failure.  In such case the
   // result is carried by mRv.
   bool mSkip;
 
-  bool mKeepStencil = false;
-
  private:
   // Compile a script contained in a SourceText.
   template <typename Unit>
@@ -82,9 +78,6 @@ class MOZ_STACK_CLASS JSExecutionContext final {
     // MOZ_ASSERT_IF(mEncodeBytecode && mScript && mRv == NS_OK, mScriptUsed);
   }
 
-  void SetKeepStencil() { mKeepStencil = true; }
-  already_AddRefed<JS::Stencil> StealStencil() { return mStencil.forget(); }
-
   // After getting a notification that an off-thread compile/decode finished,
   // this function will take the result of the off-thread operation and move it
   // to the main thread.

dom/script/ScriptLoader.cpp

@@ -2724,7 +2724,8 @@ nsresult ScriptLoader::EvaluateScriptElement(ScriptLoadRequest* aRequest) {
 void ScriptLoader::InstantiateClassicScriptFromMaybeEncodedSource(
     JSContext* aCx, JSExecutionContext& aExec,
     JS::CompileOptions& aCompileOptions, ScriptLoadRequest* aRequest,
-    JS::MutableHandle<JSScript*> aScript, ErrorResult& aRv) {
+    JS::MutableHandle<JSScript*> aScript, bool aKeepStencil,
+    RefPtr<JS::Stencil>& aStencilDup, ErrorResult& aRv) {
   nsAutoCString profilerLabelString;
   aRequest->GetScriptLoadContext()->GetProfilerLabel(profilerLabelString);
 
@@ -2737,6 +2738,13 @@ void ScriptLoader::InstantiateClassicScriptFromMaybeEncodedSource(
       aExec.JoinOffThread(aCx, aCompileOptions,
                           aRequest->GetScriptLoadContext(), stencil, storage,
                           aRv);
+      if (!aRv.Failed() && aKeepStencil) {
+        aStencilDup = JS::DuplicateStencil(aCx, stencil.get());
+        if (!aStencilDup) {
+          aRv.NoteJSContextException(aCx);
+          return;
+        }
+      }
       if (stencil) {
         bool unused;
         aExec.InstantiateStencil(aCx, aCompileOptions, std::move(stencil),
@@ -2750,6 +2758,14 @@ void ScriptLoader::InstantiateClassicScriptFromMaybeEncodedSource(
 
       RefPtr<JS::Stencil> stencil;
       aExec.Decode(aCx, aCompileOptions, aRequest->Bytecode(), stencil, aRv);
+      if (!aRv.Failed() && aKeepStencil) {
+        aStencilDup = JS::DuplicateStencil(aCx, stencil.get());
+        if (!aStencilDup) {
+          aRv.NoteJSContextException(aCx);
+          return;
+        }
+      }
+
       if (stencil) {
         bool unused;
         aExec.InstantiateStencil(aCx, aCompileOptions, std::move(stencil),
@@ -2778,6 +2794,13 @@ void ScriptLoader::InstantiateClassicScriptFromMaybeEncodedSource(
     JS::InstantiationStorage storage;
     aExec.JoinOffThread(aCx, aCompileOptions, aRequest->GetScriptLoadContext(),
                         stencil, storage, aRv);
+    if (!aRv.Failed() && aKeepStencil) {
+      aStencilDup = JS::DuplicateStencil(aCx, stencil.get());
+      if (!aStencilDup) {
+        aRv.NoteJSContextException(aCx);
+        return;
+      }
+    }
     if (stencil) {
       bool unused;
       aExec.InstantiateStencil(aCx, aCompileOptions, std::move(stencil),
@@ -2796,20 +2819,29 @@ void ScriptLoader::InstantiateClassicScriptFromMaybeEncodedSource(
                                 profilerLabelString);
 
       RefPtr<JS::Stencil> stencil;
+      ErrorResult erv;
       auto compile = [&](auto& source) {
-        aExec.Compile(aCx, aCompileOptions, source, stencil, aRv);
+        aExec.Compile(aCx, aCompileOptions, source, stencil, erv);
       };
 
       MOZ_ASSERT(!maybeSource.empty());
       TimeStamp startTime = TimeStamp::Now();
       maybeSource.mapNonEmpty(compile);
+      if (!erv.Failed() && aKeepStencil) {
+        aStencilDup = JS::DuplicateStencil(aCx, stencil.get());
+        if (!aStencilDup) {
+          erv.NoteJSContextException(aCx);
+        }
+      }
+
       if (stencil) {
         bool unused;
         aExec.InstantiateStencil(aCx, aCompileOptions, std::move(stencil),
-                                 aScript, unused, aRv);
+                                 aScript, unused, erv);
       }
 
       mMainThreadParseTime += TimeStamp::Now() - startTime;
+      aRv = std::move(erv);
     }
   }
 }
@@ -2856,18 +2888,17 @@ void ScriptLoader::InstantiateClassicScriptFromAny(
       // NOTE: Avoid creating cache regardless of CSP.
       createCache = false;
     }
-
-    if (createCache) {
-      aExec.SetKeepStencil();
-    }
   }
 
+  RefPtr<JS::Stencil> stencilDup;
   InstantiateClassicScriptFromMaybeEncodedSource(aCx, aExec, aCompileOptions,
-                                                 aRequest, aScript, aRv);
+                                                 aRequest, aScript, createCache,
+                                                 stencilDup, aRv);
   if (!aRv.Failed()) {
     if (createCache) {
       MOZ_ASSERT(mCache);
-      aRequest->SetStencil(aExec.StealStencil());
+      MOZ_ASSERT(stencilDup);
+      aRequest->SetStencil(stencilDup.forget());
       auto loadData = MakeRefPtr<ScriptLoadData>(this, aRequest);
       mCache->Insert(*loadData);
     }

dom/script/ScriptLoader.h

@@ -648,10 +648,14 @@ class ScriptLoader final : public JS::loader::ScriptLoaderInterface {
   // Instantiate classic script from one of the following data:
   //   * text source
   //   * encoded bytecode
+  //
+  // If keepStencil is true and this function is successful, aStencilDup will
+  // contain a copy of the compiled stencil for use by the caller.
   void InstantiateClassicScriptFromMaybeEncodedSource(
       JSContext* aCx, JSExecutionContext& aExec,
       JS::CompileOptions& aCompileOptions, ScriptLoadRequest* aRequest,
-      JS::MutableHandle<JSScript*> aScript, ErrorResult& aRv);
+      JS::MutableHandle<JSScript*> aScript, bool keepStencil,
+      RefPtr<JS::Stencil>& aStencilDup, ErrorResult& aRv);
 
   // Instantiate classic script from the following data:
   //   * cached stencil