Bug 1456435: Less bool outparam in Loader too. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 24 Apr 2018 14:12:54 +0200
changeset 787384 14919da1104c81321f7fea9db85e5be2ba4f511e
parent 787383 0c9734460dce8afe0fd0d7a9d8f47947215bba8e
child 787385 502ad6a69492b7b39b549c52ee7011f91fe26549
push id107715
push userbmo:emilio@crisal.io
push dateTue, 24 Apr 2018 19:35:34 +0000
reviewersheycam
bugs1456435
milestone61.0a1
Bug 1456435: Less bool outparam in Loader too. r?heycam MozReview-Commit-ID: D5A2BxwHGjn
dom/base/nsContentSink.cpp
dom/base/nsStyleLinkElement.cpp
layout/style/Loader.cpp
layout/style/Loader.h
--- a/dom/base/nsContentSink.cpp
+++ b/dom/base/nsContentSink.cpp
@@ -784,25 +784,25 @@ nsContentSink::ProcessStyleLinkFromHeade
 
   mozilla::net::ReferrerPolicy referrerPolicy =
     mozilla::net::AttributeReferrerPolicyFromString(aReferrerPolicy);
   if (referrerPolicy == net::RP_Unset) {
     referrerPolicy = mDocument->GetReferrerPolicy();
   }
   // If this is a fragment parser, we don't want to observe.
   // We don't support CORS for processing instructions
-  bool isAlternate;
+  css::Loader::IsAlternate isAlternate;
   rv = mCSSLoader->LoadStyleLink(nullptr, url, nullptr, aTitle, aMedia, aAlternate,
                                  CORS_NONE, referrerPolicy,
                                  /* integrity = */ EmptyString(),
                                  mRunsToCompletion ? nullptr : this,
                                  &isAlternate);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  if (!isAlternate && !mRunsToCompletion) {
+  if (isAlternate == css::Loader::IsAlternate::No && !mRunsToCompletion) {
     ++mPendingSheetCount;
     mScriptLoader->AddParserBlockingScriptExecutionBlocker();
   }
 
   return NS_OK;
 }
 
 
--- a/dom/base/nsStyleLinkElement.cpp
+++ b/dom/base/nsStyleLinkElement.cpp
@@ -430,17 +430,17 @@ nsStyleLinkElement::DoUpdateStyleSheet(n
   // referrerpolicy attribute, ignore this and use the document's referrer
   // policy
 
   net::ReferrerPolicy referrerPolicy = GetLinkReferrerPolicy();
   if (referrerPolicy == net::RP_Unset) {
     referrerPolicy = doc->GetReferrerPolicy();
   }
 
-  bool isAlternate;
+  IsAlternate isAlternate = IsAlternate::No;
   if (isInline) {
     nsAutoString text;
     if (!nsContentUtils::GetNodeTextContent(thisContent, false, text, fallible)) {
       return Err(NS_ERROR_OUT_OF_MEMORY);
     }
 
 
     MOZ_ASSERT(thisContent->NodeInfo()->NameAtom() != nsGkAtoms::link,
@@ -486,11 +486,10 @@ nsStyleLinkElement::DoUpdateStyleSheet(n
       // Don't propagate LoadStyleLink() errors further than this, since some
       // consumers (e.g. nsXMLContentSink) will completely abort on innocuous
       // things like a stylesheet load being blocked by the security system.
       return Update { };
     }
   }
 
   auto willNotify = doneLoading ? WillNotify::No : WillNotify::Yes;
-  auto alternate = isAlternate ? IsAlternate::Yes : IsAlternate::No;
-  return Update { willNotify, alternate };
+  return Update { willNotify, isAlternate };
 }
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -443,17 +443,18 @@ Loader::SetPreferredSheet(const nsAStrin
   if (mSheets) {
     LoadDataArray arr(mSheets->mPendingDatas.Count());
     for (auto iter = mSheets->mPendingDatas.Iter(); !iter.Done(); iter.Next()) {
       SheetLoadData* data = iter.Data();
       MOZ_ASSERT(data, "Must have a data");
 
       // Note that we don't want to affect what the selected style set is, so
       // use true for aHasAlternateRel.
-      if (!data->mLoader->IsAlternate(data->mTitle, true)) {
+      auto isAlternate = data->mLoader->IsAlternateSheet(data->mTitle, true);
+      if (isAlternate == IsAlternate::No) {
         arr.AppendElement(data);
         iter.Remove();
       }
     }
 
     mDatasToNotifyOn += arr.Length();
     for (uint32_t i = 0; i < arr.Length(); ++i) {
       --mDatasToNotifyOn;
@@ -828,40 +829,44 @@ SheetLoadData::VerifySheetReadyToParse(n
   }
 
   // Enough to set the URIs on mSheet, since any sibling datas we have share
   // the same mInner as mSheet and will thus get the same URI.
   mSheet->SetURIs(channelURI, originalURI, channelURI);
   return NS_OK_PARSE_SHEET;
 }
 
-bool
-Loader::IsAlternate(const nsAString& aTitle, bool aHasAlternateRel)
+Loader::IsAlternate
+Loader::IsAlternateSheet(const nsAString& aTitle, bool aHasAlternateRel)
 {
   // A sheet is alternate if it has a nonempty title that doesn't match the
   // currently selected style set.  But if there _is_ no currently selected
   // style set, the sheet wasn't marked as an alternate explicitly, and aTitle
   // is nonempty, we should select the style set corresponding to aTitle, since
   // that's a preferred sheet.
   //
   // FIXME(emilio): This should return false for Shadow DOM regardless of the
   // document.
   if (aTitle.IsEmpty()) {
-    return false;
+    return IsAlternate::No;
   }
 
   if (!aHasAlternateRel && mDocument && mPreferredSheet.IsEmpty()) {
     // There's no preferred set yet, and we now have a sheet with a title.
     // Make that be the preferred set.
     mDocument->SetHeaderData(nsGkAtoms::headerDefaultStyle, aTitle);
-    // We're definitely not an alternate
-    return false;
+    // We're definitely not an alternate.
+    return IsAlternate::No;
   }
 
-  return !aTitle.Equals(mPreferredSheet);
+  if (aTitle.Equals(mPreferredSheet)) {
+    return IsAlternate::No;
+  }
+
+  return IsAlternate::Yes;
 }
 
 nsresult
 Loader::ObsoleteSheet(nsIURI* aURI)
 {
   if (!mSheets) {
     return NS_OK;
   }
@@ -932,32 +937,32 @@ Loader::CreateSheet(nsIURI* aURI,
                     css::SheetParsingMode aParsingMode,
                     CORSMode aCORSMode,
                     ReferrerPolicy aReferrerPolicy,
                     const nsAString& aIntegrity,
                     bool aSyncLoad,
                     bool aHasAlternateRel,
                     const nsAString& aTitle,
                     StyleSheetState& aSheetState,
-                    bool *aIsAlternate,
+                    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 = IsAlternate(aTitle, aHasAlternateRel);
+  *aIsAlternate = IsAlternateSheet(aTitle, aHasAlternateRel);
 
   if (aURI) {
     aSheetState = eSheetComplete;
     RefPtr<StyleSheet> sheet;
 
     // First, the XUL cache
 #ifdef MOZ_XUL
     if (IsChromeURI(aURI)) {
@@ -1118,32 +1123,32 @@ Loader::CreateSheet(nsIURI* aURI,
  * well as setting the enabled state based on the title and whether
  * the sheet had "alternate" in its rel.
  */
 void
 Loader::PrepareSheet(StyleSheet* aSheet,
                      const nsAString& aTitle,
                      const nsAString& aMediaString,
                      MediaList* aMediaList,
-                     bool aIsAlternate)
+                     IsAlternate aIsAlternate)
 {
   NS_PRECONDITION(aSheet, "Must have a sheet!");
 
   RefPtr<MediaList> mediaList(aMediaList);
 
   if (!aMediaString.IsEmpty()) {
     NS_ASSERTION(!aMediaList,
                  "must not provide both aMediaString and aMediaList");
     mediaList = MediaList::Create(aMediaString);
   }
 
   aSheet->SetMedia(mediaList);
 
   aSheet->SetTitle(aTitle);
-  aSheet->SetEnabled(!aIsAlternate);
+  aSheet->SetEnabled(aIsAlternate == IsAlternate::No);
 }
 
 /**
  * InsertSheetInDoc handles ordering of sheets in the document.  Here
  * we have two types of sheets -- those with linking elements and
  * those without.  The latter are loaded by Link: headers.
  * The following constraints are observed:
  * 1) Any sheet with a linking element comes after all sheets without
@@ -1872,17 +1877,17 @@ Loader::LoadInlineStyle(nsIContent* aEle
                         const nsAString& aBuffer,
                         nsIPrincipal* aTriggeringPrincipal,
                         uint32_t aLineNumber,
                         const nsAString& aTitle,
                         const nsAString& aMedia,
                         ReferrerPolicy aReferrerPolicy,
                         nsICSSLoaderObserver* aObserver,
                         bool* aCompleted,
-                        bool* aIsAlternate)
+                        IsAlternate* aIsAlternate)
 {
   LOG(("css::Loader::LoadInlineStyle"));
 
   *aCompleted = true;
 
   if (!mEnabled) {
     LOG_WARN(("  Not enabled"));
     return NS_ERROR_NOT_AVAILABLE;
@@ -1903,17 +1908,17 @@ Loader::LoadInlineStyle(nsIContent* aEle
                             CORS_NONE, aReferrerPolicy,
                             EmptyString(), // no inline integrity checks
                             false, false, aTitle, state, aIsAlternate,
                             &sheet);
   NS_ENSURE_SUCCESS(rv, rv);
   NS_ASSERTION(state == eSheetNeedsParser,
                "Inline sheets should not be cached");
 
-  LOG(("  Sheet is alternate: %d", *aIsAlternate));
+  LOG(("  Sheet is alternate: %d", static_cast<int>(*aIsAlternate)));
 
   PrepareSheet(sheet, aTitle, aMedia, nullptr, *aIsAlternate);
 
   if (aElement->HasFlag(NODE_IS_IN_SHADOW_TREE)) {
     ShadowRoot* containingShadow = aElement->GetContainingShadow();
     MOZ_ASSERT(containingShadow);
     containingShadow->InsertSheet(sheet, aElement);
   } else {
@@ -1926,17 +1931,18 @@ Loader::LoadInlineStyle(nsIContent* aEle
     // The triggering principal may be an expanded principal, which is safe to
     // use for URL security checks, but not as the loader principal for a
     // stylesheet. So treat this as principal inheritance, and downgrade if
     // necessary.
     principal = BasePrincipal::Cast(aTriggeringPrincipal)->PrincipalToInherit();
   }
 
   SheetLoadData* data = new SheetLoadData(this, aTitle, nullptr, sheet,
-                                          owningElement, *aIsAlternate,
+                                          owningElement,
+                                          *aIsAlternate == IsAlternate::Yes,
                                           aObserver, nullptr,
                                           static_cast<nsINode*>(aElement));
 
   // We never actually load this, so just set its principal directly
   sheet->SetPrincipal(principal);
 
   NS_ADDREF(data);
   data->mLineNumber = aLineNumber;
@@ -1961,17 +1967,17 @@ Loader::LoadStyleLink(nsIContent* aEleme
                       nsIPrincipal* aTriggeringPrincipal,
                       const nsAString& aTitle,
                       const nsAString& aMedia,
                       bool aHasAlternateRel,
                       CORSMode aCORSMode,
                       ReferrerPolicy aReferrerPolicy,
                       const nsAString& aIntegrity,
                       nsICSSLoaderObserver* aObserver,
-                      bool* aIsAlternate)
+                      IsAlternate* aIsAlternate)
 {
   NS_PRECONDITION(aURL, "Must have URL to load");
   LOG(("css::Loader::LoadStyleLink"));
   LOG_URI("  Link uri: '%s'", aURL);
   LOG(("  Link title: '%s'", NS_ConvertUTF16toUTF8(aTitle).get()));
   LOG(("  Link media: '%s'", NS_ConvertUTF16toUTF8(aMedia).get()));
   LOG(("  Link alternate rel: %d", aHasAlternateRel));
 
@@ -2014,17 +2020,17 @@ Loader::LoadStyleLink(nsIContent* aEleme
   StyleSheetState state;
   RefPtr<StyleSheet> sheet;
   rv = CreateSheet(aURL, aElement, principal, eAuthorSheetFeatures,
                    aCORSMode, aReferrerPolicy, aIntegrity, false,
                    aHasAlternateRel, aTitle, state, aIsAlternate,
                    &sheet);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  LOG(("  Sheet is alternate: %d", *aIsAlternate));
+  LOG(("  Sheet is alternate: %d", static_cast<int>(*aIsAlternate)));
 
   PrepareSheet(sheet, aTitle, aMedia, nullptr, *aIsAlternate);
 
   rv = InsertSheetInDoc(sheet, aElement, mDocument);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIStyleSheetLinkingElement> owningElement(do_QueryInterface(aElement));
 
@@ -2037,23 +2043,24 @@ Loader::LoadStyleLink(nsIContent* aEleme
     }
 
     return NS_OK;
   }
 
   // Now we need to actually load it
   nsCOMPtr<nsINode> requestingNode = do_QueryInterface(context);
   SheetLoadData* data = new SheetLoadData(this, aTitle, aURL, sheet,
-                                          owningElement, *aIsAlternate,
+                                          owningElement,
+                                          *aIsAlternate == IsAlternate::Yes,
                                           aObserver, principal, requestingNode);
   NS_ADDREF(data);
 
   // If we have to parse and it's an alternate non-inline, defer it
   if (aURL && state == eSheetNeedsParser && mSheets->mLoadingDatas.Count() != 0 &&
-      *aIsAlternate) {
+      *aIsAlternate == IsAlternate::Yes) {
     LOG(("  Deferring alternate sheet load"));
     URIPrincipalReferrerPolicyAndCORSModeHashKey key(data->mURI,
                                                      data->mLoaderPrincipal,
                                                      data->mSheet->GetCORSMode(),
                                                      data->mSheet->GetReferrerPolicy());
     mSheets->mPendingDatas.Put(&key, data);
 
     data->mMustNotify = true;
@@ -2170,17 +2177,17 @@ 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 {
-    bool isAlternate;
+    IsAlternate isAlternate;
     const nsAString& empty = EmptyString();
     // For now, use CORS_NONE for child sheets
     rv = CreateSheet(aURL, nullptr, principal,
                      aParentSheet->ParsingMode(),
                      CORS_NONE, aParentSheet->GetReferrerPolicy(),
                      EmptyString(), // integrity is only checked on main sheet
                      aParentData ? aParentData->mSyncLoad : false,
                      false, empty, state, &isAlternate, &sheet);
@@ -2327,32 +2334,32 @@ Loader::InternalLoadNonDocumentSheet(nsI
   nsCOMPtr<nsIPrincipal> loadingPrincipal = (aOriginPrincipal && mDocument
                                              ? mDocument->NodePrincipal()
                                              : nullptr);
   nsresult rv = CheckContentPolicy(loadingPrincipal, aOriginPrincipal,
                                    aURL, mDocument, aIsPreload);
   NS_ENSURE_SUCCESS(rv, rv);
 
   StyleSheetState state;
-  bool isAlternate;
   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);
   NS_ENSURE_SUCCESS(rv, rv);
 
   PrepareSheet(sheet, empty, empty, nullptr, isAlternate);
 
   if (state == eSheetComplete) {
     LOG(("  Sheet already complete"));
     if (aObserver || !mObservers.IsEmpty()) {
-      rv = PostLoadEvent(aURL, sheet, aObserver, false, nullptr);
+      rv = PostLoadEvent(aURL, sheet, aObserver, IsAlternate::No, nullptr);
     }
     if (aSheet) {
       sheet.swap(*aSheet);
     }
     return rv;
   }
 
   SheetLoadData* data = new SheetLoadData(this,
@@ -2378,30 +2385,30 @@ Loader::InternalLoadNonDocumentSheet(nsI
 
   return rv;
 }
 
 nsresult
 Loader::PostLoadEvent(nsIURI* aURI,
                       StyleSheet* aSheet,
                       nsICSSLoaderObserver* aObserver,
-                      bool aWasAlternate,
+                      IsAlternate aWasAlternate,
                       nsIStyleSheetLinkingElement* aElement)
 {
   LOG(("css::Loader::PostLoadEvent"));
   NS_PRECONDITION(aSheet, "Must have sheet");
   NS_PRECONDITION(aObserver || !mObservers.IsEmpty() || aElement,
                   "Must have observer or element");
 
   RefPtr<SheetLoadData> evt =
     new SheetLoadData(this, EmptyString(), // title doesn't matter here
                       aURI,
                       aSheet,
                       aElement,
-                      aWasAlternate,
+                      aWasAlternate == IsAlternate::Yes,
                       aObserver,
                       nullptr,
                       mDocument);
 
   if (!mPostedEvents.AppendElement(evt)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
--- a/layout/style/Loader.h
+++ b/layout/style/Loader.h
@@ -14,29 +14,29 @@
 #include "nsCompatibility.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsDataHashtable.h"
 #include "nsRefPtrHashtable.h"
 #include "nsStringFwd.h"
 #include "nsTArray.h"
 #include "nsTObserverArray.h"
 #include "nsURIHashKey.h"
+#include "nsIStyleSheetLinkingElement.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/CORSMode.h"
 #include "mozilla/StyleSheetInlines.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/StyleSheet.h"
 #include "mozilla/net/ReferrerPolicy.h"
 
 class nsICSSLoaderObserver;
 class nsIConsoleReportCollector;
 class nsIContent;
 class nsIDocument;
-class nsIStyleSheetLinkingElement;
 
 namespace mozilla {
 namespace dom {
 class DocGroup;
 class Element;
 } // namespace dom
 } // namespace mozilla
 
@@ -186,16 +186,18 @@ enum StyleSheetState {
   eSheetLoading,
   eSheetComplete
 };
 
 class Loader final {
   typedef mozilla::net::ReferrerPolicy ReferrerPolicy;
 
 public:
+  typedef nsIStyleSheetLinkingElement::IsAlternate IsAlternate;
+
   Loader();
   // aDocGroup is used for dispatching SheetLoadData in PostLoadEvent(). It
   // can be null if you want to use this constructor, and there's no
   // document when the Loader is constructed.
   explicit Loader(mozilla::dom::DocGroup*);
   explicit Loader(nsIDocument*);
 
  private:
@@ -243,17 +245,17 @@ public:
                            const nsAString& aBuffer,
                            nsIPrincipal* aTriggeringPrincipal,
                            uint32_t aLineNumber,
                            const nsAString& aTitle,
                            const nsAString& aMedia,
                            ReferrerPolicy aReferrerPolicy,
                            nsICSSLoaderObserver* aObserver,
                            bool* aCompleted,
-                           bool* aIsAlternate);
+                           IsAlternate* aIsAlternate);
 
   /**
    * Load a linked (document) stylesheet.  If a successful result is returned,
    * aObserver is guaranteed to be notified asynchronously once the sheet is
    * loaded and marked complete.  If an error is returned, aObserver will not
    * be notified.  In addition to loading the sheet, this method will insert it
    * into the stylesheet list of this CSSLoader's document.
    *
@@ -278,17 +280,17 @@ public:
                          nsIPrincipal* aTriggeringPrincipal,
                          const nsAString& aTitle,
                          const nsAString& aMedia,
                          bool aHasAlternateRel,
                          CORSMode aCORSMode,
                          ReferrerPolicy aReferrerPolicy,
                          const nsAString& aIntegrity,
                          nsICSSLoaderObserver* aObserver,
-                         bool* aIsAlternate);
+                         IsAlternate* aIsAlternate);
 
   /**
    * Load a child (@import-ed) style sheet.  In addition to loading the sheet,
    * this method will insert it into the child sheet list of aParentSheet.  If
    * there is no sheet currently being parsed and the child sheet is not
    * complete when this method returns, then when the child sheet becomes
    * complete aParentSheet will be QIed to nsICSSLoaderObserver and
    * asynchronously notified, just like for LoadStyleLink.  Note that if the
@@ -459,17 +461,17 @@ public:
    */
   void RemoveObserver(nsICSSLoaderObserver* aObserver);
 
   // These interfaces are public only for the benefit of static functions
   // within nsCSSLoader.cpp.
 
   // IsAlternate can change our currently selected style set if none
   // is selected and aHasAlternateRel is false.
-  bool IsAlternate(const nsAString& aTitle, bool aHasAlternateRel);
+  IsAlternate IsAlternateSheet(const nsAString& aTitle, bool aHasAlternateRel);
 
   typedef nsTArray<RefPtr<SheetLoadData> > LoadDataArray;
 
   // Measure our size.
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   // Marks all the sheets at the given URI obsolete, and removes them from the
   // cache.
@@ -503,28 +505,28 @@ private:
                        css::SheetParsingMode aParsingMode,
                        CORSMode aCORSMode,
                        ReferrerPolicy aReferrerPolicy,
                        const nsAString& aIntegrity,
                        bool aSyncLoad,
                        bool aHasAlternateRel,
                        const nsAString& aTitle,
                        StyleSheetState& aSheetState,
-                       bool *aIsAlternate,
+                       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
   void PrepareSheet(StyleSheet* aSheet,
                     const nsAString& aTitle,
                     const nsAString& aMediaString,
                     dom::MediaList* aMediaList,
-                    bool aIsAlternate);
+                    IsAlternate);
 
   nsresult InsertSheetInDoc(StyleSheet* aSheet,
                             nsIContent* aLinkingContent,
                             nsIDocument* aDocument);
 
   nsresult InsertChildSheet(StyleSheet* aSheet,
                             StyleSheet* aParentSheet);
 
@@ -546,17 +548,17 @@ private:
   // canceled at some point (in which case it will be sent with
   // NS_BINDING_ABORTED).  aWasAlternate indicates the state when the load was
   // initiated, not the state at some later time.  aURI should be the URI the
   // sheet was loaded from (may be null for inline sheets).  aElement is the
   // owning element for this sheet.
   nsresult PostLoadEvent(nsIURI* aURI,
                          StyleSheet* aSheet,
                          nsICSSLoaderObserver* aObserver,
-                         bool aWasAlternate,
+                         IsAlternate aWasAlternate,
                          nsIStyleSheetLinkingElement* aElement);
 
   // Start the loads of all the sheets in mPendingDatas
   void StartAlternateLoads();
 
   // Handle an event posted by PostLoadEvent
   void HandleLoadEvent(SheetLoadData* aEvent);