Bug 1290209 - Part 6: Move CSSStyleSheet::mMedia up to StyleSheet. r?xidorn draft
authorCameron McCormack <cam@mcc.id.au>
Thu, 05 Jan 2017 14:25:54 +0800
changeset 456285 c8cecdae8cf1437f31c5123ce26ccc1a84342144
parent 456284 84ebd56221513157aba5360e62968ad512df3043
child 456286 5ffcc5d74a2d2834359a9dc80350cfd9d7dff2cf
push id40445
push userbmo:cam@mcc.id.au
push dateThu, 05 Jan 2017 09:33:32 +0000
reviewersxidorn
bugs1290209
milestone53.0a1
Bug 1290209 - Part 6: Move CSSStyleSheet::mMedia up to StyleSheet. r?xidorn MozReview-Commit-ID: LZraHzme6vj
layout/style/CSSStyleSheet.cpp
layout/style/CSSStyleSheet.h
layout/style/Loader.cpp
layout/style/ServoStyleSheet.cpp
layout/style/ServoStyleSheet.h
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
--- a/layout/style/CSSStyleSheet.cpp
+++ b/layout/style/CSSStyleSheet.cpp
@@ -413,38 +413,31 @@ CSSStyleSheet::CSSStyleSheet(const CSSSt
 
   mInner->AddSheet(this);
 
   if (mDirty) { // CSSOM's been there, force full copy now
     NS_ASSERTION(mInner->mComplete, "Why have rules been accessed on an incomplete sheet?");
     // FIXME: handle failure?
     EnsureUniqueInner();
   }
-
-  if (aCopy.mMedia) {
-    // XXX This is wrong; we should be keeping @import rules and
-    // sheets in sync!
-    mMedia = aCopy.mMedia->Clone();
-  }
 }
 
 CSSStyleSheet::~CSSStyleSheet()
 {
   for (CSSStyleSheet* child = mInner->mFirstChild;
        child;
        child = child->mNext) {
     // XXXbz this is a little bogus; see the XXX comment where we
     // declare mFirstChild.
     if (child->mParent == this) {
       child->mParent = nullptr;
       child->mDocument = nullptr;
     }
   }
   DropRuleCollection();
-  DropMedia();
   mInner->RemoveSheet(this);
   // XXX The document reference is not reference counted and should
   // not be released. The document will let us know when it is going
   // away.
   if (mRuleProcessors) {
     NS_ASSERTION(mRuleProcessors->Length() == 0, "destructing sheet with rule processor reference");
     delete mRuleProcessors; // weak refs, should be empty here anyway
   }
@@ -458,25 +451,16 @@ CSSStyleSheet::DropRuleCollection()
 {
   if (mRuleCollection) {
     mRuleCollection->DropReference();
     mRuleCollection = nullptr;
   }
 }
 
 void
-CSSStyleSheet::DropMedia()
-{
-  if (mMedia) {
-    mMedia->SetStyleSheet(nullptr);
-    mMedia = nullptr;
-  }
-}
-
-void
 CSSStyleSheet::UnlinkInner()
 {
   // We can only have a cycle through our inner if we have a unique inner,
   // because otherwise there are no JS wrappers for anything in the inner.
   if (mInner->mSheets.Length() != 1) {
     return;
   }
 
@@ -538,27 +522,25 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_
 NS_INTERFACE_MAP_END_INHERITING(StyleSheet)
 
 NS_IMPL_ADDREF_INHERITED(CSSStyleSheet, StyleSheet)
 NS_IMPL_RELEASE_INHERITED(CSSStyleSheet, StyleSheet)
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(CSSStyleSheet)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(CSSStyleSheet)
-  tmp->DropMedia();
   // We do not unlink mNext; our parent will handle that.  If we
   // unlinked it here, our parent would not be able to walk its list
   // of child sheets and null out the back-references to it, if we got
   // unlinked before it does.
   tmp->DropRuleCollection();
   tmp->UnlinkInner();
   tmp->mScopeElement = nullptr;
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END_INHERITED(StyleSheet)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(CSSStyleSheet, StyleSheet)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMedia)
   // We do not traverse mNext; our parent will handle that.  See
   // comments in Unlink for why.
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRuleCollection)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScopeElement)
   tmp->TraverseInner(cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 nsresult
@@ -606,22 +588,16 @@ CSSStyleSheet::UseForPresentation(nsPres
 {
   if (mMedia) {
     return mMedia->Matches(aPresContext, &aKey);
   }
   return true;
 }
 
 
-void
-CSSStyleSheet::SetMedia(nsMediaList* aMedia)
-{
-  mMedia = aMedia;
-}
-
 bool
 CSSStyleSheet::HasRules() const
 {
   return StyleRuleCount() != 0;
 }
 
 void
 CSSStyleSheet::SetEnabled(bool aEnabled)
@@ -904,27 +880,16 @@ CSSStyleSheet::RegisterNamespaceRule(css
     nsresult rv = mInner->CreateNamespaceMap();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   AddNamespaceRuleToMap(aRule, mInner->mNameSpaceMap);
   return NS_OK;
 }
 
-nsMediaList*
-CSSStyleSheet::Media()
-{
-  if (!mMedia) {
-    mMedia = new nsMediaList();
-    mMedia->SetStyleSheet(this);
-  }
-
-  return mMedia;
-}
-
 nsIDOMCSSRule*
 CSSStyleSheet::GetDOMOwnerRule() const
 {
   return mOwnerRule ? mOwnerRule->GetDOMRule() : nullptr;
 }
 
 CSSRuleList*
 CSSStyleSheet::GetCssRulesInternal(ErrorResult& aRv)
--- a/layout/style/CSSStyleSheet.h
+++ b/layout/style/CSSStyleSheet.h
@@ -26,17 +26,16 @@
 #include "mozilla/CORSMode.h"
 #include "nsCycleCollectionParticipant.h"
 #include "mozilla/net/ReferrerPolicy.h"
 #include "mozilla/dom/SRIMetadata.h"
 
 class CSSRuleListImpl;
 class nsCSSRuleProcessor;
 class nsIURI;
-class nsMediaList;
 class nsMediaQueryResultCacheKey;
 class nsStyleSet;
 class nsPresContext;
 class nsXMLNameSpaceMap;
 
 namespace mozilla {
 struct ChildSheetListBuilder;
 class CSSStyleSheet;
@@ -143,18 +142,16 @@ public:
   void AppendStyleRule(css::Rule* aRule);
 
   int32_t StyleRuleCount() const;
   css::Rule* GetStyleRuleAt(int32_t aIndex) const;
 
   nsresult DeleteRuleFromGroup(css::GroupRule* aGroup, uint32_t aIndex);
   nsresult InsertRuleIntoGroup(const nsAString& aRule, css::GroupRule* aGroup, uint32_t aIndex, uint32_t* _retval);
 
-  void SetMedia(nsMediaList* aMedia);
-
   void SetOwnerRule(css::ImportRule* aOwnerRule) { mOwnerRule = aOwnerRule; /* Not ref counted */ }
   css::ImportRule* GetOwnerRule() const { return mOwnerRule; }
   // Workaround overloaded-virtual warning in GCC.
   using StyleSheet::GetOwnerRule;
 
   nsXMLNameSpaceMap* GetNameSpaceMap() const { return mInner->mNameSpaceMap; }
 
   already_AddRefed<CSSStyleSheet> Clone(CSSStyleSheet* aCloneParent,
@@ -199,19 +196,16 @@ public:
   size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const;
 
   dom::Element* GetScopeElement() const { return mScopeElement; }
   void SetScopeElement(dom::Element* aScopeElement)
   {
     mScopeElement = aScopeElement;
   }
 
-  // WebIDL StyleSheet API
-  nsMediaList* Media() final;
-
   // WebIDL CSSStyleSheet API
   // Can't be inline because we can't include ImportRule here.  And can't be
   // called GetOwnerRule because that would be ambiguous with the ImportRule
   // version.
   nsIDOMCSSRule* GetDOMOwnerRule() const final;
 
   void WillDirty();
   void DidDirty();
@@ -232,32 +226,28 @@ protected:
   void ClearRuleCascades();
 
   // Add the namespace mapping from this @namespace rule to our namespace map
   nsresult RegisterNamespaceRule(css::Rule* aRule);
 
   // Drop our reference to mRuleCollection
   void DropRuleCollection();
 
-  // Drop our reference to mMedia
-  void DropMedia();
-
   // Unlink our inner, if needed, for cycle collection
   void UnlinkInner();
   // Traverse our inner, if needed, for cycle collection
   void TraverseInner(nsCycleCollectionTraversalCallback &);
 
 protected:
   // Internal methods which do not have security check and completeness check.
   dom::CSSRuleList* GetCssRulesInternal(ErrorResult& aRv);
   uint32_t InsertRuleInternal(const nsAString& aRule,
                               uint32_t aIndex, ErrorResult& aRv);
   void DeleteRuleInternal(uint32_t aIndex, ErrorResult& aRv);
 
-  RefPtr<nsMediaList> mMedia;
   RefPtr<CSSStyleSheet> mNext;
   CSSStyleSheet*        mParent;    // weak ref
   css::ImportRule*      mOwnerRule; // weak ref
 
   RefPtr<CSSRuleListImpl> mRuleCollection;
   bool                  mDirty; // has been modified 
   bool                  mInRuleProcessorCache;
   RefPtr<dom::Element> mScopeElement;
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -1299,17 +1299,17 @@ Loader::PrepareSheet(StyleSheet* aSheet,
 
     nsCSSParser mediumParser(this);
 
     // We have aMediaString only when linked from link elements, style
     // elements, or PIs, so pass true.
     mediumParser.ParseMediaList(aMediaString, nullptr, 0, mediaList);
   }
 
-  sheet->SetMedia(mediaList);
+  aSheet->SetMedia(mediaList);
 
   aSheet->SetTitle(aTitle);
   sheet->SetEnabled(!isAlternate);
   sheet->SetScopeElement(aScopeElement);
 }
 
 /**
  * InsertSheetInDoc handles ordering of sheets in the document.  Here
--- a/layout/style/ServoStyleSheet.cpp
+++ b/layout/style/ServoStyleSheet.cpp
@@ -117,22 +117,16 @@ ServoStyleSheet::SizeOfIncludingThis(Mal
 #ifdef DEBUG
 void
 ServoStyleSheet::List(FILE* aOut, int32_t aIndex) const
 {
   MOZ_CRASH("stylo: not implemented");
 }
 #endif
 
-nsMediaList*
-ServoStyleSheet::Media()
-{
-  return nullptr;
-}
-
 nsIDOMCSSRule*
 ServoStyleSheet::GetDOMOwnerRule() const
 {
   return nullptr;
 }
 
 CSSRuleList*
 ServoStyleSheet::GetCssRulesInternal(ErrorResult& aRv)
--- a/layout/style/ServoStyleSheet.h
+++ b/layout/style/ServoStyleSheet.h
@@ -61,19 +61,16 @@ public:
 #endif
 
   RawServoStyleSheet* RawSheet() const { return mSheet; }
   void SetSheetForImport(RawServoStyleSheet* aSheet) {
     MOZ_ASSERT(!mSheet);
     mSheet = aSheet;
   }
 
-  // WebIDL StyleSheet API
-  nsMediaList* Media() final;
-
   // WebIDL CSSStyleSheet API
   // Can't be inline because we can't include ImportRule here.  And can't be
   // called GetOwnerRule because that would be ambiguous with the ImportRule
   // version.
   nsIDOMCSSRule* GetDOMOwnerRule() const final;
 
   void WillDirty() {}
   void DidDirty() {}
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -32,29 +32,53 @@ StyleSheet::StyleSheet(const StyleSheet&
                        nsINode* aOwningNodeToUse)
   : mTitle(aCopy.mTitle)
   , mDocument(aDocumentToUse)
   , mOwningNode(aOwningNodeToUse)
   , mParsingMode(aCopy.mParsingMode)
   , mType(aCopy.mType)
   , mDisabled(aCopy.mDisabled)
 {
+  if (aCopy.mMedia) {
+    // XXX This is wrong; we should be keeping @import rules and
+    // sheets in sync!
+    mMedia = aCopy.mMedia->Clone();
+  }
+}
+
+StyleSheet::~StyleSheet()
+{
+  DropMedia();
 }
 
 // QueryInterface implementation for StyleSheet
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(StyleSheet)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsIDOMStyleSheet)
   NS_INTERFACE_MAP_ENTRY(nsIDOMCSSStyleSheet)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(StyleSheet)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(StyleSheet)
 
-NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_0(StyleSheet)
+NS_IMPL_CYCLE_COLLECTION_CLASS(StyleSheet)
+
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(StyleSheet)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMedia)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(StyleSheet)
+  tmp->DropMedia();
+  NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
+NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(StyleSheet)
+  NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
+NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 mozilla::dom::CSSStyleSheetParsingMode
 StyleSheet::ParsingModeDOM()
 {
 #define CHECK(X, Y) \
   static_assert(static_cast<int>(X) == static_cast<int>(Y),             \
                 "mozilla::dom::CSSStyleSheetParsingMode and mozilla::css::SheetParsingMode should have identical values");
 
@@ -315,16 +339,42 @@ StyleSheet::AreRulesAvailable(nsIPrincip
   //   style sheet can access rule collections.
   SubjectSubsumesInnerPrincipal(aSubjectPrincipal, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return false;
   }
   return true;
 }
 
+void
+StyleSheet::SetMedia(nsMediaList* aMedia)
+{
+  mMedia = aMedia;
+}
+
+void
+StyleSheet::DropMedia()
+{
+  if (mMedia) {
+    mMedia->SetStyleSheet(nullptr);
+    mMedia = nullptr;
+  }
+}
+
+nsMediaList*
+StyleSheet::Media()
+{
+  if (!mMedia) {
+    mMedia = new nsMediaList();
+    mMedia->SetStyleSheet(this);
+  }
+
+  return mMedia;
+}
+
 // nsWrapperCache
 
 JSObject*
 StyleSheet::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return dom::CSSStyleSheetBinding::Wrap(aCx, this, aGivenProto);
 }
 
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -39,17 +39,17 @@ class SRIMetadata;
 class StyleSheet : public nsIDOMCSSStyleSheet
                  , public nsWrapperCache
 {
 protected:
   StyleSheet(StyleBackendType aType, css::SheetParsingMode aParsingMode);
   StyleSheet(const StyleSheet& aCopy,
              nsIDocument* aDocumentToUse,
              nsINode* aOwningNodeToUse);
-  virtual ~StyleSheet() {}
+  virtual ~StyleSheet();
 
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(StyleSheet)
 
   void SetOwningNode(nsINode* aOwningNode)
   {
     mOwningNode = aOwningNode;
@@ -107,16 +107,17 @@ public:
   /**
    * SetPrincipal should be called on all sheets before parsing into them.
    * This can only be called once with a non-null principal.  Calling this with
    * a null pointer is allowed and is treated as a no-op.
    */
   inline void SetPrincipal(nsIPrincipal* aPrincipal);
 
   void SetTitle(const nsAString& aTitle) { mTitle = aTitle; }
+  void SetMedia(nsMediaList* aMedia);
 
   // Get this style sheet's CORS mode
   inline CORSMode GetCORSMode() const;
   // Get this style sheet's Referrer Policy
   inline net::ReferrerPolicy GetReferrerPolicy() const;
   // Get this style sheet's integrity metadata
   inline void GetIntegrity(dom::SRIMetadata& aResult) const;
 
@@ -126,17 +127,17 @@ public:
 #endif
 
   // WebIDL StyleSheet API
   // The XPCOM GetType is fine for WebIDL.
   // The XPCOM GetHref is fine for WebIDL
   // GetOwnerNode is defined above.
   inline StyleSheet* GetParentStyleSheet() const;
   // The XPCOM GetTitle is fine for WebIDL.
-  virtual nsMediaList* Media() = 0;
+  nsMediaList* Media();
   bool Disabled() const { return mDisabled; }
   // The XPCOM SetDisabled is fine for WebIDL.
 
   // WebIDL CSSStyleSheet API
   virtual nsIDOMCSSRule* GetDOMOwnerRule() const = 0;
   dom::CSSRuleList* GetCssRules(nsIPrincipal& aSubjectPrincipal,
                                 ErrorResult& aRv);
   uint32_t InsertRule(const nsAString& aRule, uint32_t aIndex,
@@ -189,20 +190,25 @@ private:
 protected:
   // Return success if the subject principal subsumes the principal of our
   // inner, error otherwise.  This will also succeed if the subject has
   // UniversalXPConnect or if access is allowed by CORS.  In the latter case,
   // it will set the principal of the inner to the subject principal.
   void SubjectSubsumesInnerPrincipal(nsIPrincipal& aSubjectPrincipal,
                                      ErrorResult& aRv);
 
+  // Drop our reference to mMedia
+  void DropMedia();
+
   nsString              mTitle;
   nsIDocument*          mDocument; // weak ref; parents maintain this for their children
   nsINode*              mOwningNode; // weak ref
 
+  RefPtr<nsMediaList> mMedia;
+
   // mParsingMode controls access to nonstandard style constructs that
   // are not safe for use on the public Web but necessary in UA sheets
   // and/or useful in user sheets.
   css::SheetParsingMode mParsingMode;
 
   const StyleBackendType mType;
   bool                  mDisabled;
 };