Bug 1470420: Cleanup ParseSheet. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 22 Jun 2018 14:40:52 +0200
changeset 480118 0f3dd9d99d7899b9c402edf8a87e69c4e8ce8cee
parent 480117 f1f7160233b10487456b976cf1df0b686899017f
child 480119 ad3352de6bca44bf782074c5efa2ff7381f113cd
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1470420
milestone62.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 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,41 @@ 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)
+                   AllowAsyncParse aAllowAsync)
 {
   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)
-{
+  MOZ_ASSERT(aLoadData);
   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 +1658,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 +1927,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,30 @@ 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. It may not finish 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,