Bug 1571530 - Cleanup slightly SheetComplete, and use the right boolean to notify. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 16 Aug 2019 10:56:16 +0000
changeset 488487 a28efee19e9ceda71182874e51e19ee5e3ea72cb
parent 488486 aec66f88bfd05871e1b03067ba70608f6b320979
child 488488 3d9de500e9c51f8f8439d2c35d643e7847bcf260
push id36444
push userccoroiu@mozilla.com
push dateFri, 16 Aug 2019 16:24:18 +0000
treeherdermozilla-central@8a9e9189cd98 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1571530, 1386840
milestone70.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 1571530 - Cleanup slightly SheetComplete, and use the right boolean to notify. r=heycam This is mostly straight-forward cleanup, but there's a behavior change which was an oversight from bug 1386840, the regular mObservers stuff didn't pass the right thing (was passing only mWasAlternate rather than whether it was deferred). I think that as a result, only in XML documents (since nsXMLContentSink is the only thing that adds itself as a global CSS loader observer that ever looks at this boolean), we may end up breaking this assert: https://searchfox.org/mozilla-central/rev/9ae20497229225cb3fa729a787791f424ff8087b/dom/base/nsContentSink.cpp#183 (If there are any sheets with non-matching media queries and such). But I couldn't find a test-case that broke it (tried SVG / XHTML), and in other documents like pure XML you cannot specify a media query... Differential Revision: https://phabricator.services.mozilla.com/D41091
dom/base/nsContentSink.h
dom/prototype/PrototypeDocumentContentSink.h
dom/xml/nsXMLContentSink.h
dom/xslt/xslt/txMozillaXMLOutput.h
layout/style/FontFaceSet.h
layout/style/Loader.cpp
layout/style/PreloadedStyleSheet.h
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
layout/style/nsICSSLoaderObserver.h
--- a/dom/base/nsContentSink.h
+++ b/dom/base/nsContentSink.h
@@ -86,17 +86,17 @@ class nsContentSink : public nsICSSLoade
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsContentSink, nsICSSLoaderObserver)
   // nsITimerCallback
   NS_DECL_NSITIMERCALLBACK
 
   NS_DECL_NSINAMED
 
   // nsICSSLoaderObserver
-  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasAlternate,
+  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasDeferred,
                               nsresult aStatus) override;
 
   virtual nsresult ProcessMETATag(nsIContent* aContent);
 
   // nsIContentSink implementation helpers
   nsresult WillParseImpl(void);
   nsresult WillInterruptImpl(void);
   nsresult WillResumeImpl(void);
--- a/dom/prototype/PrototypeDocumentContentSink.h
+++ b/dom/prototype/PrototypeDocumentContentSink.h
@@ -75,17 +75,17 @@ class PrototypeDocumentContentSink final
   virtual void InitialDocumentTranslationCompleted() override;
   virtual void FlushPendingNotifications(FlushType aType) override{};
   virtual void SetDocumentCharset(NotNull<const Encoding*> aEncoding) override;
   virtual nsISupports* GetTarget() override;
   virtual bool IsScriptExecuting() override;
   virtual void ContinueInterruptedParsingAsync() override;
 
   // nsICSSLoaderObserver
-  NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet, bool aWasAlternate,
+  NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet, bool aWasDeferred,
                               nsresult aStatus) override;
 
   // nsIOffThreadScriptReceiver
   NS_IMETHOD OnScriptCompileComplete(JSScript* aScript,
                                      nsresult aStatus) override;
 
   nsresult OnPrototypeLoadDone(nsXULPrototypeDocument* aPrototype);
 
--- a/dom/xml/nsXMLContentSink.h
+++ b/dom/xml/nsXMLContentSink.h
@@ -76,17 +76,17 @@ class nsXMLContentSink : public nsConten
 
   // nsITransformObserver
   NS_IMETHOD OnDocumentCreated(
       mozilla::dom::Document* aResultDocument) override;
   NS_IMETHOD OnTransformDone(nsresult aResult,
                              mozilla::dom::Document* aResultDocument) override;
 
   // nsICSSLoaderObserver
-  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasAlternate,
+  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasDeferred,
                               nsresult aStatus) override;
   static bool ParsePIData(const nsString& aData, nsString& aHref,
                           nsString& aTitle, nsString& aMedia,
                           bool& aIsAlternate);
 
  protected:
   virtual ~nsXMLContentSink();
 
--- a/dom/xslt/xslt/txMozillaXMLOutput.h
+++ b/dom/xslt/xslt/txMozillaXMLOutput.h
@@ -33,17 +33,17 @@ class txTransformNotifier final : public
                                   public nsICSSLoaderObserver {
  public:
   txTransformNotifier();
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSISCRIPTLOADEROBSERVER
 
   // nsICSSLoaderObserver
-  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasAlternate,
+  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasDeferred,
                               nsresult aStatus) override;
 
   void Init(nsITransformObserver* aObserver);
   nsresult AddScriptElement(nsIScriptElement* aElement);
   void AddPendingStylesheet();
   void OnTransformEnd(nsresult aResult = NS_OK);
   void OnTransformStart();
   nsresult SetOutputDocument(mozilla::dom::Document* aDocument);
--- a/layout/style/FontFaceSet.h
+++ b/layout/style/FontFaceSet.h
@@ -148,17 +148,17 @@ class FontFaceSet final : public DOMEven
   void DidRefresh();
 
   /**
    * Returns whether the "layout.css.font-loading-api.enabled" pref is true.
    */
   static bool PrefEnabled();
 
   // nsICSSLoaderObserver
-  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasAlternate,
+  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasDeferred,
                               nsresult aStatus) override;
 
   FontFace* GetFontFaceAt(uint32_t aIndex);
 
   void FlushUserFontSet();
 
   static nsPresContext* GetPresContextFor(gfxUserFontSet* aUserFontSet) {
     FontFaceSet* set = static_cast<UserFontSet*>(aUserFontSet)->mFontFaceSet;
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -1259,18 +1259,18 @@ nsresult Loader::LoadSheet(SheetLoadData
     return NS_BINDING_ABORTED;
   }
 
   SRIMetadata sriMetadata;
   aLoadData.mSheet->GetIntegrity(sriMetadata);
 
   if (aLoadData.mSyncLoad) {
     LOG(("  Synchronous load"));
-    NS_ASSERTION(!aLoadData.mObserver, "Observer for a sync load?");
-    NS_ASSERTION(aSheetState == SheetState::NeedsParser,
+    MOZ_ASSERT(!aLoadData.mObserver, "Observer for a sync load?");
+    MOZ_ASSERT(aSheetState == SheetState::NeedsParser,
                  "Sync loads can't reuse existing async loads");
 
     // Create a StreamLoader instance to which we will feed
     // the data from the sync load.  Do this before creating the
     // channel to make error recovery simpler.
     nsCOMPtr<nsIStreamListener> streamLoader = new StreamLoader(aLoadData);
 
     if (mDocument) {
@@ -1645,36 +1645,35 @@ void Loader::SheetComplete(SheetLoadData
   // imports will nest more than 8 deep, and multiple sheets with the same URI
   // are rare.
   AutoTArray<RefPtr<SheetLoadData>, 8> datasToNotify;
   DoSheetComplete(aLoadData, datasToNotify);
 
   // Now it's safe to go ahead and notify observers
   uint32_t count = datasToNotify.Length();
   mDatasToNotifyOn += count;
-  for (uint32_t i = 0; i < count; ++i) {
+  for (RefPtr<SheetLoadData>& data : datasToNotify) {
     --mDatasToNotifyOn;
 
-    SheetLoadData* data = datasToNotify[i];
-    NS_ASSERTION(data && data->mMustNotify, "How did this data get here?");
+    MOZ_ASSERT(data, "How did this data get here?");
     if (data->mObserver) {
-      LOG(("  Notifying observer %p for data %p.  wasAlternate: %d",
-           data->mObserver.get(), data, data->mWasAlternate));
+      LOG(("  Notifying observer %p for data %p.  deferred: %d",
+           data->mObserver.get(), data.get(), data->ShouldDefer()));
       data->mObserver->StyleSheetLoaded(data->mSheet, data->ShouldDefer(),
                                         aStatus);
     }
 
     nsTObserverArray<nsCOMPtr<nsICSSLoaderObserver>>::ForwardIterator iter(
         mObservers);
     nsCOMPtr<nsICSSLoaderObserver> obs;
     while (iter.HasMore()) {
       obs = iter.GetNext();
-      LOG(("  Notifying global observer %p for data %p.  wasAlternate: %d",
-           obs.get(), data, data->mWasAlternate));
-      obs->StyleSheetLoaded(data->mSheet, data->mWasAlternate, aStatus);
+      LOG(("  Notifying global observer %p for data %p.  deferred: %d",
+           obs.get(), data.get(), data->ShouldDefer()));
+      obs->StyleSheetLoaded(data->mSheet, data->ShouldDefer(), aStatus);
     }
   }
 
   if (mSheets &&
       mSheets->mLoadingDatas.Count() == 0 &&
       mSheets->mPendingDatas.Count() > 0) {
     LOG(("  No more loading sheets; starting deferred loads"));
     StartDeferredLoads();
--- a/layout/style/PreloadedStyleSheet.h
+++ b/layout/style/PreloadedStyleSheet.h
@@ -47,17 +47,17 @@ class PreloadedStyleSheet : public nsIPr
   class StylesheetPreloadObserver final : public nsICSSLoaderObserver {
    public:
     NS_DECL_ISUPPORTS
 
     explicit StylesheetPreloadObserver(NotNull<dom::Promise*> aPromise,
                                        PreloadedStyleSheet* aSheet)
         : mPromise(aPromise), mPreloadedSheet(aSheet) {}
 
-    NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet, bool aWasAlternate,
+    NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet, bool aWasDeferred,
                                 nsresult aStatus) override;
 
    protected:
     virtual ~StylesheetPreloadObserver() {}
 
    private:
     RefPtr<dom::Promise> mPromise;
     RefPtr<PreloadedStyleSheet> mPreloadedSheet;
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -1110,17 +1110,17 @@ nsresult StyleSheet::ReparseSheet(const 
   // Our rules are no longer considered modified.
   ClearModifiedRules();
 
   return NS_OK;
 }
 
 // nsICSSLoaderObserver implementation
 NS_IMETHODIMP
-StyleSheet::StyleSheetLoaded(StyleSheet* aSheet, bool aWasAlternate,
+StyleSheet::StyleSheetLoaded(StyleSheet* aSheet, bool aWasDeferred,
                              nsresult aStatus) {
   if (!aSheet->GetParentSheet()) {
     return NS_OK;  // ignore if sheet has been detached already
   }
   NS_ASSERTION(this == aSheet->GetParentSheet(),
                "We are being notified of a sheet load for a sheet that is not "
                "our child!");
 
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -129,17 +129,17 @@ class StyleSheet final : public nsICSSLo
   void SetContentsForImport(const RawServoStyleSheetContents* aContents) {
     MOZ_ASSERT(!Inner().mContents);
     Inner().mContents = aContents;
   }
 
   URLExtraData* URLData() const { return Inner().mURLData; }
 
   // nsICSSLoaderObserver interface
-  NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet, bool aWasAlternate,
+  NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet, bool aWasDeferred,
                               nsresult aStatus) final;
 
   // Internal GetCssRules methods which do not have security check and
   // completeness check.
   ServoCSSRuleList* GetCssRulesInternal();
 
   // Returns the stylesheet's Servo origin as a StyleOrigin value.
   mozilla::StyleOrigin GetOrigin() const;
--- a/layout/style/nsICSSLoaderObserver.h
+++ b/layout/style/nsICSSLoaderObserver.h
@@ -26,25 +26,23 @@ class nsICSSLoaderObserver : public nsIS
  public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_ICSSLOADEROBSERVER_IID)
 
   /**
    * StyleSheetLoaded is called after aSheet is marked complete and before any
    * load events associated with aSheet are fired.
    * @param aSheet the sheet that was loaded. Guaranteed to always be
    *        non-null, even if aStatus indicates failure.
-   * @param aWasAlternate whether the sheet was an alternate.  This will always
-   *        match the value LoadStyleLink or LoadInlineStyle returned in
-   *        aIsAlternate if one of those methods were used to load the sheet,
-   *        and will always be false otherwise.
+   * @param aWasDeferred whether the sheet load was deferred, due to it being an
+   *        alternate sheet, or having a non-matching media list.
    * @param aStatus is a success code if the sheet loaded successfully and a
    *        failure code otherwise.  Note that successful load of aSheet
    *        doesn't indicate anything about whether the data actually parsed
    *        as CSS, and doesn't indicate anything about the status of any child
    *        sheets of aSheet.
    */
-  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasAlternate,
+  NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet, bool aWasDeferred,
                               nsresult aStatus) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsICSSLoaderObserver, NS_ICSSLOADEROBSERVER_IID)
 
 #endif  // nsICSSLoaderObserver_h___