Bug 1290218 Part 4: Implement shared mInners for ServoStyleSheets, and standardize calling of AddSheet into CSSStyleSheet and ServoStyleSheet constructors. draft
authorBrad Werth <bwerth@mozilla.com>
Tue, 07 Feb 2017 09:40:50 -0800
changeset 480743 a93aaf305f014e62815ad1ea4b31caa2a3db6754
parent 480742 b5e6576befc4aa94699fd3b6c84fb39b32311a59
child 480744 19710016755f36b05d9bd3eedb90acd1c7d342b4
push id44647
push userbwerth@mozilla.com
push dateWed, 08 Feb 2017 21:49:09 +0000
bugs1290218
milestone54.0a1
Bug 1290218 Part 4: Implement shared mInners for ServoStyleSheets, and standardize calling of AddSheet into CSSStyleSheet and ServoStyleSheet constructors. MozReview-Commit-ID: 7u89J0WfMcX
layout/style/CSSStyleSheet.cpp
layout/style/CSSStyleSheet.h
layout/style/ServoStyleSheet.cpp
layout/style/StyleSheet.cpp
layout/style/StyleSheetInfo.h
--- a/layout/style/CSSStyleSheet.cpp
+++ b/layout/style/CSSStyleSheet.cpp
@@ -122,24 +122,22 @@ CSSRuleListImpl::IndexedGetter(uint32_t 
 
 namespace mozilla {
 
 // -------------------------------
 // CSS Style Sheet Inner Data Container
 //
 
 
-CSSStyleSheetInner::CSSStyleSheetInner(CSSStyleSheet* aPrimarySheet,
-                                       CORSMode aCORSMode,
+CSSStyleSheetInner::CSSStyleSheetInner(CORSMode aCORSMode,
                                        ReferrerPolicy aReferrerPolicy,
                                        const SRIMetadata& aIntegrity)
   : StyleSheetInfo(aCORSMode, aReferrerPolicy, aIntegrity)
 {
   MOZ_COUNT_CTOR(CSSStyleSheetInner);
-  mSheets.AppendElement(aPrimarySheet);
 }
 
 static bool SetStyleSheetReference(css::Rule* aRule, void* aSheet)
 {
   if (aRule) {
     aRule->SetStyleSheet(static_cast<CSSStyleSheet*>(aSheet));
   }
   return true;
@@ -233,20 +231,19 @@ CSSStyleSheet::SizeOfIncludingThis(Mallo
 
     s = s->mNext ? s->mNext->AsGecko() : nullptr;
   }
   return n;
 }
 
 CSSStyleSheetInner::CSSStyleSheetInner(CSSStyleSheetInner& aCopy,
                                        CSSStyleSheet* aPrimarySheet)
-  : StyleSheetInfo(aCopy)
+  : StyleSheetInfo(aCopy, aPrimarySheet)
 {
   MOZ_COUNT_CTOR(CSSStyleSheetInner);
-  AddSheet(aPrimarySheet);
   aCopy.mOrderedRules.EnumerateForwards(css::GroupRule::CloneRuleInto, &mOrderedRules);
   mOrderedRules.EnumerateForwards(SetStyleSheetReference, aPrimarySheet);
 
   ChildSheetListBuilder builder = { &mFirstChild, aPrimarySheet };
   mOrderedRules.EnumerateForwards(CSSStyleSheet::RebuildChildList, &builder);
 
   RebuildNameSpaces();
 }
@@ -259,40 +256,31 @@ CSSStyleSheetInner::~CSSStyleSheetInner(
 
 CSSStyleSheetInner*
 CSSStyleSheetInner::CloneFor(CSSStyleSheet* aPrimarySheet)
 {
   return new CSSStyleSheetInner(*this, aPrimarySheet);
 }
 
 void
-CSSStyleSheetInner::AddSheet(CSSStyleSheet* aSheet)
-{
-  mSheets.AppendElement(aSheet);
-}
-
-void
-CSSStyleSheetInner::RemoveSheet(CSSStyleSheet* aSheet)
+CSSStyleSheetInner::RemoveSheet(StyleSheet* aSheet)
 {
-  if (1 == mSheets.Length()) {
-    NS_ASSERTION(aSheet == mSheets.ElementAt(0), "bad parent");
-    delete this;
-    return;
-  }
-  if (aSheet == mSheets.ElementAt(0)) {
-    mSheets.RemoveElementAt(0);
-    NS_ASSERTION(mSheets.Length(), "no parents");
+  // Get some information now that we'll use if we survive the superclass
+  // implementation of RemoveSheet.
+  bool removingFirstOfSeveral = (aSheet == mSheets.ElementAt(0)) &&
+                                (mSheets.Length() > 1);
+
+  StyleSheetInfo::RemoveSheet(aSheet);
+
+  if (removingFirstOfSeveral) {
     mOrderedRules.EnumerateForwards(SetStyleSheetReference,
                                     mSheets.ElementAt(0));
 
     ChildSheetListBuilder::ReparentChildList(mSheets[0], mFirstChild);
   }
-  else {
-    mSheets.RemoveElement(aSheet);
-  }
 }
 
 static void
 AddNamespaceRuleToMap(css::Rule* aRule, nsXMLNameSpaceMap* aMap)
 {
   NS_ASSERTION(aRule->GetType() == css::Rule::NAMESPACE_RULE, "Bogus rule type");
 
   RefPtr<css::NameSpaceRule> nameSpaceRule = do_QueryObject(aRule);
@@ -369,67 +357,66 @@ CSSStyleSheet::CSSStyleSheet(css::SheetP
                              CORSMode aCORSMode, ReferrerPolicy aReferrerPolicy)
   : StyleSheet(StyleBackendType::Gecko, aParsingMode),
     mOwnerRule(nullptr),
     mDirty(false),
     mInRuleProcessorCache(false),
     mScopeElement(nullptr),
     mRuleProcessors(nullptr)
 {
-  mInner = new CSSStyleSheetInner(this, aCORSMode, aReferrerPolicy,
+  mInner = new CSSStyleSheetInner(aCORSMode, aReferrerPolicy,
                                   SRIMetadata());
+  mInner->AddSheet(this);
 }
 
 CSSStyleSheet::CSSStyleSheet(css::SheetParsingMode aParsingMode,
                              CORSMode aCORSMode,
                              ReferrerPolicy aReferrerPolicy,
                              const SRIMetadata& aIntegrity)
   : StyleSheet(StyleBackendType::Gecko, aParsingMode),
     mOwnerRule(nullptr),
     mDirty(false),
     mInRuleProcessorCache(false),
     mScopeElement(nullptr),
     mRuleProcessors(nullptr)
 {
-  mInner = new CSSStyleSheetInner(this, aCORSMode, aReferrerPolicy,
+  mInner = new CSSStyleSheetInner(aCORSMode, aReferrerPolicy,
                                   aIntegrity);
+  mInner->AddSheet(this);
 }
 
 CSSStyleSheet::CSSStyleSheet(const CSSStyleSheet& aCopy,
                              CSSStyleSheet* aParentToUse,
                              css::ImportRule* aOwnerRuleToUse,
                              nsIDocument* aDocumentToUse,
                              nsINode* aOwningNodeToUse)
   : StyleSheet(aCopy, aDocumentToUse, aOwningNodeToUse),
     mOwnerRule(aOwnerRuleToUse),
     mDirty(aCopy.mDirty),
     mInRuleProcessorCache(false),
     mScopeElement(nullptr),
     mRuleProcessors(nullptr)
 {
-  mParent = aParentToUse;
-  mInner = aCopy.mInner;
+  MOZ_ASSERT(mInner, "We should have an mInner after copy.");
+  MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us.");
 
-  CSSStyleSheetInner* inner = static_cast<CSSStyleSheetInner*>(mInner);
-  inner->AddSheet(this);
+  mParent = aParentToUse;
 
   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();
   }
 }
 
 CSSStyleSheet::~CSSStyleSheet()
 {
   UnparentChildren();
 
   DropRuleCollection();
-  CSSStyleSheetInner* inner = static_cast<CSSStyleSheetInner*>(mInner);
-  inner->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
   }
   if (mInRuleProcessorCache) {
--- a/layout/style/CSSStyleSheet.h
+++ b/layout/style/CSSStyleSheet.h
@@ -49,36 +49,33 @@ class CSSRuleList;
 } // namespace dom
 
   // -------------------------------
 // CSS Style Sheet Inner Data Container
 //
 
 struct CSSStyleSheetInner : public StyleSheetInfo
 {
-  CSSStyleSheetInner(CSSStyleSheet* aPrimarySheet,
-                     CORSMode aCORSMode,
+  CSSStyleSheetInner(CORSMode aCORSMode,
                      ReferrerPolicy aReferrerPolicy,
                      const dom::SRIMetadata& aIntegrity);
   CSSStyleSheetInner(CSSStyleSheetInner& aCopy,
                      CSSStyleSheet* aPrimarySheet);
   ~CSSStyleSheetInner();
 
   CSSStyleSheetInner* CloneFor(CSSStyleSheet* aPrimarySheet);
-  void AddSheet(CSSStyleSheet* aSheet);
-  void RemoveSheet(CSSStyleSheet* aSheet);
+  void RemoveSheet(StyleSheet* aSheet) override;
 
   void RebuildNameSpaces();
 
   // Create a new namespace map
   nsresult CreateNamespaceMap();
 
   size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const;
 
-  AutoTArray<CSSStyleSheet*, 8> mSheets;
   IncrementalClearCOMRuleArray mOrderedRules;
   nsAutoPtr<nsXMLNameSpaceMap> mNameSpaceMap;
 };
 
 
 // -------------------------------
 // CSS Style Sheet
 //
--- a/layout/style/ServoStyleSheet.cpp
+++ b/layout/style/ServoStyleSheet.cpp
@@ -20,25 +20,24 @@ namespace mozilla {
 
 ServoStyleSheet::ServoStyleSheet(css::SheetParsingMode aParsingMode,
                                  CORSMode aCORSMode,
                                  net::ReferrerPolicy aReferrerPolicy,
                                  const dom::SRIMetadata& aIntegrity)
   : StyleSheet(StyleBackendType::Servo, aParsingMode)
 {
   mInner = new StyleSheetInfo(aCORSMode, aReferrerPolicy, aIntegrity);
+  mInner->AddSheet(this);
 }
 
 ServoStyleSheet::~ServoStyleSheet()
 {
   UnparentChildren();
 
   DropSheet();
-
-  delete mInner;
 }
 
 // QueryInterface implementation for ServoStyleSheet
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(ServoStyleSheet)
 NS_INTERFACE_MAP_END_INHERITING(StyleSheet)
 
 NS_IMPL_ADDREF_INHERITED(ServoStyleSheet, StyleSheet)
 NS_IMPL_RELEASE_INHERITED(ServoStyleSheet, StyleSheet)
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -38,27 +38,35 @@ StyleSheet::StyleSheet(const StyleSheet&
   , mDocument(aDocumentToUse)
   , mOwningNode(aOwningNodeToUse)
   , mParsingMode(aCopy.mParsingMode)
   , mType(aCopy.mType)
   , mDisabled(aCopy.mDisabled)
     // We only use this constructor during cloning.  It's the cloner's
     // responsibility to notify us if we end up being owned by a document.
   , mDocumentAssociationMode(NotOwnedByDocument)
-  , mInner(nullptr)
+  , mInner(aCopy.mInner)
 {
+  MOZ_ASSERT(mInner, "Should only copy StyleSheets with an mInner.");
+  mInner->AddSheet(this);
+
   if (aCopy.mMedia) {
     // XXX This is wrong; we should be keeping @import rules and
     // sheets in sync!
     mMedia = aCopy.mMedia->Clone();
   }
 }
 
 StyleSheet::~StyleSheet()
 {
+  MOZ_ASSERT(mInner, "Should have an mInner at time of destruction.");
+  MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us.");
+  mInner->RemoveSheet(this);
+  mInner = nullptr;
+
   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)
@@ -151,16 +159,50 @@ StyleSheetInfo::StyleSheetInfo(CORSMode 
   , mPrincipalSet(false)
 #endif
 {
   if (!mPrincipal) {
     NS_RUNTIMEABORT("nsNullPrincipal::Init failed");
   }
 }
 
+StyleSheetInfo::StyleSheetInfo(StyleSheetInfo& aCopy,
+                               StyleSheet* aPrimarySheet)
+  : mSheetURI(aCopy.mSheetURI)
+  , mOriginalSheetURI(aCopy.mOriginalSheetURI)
+  , mBaseURI(aCopy.mBaseURI)
+  , mPrincipal(aCopy.mPrincipal)
+  , mCORSMode(aCopy.mCORSMode)
+  , mReferrerPolicy(aCopy.mReferrerPolicy)
+  , mIntegrity(aCopy.mIntegrity)
+  , mComplete(aCopy.mComplete)
+  , mFirstChild()  // We don't rebuild the child because we're making a copy
+                   // without children.
+{
+  AddSheet(aPrimarySheet);
+}
+
+void
+StyleSheetInfo::AddSheet(StyleSheet* aSheet)
+{
+  mSheets.AppendElement(aSheet);
+}
+
+void
+StyleSheetInfo::RemoveSheet(StyleSheet* aSheet)
+{
+  if (1 == mSheets.Length()) {
+    NS_ASSERTION(aSheet == mSheets.ElementAt(0), "bad parent");
+    delete this;
+    return;
+  }
+
+  mSheets.RemoveElement(aSheet);
+}
+
 // nsIDOMStyleSheet interface
 
 NS_IMETHODIMP
 StyleSheet::GetType(nsAString& aType)
 {
   aType.AssignLiteral("text/css");
   return NS_OK;
 }
--- a/layout/style/StyleSheetInfo.h
+++ b/layout/style/StyleSheetInfo.h
@@ -27,16 +27,24 @@ namespace mozilla {
 struct StyleSheetInfo
 {
   typedef net::ReferrerPolicy ReferrerPolicy;
 
   StyleSheetInfo(CORSMode aCORSMode,
                  ReferrerPolicy aReferrerPolicy,
                  const dom::SRIMetadata& aIntegrity);
 
+  StyleSheetInfo(StyleSheetInfo& aCopy,
+                 StyleSheet* aPrimarySheet);
+
+  virtual ~StyleSheetInfo() {}
+
+  virtual void AddSheet(StyleSheet* aSheet);
+  virtual void RemoveSheet(StyleSheet* aSheet);
+
   nsCOMPtr<nsIURI>       mSheetURI; // for error reports, etc.
   nsCOMPtr<nsIURI>       mOriginalSheetURI;  // for GetHref.  Can be null.
   nsCOMPtr<nsIURI>       mBaseURI; // for resolving relative URIs
   nsCOMPtr<nsIPrincipal> mPrincipal;
   CORSMode               mCORSMode;
   // The Referrer Policy of a stylesheet is used for its child sheets, so it is
   // stored here.
   ReferrerPolicy         mReferrerPolicy;
@@ -47,13 +55,15 @@ struct StyleSheetInfo
   // broken, because each of the child sheets has a unique parent... We can
   // only hope (and currently this is the case) that any time page JS can get
   // its hands on a child sheet that means we've already ensured unique infos
   // throughout its parent chain and things are good.
   RefPtr<StyleSheet>     mFirstChild;
 #ifdef DEBUG
   bool                   mPrincipalSet;
 #endif
+
+  AutoTArray<StyleSheet*, 8> mSheets;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_StyleSheetInfo_h