Bug 1470420: Cleanup ParseSheet. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 22 Jun 2018 14:40:52 +0200
changeset 809584 4771711f6d6287a0830b1ef04dcdb9941b574214
parent 809583 2fe6eb6db15b9709a5b91d189fc465b05cf1ea12
child 809585 fdcdfafb787e92c6543f3c40aa65785d2ce8274b
push id113714
push userbmo:emilio@crisal.io
push dateFri, 22 Jun 2018 13:03:48 +0000
reviewersxidorn
bugs1470420
milestone62.0a1
Bug 1470420: Cleanup ParseSheet. r?xidorn MozReview-Commit-ID: 3RtTHSo9Z1G
layout/style/Loader.cpp
layout/style/Loader.h
layout/style/StreamLoader.cpp
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -1611,63 +1611,39 @@ Loader::LoadSheet(SheetLoadData* aLoadDa
   aLoadData->mIsLoading = true;
 
   return NS_OK;
 }
 
 /**
  * ParseSheet handles parsing the data stream.
  */
-void
-Loader::ParseSheet(const nsAString& aUTF16,
-                   const nsACString& aUTF8,
+Loader::Completed
+Loader::ParseSheet(const nsACString& aBytes,
                    SheetLoadData* aLoadData,
-                   bool aAllowAsync,
-                   bool& aCompleted)
-{
-  LOG(("css::Loader::ParseSheet"));
-  MOZ_ASSERT(aLoadData, "Must have load data");
-  MOZ_ASSERT(aLoadData->mSheet, "Must have sheet to parse into");
-  aCompleted = false;
-  MOZ_ASSERT(aUTF16.IsEmpty() || aUTF8.IsEmpty());
-  if (!aUTF16.IsEmpty()) {
-    DoParseSheetServo(NS_ConvertUTF16toUTF8(aUTF16),
-                      aLoadData,
-                      aAllowAsync,
-                      aCompleted);
-  } else {
-    DoParseSheetServo(aUTF8, aLoadData, aAllowAsync, aCompleted);
-  }
-}
-
-void
-Loader::DoParseSheetServo(const nsACString& aBytes,
-                          SheetLoadData* aLoadData,
-                          bool aAllowAsync,
-                          bool& aCompleted)
+                   AllowAsyncParse aAllowAsync)
 {
   aLoadData->mIsBeingParsed = true;
 
   StyleSheet* sheet = aLoadData->mSheet;
   MOZ_ASSERT(sheet);
 
   // Some cases, like inline style and UA stylesheets, need to be parsed
   // synchronously. The former may trigger child loads, the latter must not.
-  if (aLoadData->mSyncLoad || !aAllowAsync) {
+  if (aLoadData->mSyncLoad || aAllowAsync == AllowAsyncParse::No) {
     sheet->ParseSheetSync(this, aBytes, aLoadData, aLoadData->mLineNumber);
     aLoadData->mIsBeingParsed = false;
 
     bool noPendingChildren = aLoadData->mPendingChildren == 0;
     MOZ_ASSERT_IF(aLoadData->mSyncLoad, noPendingChildren);
     if (noPendingChildren) {
-      aCompleted = true;
       SheetComplete(aLoadData, NS_OK);
+      return Completed::Yes;
     }
-
-    return;
+    return Completed::No;
   }
 
   // This parse does not need to be synchronous. \o/
   //
   // Note that we need to block onload because there may be no network requests
   // pending.
   BlockOnload();
   RefPtr<SheetLoadData> loadData = aLoadData;
@@ -1680,16 +1656,17 @@ Loader::DoParseSheetServo(const nsACStri
       // If there are no child sheets outstanding, mark us as complete.
       // Otherwise, the children are holding strong refs to the data and
       // will call SheetComplete() on it when they complete.
       if (loadData->mPendingChildren == 0) {
         loadData->mLoader->SheetComplete(loadData, NS_OK);
       }
     }, [] { MOZ_CRASH("rejected parse promise"); }
   );
+  return Completed::No;
 }
 
 /**
  * SheetComplete is the do-it-all cleanup function.  It removes the
  * load data from the "loading" hashtable, adds the sheet to the
  * "completed" hashtable, massages the XUL cache, handles siblings of
  * the load data (other loads for the same URI), handles unblocking
  * blocked parent loads as needed, and most importantly calls
@@ -1948,28 +1925,29 @@ Loader::LoadInlineStyle(const SheetInfo&
                                           nullptr,
                                           aInfo.mContent);
 
   // We never actually load this, so just set its principal directly
   sheet->SetPrincipal(principal);
 
   NS_ADDREF(data);
   data->mLineNumber = aLineNumber;
-  bool completed = true;
   // Parse completion releases the load data.
   //
   // Note that we need to parse synchronously, since the web expects that the
-  // effects of inline stylesheets are visible immediately (aside from @imports).
-  ParseSheet(aBuffer, EmptyCString(), data, /* aAllowAsync = */ false, completed);
+  // effects of inline stylesheets are visible immediately (aside from
+  // @imports).
+  NS_ConvertUTF16toUTF8 utf8(aBuffer);
+  Completed completed = ParseSheet(utf8, data, AllowAsyncParse::No);
 
-  // If completed is true, |data| may well be deleted by now.
-  if (!completed) {
+  // If the sheet is complete already, |data| may well be deleted by now.
+  if (completed == Completed::No) {
     data->mMustNotify = true;
   }
-  return LoadSheetResult { completed ? Completed::Yes : Completed::No, isAlternate, matched };
+  return LoadSheetResult { completed, isAlternate, matched };
 }
 
 Result<Loader::LoadSheetResult, nsresult>
 Loader::LoadStyleLink(const SheetInfo& aInfo, nsICSSLoaderObserver* aObserver)
 {
   MOZ_ASSERT(aInfo.mURI, "Must have URL to load");
   LOG(("css::Loader::LoadStyleLink"));
   LOG_URI("  Link uri: '%s'", aInfo.mURI);
--- a/layout/style/Loader.h
+++ b/layout/style/Loader.h
@@ -547,31 +547,29 @@ private:
   void HandleLoadEvent(SheetLoadData* aEvent);
 
   // Note: LoadSheet is responsible for releasing aLoadData and setting the
   // sheet to complete on failure.
   nsresult LoadSheet(SheetLoadData* aLoadData,
                      StyleSheetState aSheetState,
                      bool aIsPreLoad);
 
-  // Parse the stylesheet in aLoadData. The sheet data comes from aUTF16 if
-  // UTF-16 and from aUTF8 if UTF-8.
-  // Sets aCompleted to true if the parse finished, false otherwise (e.g. if the
-  // sheet had an @import).  If aCompleted is true when this returns, then
-  // ParseSheet also called SheetComplete on aLoadData.
-  void ParseSheet(const nsAString& aUTF16,
-                  const nsACString& aUTF8,
-                  SheetLoadData* aLoadData,
-                  bool aAllowAsync,
-                  bool& aCompleted);
+  enum class AllowAsyncParse {
+    Yes,
+    No,
+  };
 
-  void DoParseSheetServo(const nsACString& aBytes,
-                         SheetLoadData* aLoadData,
-                         bool aAllowAsync,
-                         bool& aCompleted);
+  // Parse the stylesheet in the load data.
+  //
+  // Returns whether the parse finished, false otherwise (e.g. if the sheet had
+  // an @import).
+  //
+  // If this function returns Completed::Yes, then ParseSheet also called
+  // SheetComplete on aLoadData.
+  Completed ParseSheet(const nsACString& aBytes, SheetLoadData*, AllowAsyncParse);
 
   // The load of the sheet in aLoadData is done, one way or another.  Do final
   // cleanup, including releasing aLoadData.
   void SheetComplete(SheetLoadData* aLoadData, nsresult aStatus);
 
   // The guts of SheetComplete.  This may be called recursively on parent datas
   // or datas that had glommed on to a single load.  The array is there so load
   // datas whose observers need to be notified can be added to it.
--- a/layout/style/StreamLoader.cpp
+++ b/layout/style/StreamLoader.cpp
@@ -109,23 +109,20 @@ StreamLoader::OnStopRequest(nsIRequest* 
       rv = encoding->DecodeWithoutBOMHandling(bytes, utf8String, validated);
       NS_ENSURE_SUCCESS(rv, rv);
     }
   } // run destructor for `bytes`
 
   // For reasons I don't understand, factoring the below lines into
   // a method on SheetLoadData resulted in a linker error. Hence,
   // accessing fields of mSheetLoadData from here.
-  bool dummy;
   mSheetLoadData->mLoader->ParseSheet(
-    EmptyString(),
     utf8String,
     mSheetLoadData,
-    /* aAllowAsync = */ true,
-    dummy);
+    Loader::AllowAsyncParse::Yes);
   return NS_OK;
 }
 
 /* nsIStreamListener implementation */
 NS_IMETHODIMP
 StreamLoader::OnDataAvailable(nsIRequest*,
                               nsISupports*,
                               nsIInputStream* aInputStream,