Bug 1742438 - Part 7: Make ScriptDecoder an independent class rather than a base class; r=arai

Now that the Worker ScriptLoader largely conforms to the same shape as the DOM ScriptLoader, and
represents requests in the same way, we can start sharing some code. The ScriptLoadHandler isn't
entirely common to both cases, as it handles preloads (which are not possible in workers). This
patch splits out the common class, ScriptDecoder, as it's own thing. A demonstration of its use is
in Part 8 of this patch queue.

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

dom/script/ScriptLoadHandler.cpp

@@ -51,6 +51,16 @@ namespace mozilla::dom {
 #define LOG_ENABLED() \
   MOZ_LOG_TEST(ScriptLoader::gScriptLoaderLog, mozilla::LogLevel::Debug)
 
+ScriptDecoder::ScriptDecoder(const Encoding* aEncoding,
+                             ScriptDecoder::BOMHandling handleBOM) {
+  if (handleBOM == BOMHandling::Ignore) {
+    mDecoder = aEncoding->NewDecoderWithoutBOMHandling();
+  } else {
+    mDecoder = aEncoding->NewDecoderWithBOMRemoval();
+  }
+  MOZ_ASSERT(mDecoder);
+}
+
 template <typename Unit>
 nsresult ScriptDecoder::DecodeRawDataHelper(
     JS::loader::ScriptLoadRequest* aRequest, const uint8_t* aData,
@@ -156,8 +166,8 @@ ScriptLoadHandler::OnIncrementalData(nsIIncrementalStreamLoader* aLoader,
 
     // Decoder has already been initialized. -- trying to decode all loaded
     // bytes.
-    rv =
-        DecodeRawData(mRequest, aData, aDataLength, /* aEndOfStream = */ false);
+    rv = mDecoder->DecodeRawData(mRequest, aData, aDataLength,
+                                 /* aEndOfStream = */ false);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // If SRI is required for this load, appending new bytes to the hash.
@@ -192,7 +202,8 @@ bool ScriptLoadHandler::TrySetDecoder(nsIIncrementalStreamLoader* aLoader,
 
   // JavaScript modules are always UTF-8.
   if (mRequest->IsModuleRequest()) {
-    mDecoder = UTF_8_ENCODING->NewDecoderWithBOMRemoval();
+    mDecoder = MakeUnique<ScriptDecoder>(UTF_8_ENCODING,
+                                         ScriptDecoder::BOMHandling::Remove);
     return true;
   }
 
@@ -207,7 +218,8 @@ bool ScriptLoadHandler::TrySetDecoder(nsIIncrementalStreamLoader* aLoader,
   const Encoding* encoding;
   std::tie(encoding, std::ignore) = Encoding::ForBOM(Span(aData, aDataLength));
   if (encoding) {
-    mDecoder = encoding->NewDecoderWithBOMRemoval();
+    mDecoder =
+        MakeUnique<ScriptDecoder>(encoding, ScriptDecoder::BOMHandling::Remove);
     return true;
   }
 
@@ -223,7 +235,8 @@ bool ScriptLoadHandler::TrySetDecoder(nsIIncrementalStreamLoader* aLoader,
     nsAutoCString label;
     if (NS_SUCCEEDED(channel->GetContentCharset(label)) &&
         (encoding = Encoding::ForLabel(label))) {
-      mDecoder = encoding->NewDecoderWithoutBOMHandling();
+      mDecoder = MakeUnique<ScriptDecoder>(encoding,
+                                           ScriptDecoder::BOMHandling::Ignore);
       return true;
     }
   }
@@ -245,21 +258,24 @@ bool ScriptLoadHandler::TrySetDecoder(nsIIncrementalStreamLoader* aLoader,
   }
 
   if ((encoding = Encoding::ForLabel(hintCharset))) {
-    mDecoder = encoding->NewDecoderWithoutBOMHandling();
+    mDecoder =
+        MakeUnique<ScriptDecoder>(encoding, ScriptDecoder::BOMHandling::Ignore);
     return true;
   }
 
   // Get the charset from the charset of the document.
   if (mScriptLoader->mDocument) {
     encoding = mScriptLoader->mDocument->GetDocumentCharacterSet();
-    mDecoder = encoding->NewDecoderWithoutBOMHandling();
+    mDecoder =
+        MakeUnique<ScriptDecoder>(encoding, ScriptDecoder::BOMHandling::Ignore);
     return true;
   }
 
   // Curiously, there are various callers that don't pass aDocument. The
   // fallback in the old code was ISO-8859-1, which behaved like
   // windows-1252.
-  mDecoder = WINDOWS_1252_ENCODING->NewDecoderWithoutBOMHandling();
+  mDecoder = MakeUnique<ScriptDecoder>(WINDOWS_1252_ENCODING,
+                                       ScriptDecoder::BOMHandling::Ignore);
   return true;
 }
 
@@ -375,8 +391,8 @@ ScriptLoadHandler::OnStreamComplete(nsIIncrementalStreamLoader* aLoader,
       DebugOnly<bool> encoderSet =
           EnsureDecoder(aLoader, aData, aDataLength, /* aEndOfStream = */ true);
       MOZ_ASSERT(encoderSet);
-      rv = DecodeRawData(mRequest, aData, aDataLength,
-                         /* aEndOfStream = */ true);
+      rv = mDecoder->DecodeRawData(mRequest, aData, aDataLength,
+                                   /* aEndOfStream = */ true);
       NS_ENSURE_SUCCESS(rv, rv);
 
       LOG(("ScriptLoadRequest (%p): Source length in code units = %u",

dom/script/ScriptLoadHandler.h

@@ -13,6 +13,7 @@
 
 #include "nsIIncrementalStreamLoader.h"
 #include "nsISupports.h"
+#include "mozilla/Encoding.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/UniquePtr.h"
@@ -32,8 +33,13 @@ class SRICheckDataVerifier;
 
 class ScriptDecoder {
  public:
-  ScriptDecoder() = default;
+  enum BOMHandling { Ignore, Remove };
+
+  ScriptDecoder(const Encoding* aEncoding,
+                ScriptDecoder::BOMHandling handleBOM);
+
   ~ScriptDecoder() = default;
+
   /*
    * Once the charset is found by the EnsureDecoder function, we can
    * incrementally convert the charset to the one expected by the JS Parser.
@@ -42,9 +48,6 @@ class ScriptDecoder {
                          const uint8_t* aData, uint32_t aDataLength,
                          bool aEndOfStream);
 
-  // Unicode decoder for charset.
-  mozilla::UniquePtr<mozilla::Decoder> mDecoder;
-
  private:
   /*
    * Decode the given data into the already-allocated internal
@@ -57,10 +60,12 @@ class ScriptDecoder {
   nsresult DecodeRawDataHelper(JS::loader::ScriptLoadRequest* aRequest,
                                const uint8_t* aData, uint32_t aDataLength,
                                bool aEndOfStream);
+
+  // Unicode decoder for charset.
+  mozilla::UniquePtr<mozilla::Decoder> mDecoder;
 };
 
-class ScriptLoadHandler final : public nsIIncrementalStreamLoaderObserver,
-                                private ScriptDecoder {
+class ScriptLoadHandler final : public nsIIncrementalStreamLoaderObserver {
  public:
   explicit ScriptLoadHandler(
       ScriptLoader* aScriptLoader, JS::loader::ScriptLoadRequest* aRequest,
@@ -119,6 +124,8 @@ class ScriptLoadHandler final : public nsIIncrementalStreamLoaderObserver,
   // Status of SRI data operations.
   nsresult mSRIStatus;
 
+  UniquePtr<ScriptDecoder> mDecoder;
+
   // Flipped to true after calling NotifyStart the first time
   bool mPreloadStartNotified = false;
 };