Bug 1459498: Remove useless CreateSheet arguments. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 06 May 2018 16:18:27 +0200
changeset 791903 f23ca27993eeedd820d1450b738cb8009e73c3f8
parent 791902 56b67e492d5788d1d841f73a19b66c790f20e8be
child 791904 1f21c819314c5a31da085407164791ec414b14e6
child 791924 6c61476cff3e1c1e59766b7a8c16953e4c6add79
push id108915
push userbmo:emilio@crisal.io
push dateSun, 06 May 2018 14:46:32 +0000
reviewersheycam
bugs1459498
milestone61.0a1
Bug 1459498: Remove useless CreateSheet arguments. r?heycam This is one of the most important steps for bug 1459498. After this I can use StyleSheetInfo to compute IsAlternate. Still all the preferred stylesheet stuff is crazy... MozReview-Commit-ID: 9ZHW9AYGoBe
layout/style/Loader.cpp
layout/style/Loader.h
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -925,36 +925,29 @@ nsresult
 Loader::CreateSheet(nsIURI* aURI,
                     nsIContent* aLinkingContent,
                     nsIPrincipal* aLoaderPrincipal,
                     css::SheetParsingMode aParsingMode,
                     CORSMode aCORSMode,
                     ReferrerPolicy aReferrerPolicy,
                     const nsAString& aIntegrity,
                     bool aSyncLoad,
-                    bool aHasAlternateRel,
-                    const nsAString& aTitle,
                     StyleSheetState& aSheetState,
-                    IsAlternate* aIsAlternate,
                     RefPtr<StyleSheet>* aSheet)
 {
   LOG(("css::Loader::CreateSheet"));
   NS_PRECONDITION(aSheet, "Null out param!");
 
   if (!mSheets) {
     mSheets = new Sheets();
   }
 
   *aSheet = nullptr;
   aSheetState = eSheetStateUnknown;
 
-  // Check the alternate state before doing anything else, because it
-  // can mess with our hashtables.
-  *aIsAlternate = IsAlternateSheet(aTitle, aHasAlternateRel);
-
   if (aURI) {
     aSheetState = eSheetComplete;
     RefPtr<StyleSheet> sheet;
 
     // First, the XUL cache
 #ifdef MOZ_XUL
     if (IsChromeURI(aURI)) {
       nsXULPrototypeCache* cache = nsXULPrototypeCache::GetInstance();
@@ -1903,25 +1896,26 @@ Loader::LoadInlineStyle(const StyleSheet
 
   nsCOMPtr<nsIStyleSheetLinkingElement> owningElement(
       do_QueryInterface(aInfo.mContent));
   NS_ASSERTION(owningElement, "Element is not a style linking element!");
 
   // Since we're not planning to load a URI, no need to hand a principal to the
   // load data or to CreateSheet().
 
+  // Check IsAlternateSheet now, since it can mutate our document.
+  auto isAlternate = IsAlternateSheet(aInfo.mTitle, aInfo.mHasAlternateRel);
+
   StyleSheetState state;
   RefPtr<StyleSheet> sheet;
-  IsAlternate isAlternate;
   nsresult rv = CreateSheet(aInfo,
                             nullptr,
                             eAuthorSheetFeatures,
                             false,
                             state,
-                            &isAlternate,
                             &sheet);
   if (NS_FAILED(rv)) {
     return Err(rv);
   }
   NS_ASSERTION(state == eSheetNeedsParser,
                "Inline sheets should not be cached");
 
   LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
@@ -2028,23 +2022,22 @@ Loader::LoadStyleLink(const StyleSheetIn
                                              false, false);
       loadBlockingAsyncDispatcher->PostDOMEvent();
     }
     return Err(rv);
   }
 
   StyleSheetState state;
   RefPtr<StyleSheet> sheet;
-  IsAlternate isAlternate;
+  auto isAlternate = IsAlternateSheet(aInfo.mTitle, aInfo.mHasAlternateRel);
   rv = CreateSheet(aInfo,
                    principal,
                    eAuthorSheetFeatures,
                    false,
                    state,
-                   &isAlternate,
                    &sheet);
   if (NS_FAILED(rv)) {
     return Err(rv);
   }
 
   LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
 
   auto matched =
@@ -2222,28 +2215,31 @@ Loader::LoadChildSheet(StyleSheet* aPare
 
   // Now that we know it's safe to load this (passes security check and not a
   // loop) do so.
   RefPtr<StyleSheet> sheet;
   StyleSheetState state;
   if (aReusableSheets && aReusableSheets->FindReusableStyleSheet(aURL, sheet)) {
     state = eSheetComplete;
   } else {
-    IsAlternate isAlternate;
     const nsAString& empty = EmptyString();
     // For now, use CORS_NONE for child sheets
-    rv = CreateSheet(aURL, nullptr, principal,
+    rv = CreateSheet(aURL,
+                     nullptr,
+                     principal,
                      aParentSheet->ParsingMode(),
-                     CORS_NONE, aParentSheet->GetReferrerPolicy(),
+                     CORS_NONE,
+                     aParentSheet->GetReferrerPolicy(),
                      EmptyString(), // integrity is only checked on main sheet
                      aParentData ? aParentData->mSyncLoad : false,
-                     false, empty, state, &isAlternate, &sheet);
+                     state,
+                     &sheet);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    PrepareSheet(sheet, empty, empty, aMedia, isAlternate);
+    PrepareSheet(sheet, empty, empty, aMedia, IsAlternate::No);
   }
 
   rv = InsertChildSheet(sheet, aParentSheet);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (state == eSheetComplete) {
     LOG(("  Sheet already complete"));
     // We're completely done.  No need to notify, even, since the
@@ -2381,24 +2377,22 @@ Loader::InternalLoadNonDocumentSheet(nsI
   nsresult rv = CheckContentPolicy(loadingPrincipal, aOriginPrincipal,
                                    aURL, mDocument, aIsPreload);
   NS_ENSURE_SUCCESS(rv, rv);
 
   StyleSheetState state;
   RefPtr<StyleSheet> sheet;
   bool syncLoad = (aObserver == nullptr);
   const nsAString& empty = EmptyString();
-  IsAlternate isAlternate;
-
   rv = CreateSheet(aURL, nullptr, aOriginPrincipal, aParsingMode,
                    aCORSMode, aReferrerPolicy, aIntegrity, syncLoad,
-                   false, empty, state, &isAlternate, &sheet);
+                   state, &sheet);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  PrepareSheet(sheet, empty, empty, nullptr, isAlternate);
+  PrepareSheet(sheet, empty, empty, nullptr, IsAlternate::No);
 
   if (state == eSheetComplete) {
     LOG(("  Sheet already complete"));
     if (aObserver || !mObservers.IsEmpty()) {
       rv = PostLoadEvent(aURL,
                          sheet,
                          aObserver,
                          IsAlternate::No,
--- a/layout/style/Loader.h
+++ b/layout/style/Loader.h
@@ -463,50 +463,42 @@ private:
                               nsISupports* aContext,
                               bool aIsPreload);
 
   nsresult CreateSheet(const StyleSheetInfo& aInfo,
                        nsIPrincipal* aLoaderPrincipal,
                        css::SheetParsingMode aParsingMode,
                        bool aSyncLoad,
                        StyleSheetState& aSheetState,
-                       IsAlternate* aIsAlternate,
                        RefPtr<StyleSheet>* aSheet)
   {
     return CreateSheet(aInfo.mURI,
                        aInfo.mContent,
                        aLoaderPrincipal,
                        aParsingMode,
                        aInfo.mCORSMode,
                        aInfo.mReferrerPolicy,
                        aInfo.mIntegrity,
                        aSyncLoad,
-                       aInfo.mHasAlternateRel,
-                       aInfo.mTitle,
                        aSheetState,
-                       aIsAlternate,
                        aSheet);
   }
 
   // For inline style, the aURI param is null, but the aLinkingContent
   // must be non-null then.  The loader principal must never be null
   // if aURI is not null.
-  // *aIsAlternate is set based on aTitle and aHasAlternateRel.
   nsresult CreateSheet(nsIURI* aURI,
                        nsIContent* aLinkingContent,
                        nsIPrincipal* aLoaderPrincipal,
                        css::SheetParsingMode aParsingMode,
                        CORSMode aCORSMode,
                        ReferrerPolicy aReferrerPolicy,
                        const nsAString& aIntegrity,
                        bool aSyncLoad,
-                       bool aHasAlternateRel,
-                       const nsAString& aTitle,
                        StyleSheetState& aSheetState,
-                       IsAlternate* aIsAlternate,
                        RefPtr<StyleSheet>* aSheet);
 
   // Pass in either a media string or the MediaList from the CSSParser.  Don't
   // pass both.
   //
   // This method will set the sheet's enabled state based on aIsAlternate
   MediaMatched PrepareSheet(StyleSheet* aSheet,
                             const nsAString& aTitle,