Bug 1290218 Part 4: Implement shared mInners for ServoStyleSheets, and standardize calling of AddSheet into CSSStyleSheet and ServoStyleSheet constructors. r=heycam
authorBrad Werth <bwerth@mozilla.com>
Fri, 17 Feb 2017 15:48:35 -0800
changeset 391200 b433bbbd6228fa4a0bdfb3fc4b0fcdc562fb419b
parent 391199 5e9b9569c297ee944aa7b6eece3ba2b7cf65e088
child 391201 f42cbc7439d02019fb663a49010e62c3af620598
push id7198
push userjlorenzo@mozilla.com
push dateTue, 18 Apr 2017 12:07:49 +0000
treeherdermozilla-beta@d57aa49c3948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1290218
milestone54.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 1290218 Part 4: Implement shared mInners for ServoStyleSheets, and standardize calling of AddSheet into CSSStyleSheet and ServoStyleSheet constructors. r=heycam MozReview-Commit-ID: 7u89J0WfMcX
layout/style/CSSStyleSheet.cpp
layout/style/CSSStyleSheet.h
layout/style/ServoStyleSheet.cpp
layout/style/ServoStyleSheet.h
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;
@@ -213,17 +211,17 @@ CSSStyleSheet::SizeOfIncludingThis(Mallo
   while (s) {
     // Each inner can be shared by multiple sheets.  So we only count the inner
     // if this sheet is the last one in the list of those sharing it.  As a
     // result, the last such sheet takes all the blame for the memory
     // consumption of the inner, which isn't ideal but it's better than
     // double-counting the inner.  We use last instead of first since the first
     // sheet may be held in the nsXULPrototypeCache and not used in a window at
     // all.
-    if (Inner()->mSheets.LastElement() == s) {
+    if (mInner->mSheets.LastElement() == s) {
       n += Inner()->SizeOfIncludingThis(aMallocSizeOf);
     }
 
     // Measurement of the following members may be added later if DMD finds it
     // is worthwhile:
     // - s->mRuleCollection
     // - s->mRuleProcessors
     //
@@ -232,20 +230,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();
 }
@@ -258,40 +255,27 @@ 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.Length() > 1)) {
+    mOrderedRules.EnumerateForwards(SetStyleSheetReference, mSheets[1]);
+
+    ChildSheetListBuilder::ReparentChildList(mSheets[1], mFirstChild);
   }
-  if (aSheet == mSheets.ElementAt(0)) {
-    mSheets.RemoveElementAt(0);
-    NS_ASSERTION(mSheets.Length(), "no parents");
-    mOrderedRules.EnumerateForwards(SetStyleSheetReference,
-                                    mSheets.ElementAt(0));
 
-    ChildSheetListBuilder::ReparentChildList(mSheets[0], mFirstChild);
-  }
-  else {
-    mSheets.RemoveElement(aSheet);
-  }
+  // Don't do anything after this call, because superclass implementation
+  // may delete this.
+  StyleSheetInfo::RemoveSheet(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);
@@ -368,64 +352,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;
+  MOZ_ASSERT(mInner, "We should have an mInner after copy.");
+  MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us.");
 
-  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();
-  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) {
@@ -442,17 +428,17 @@ CSSStyleSheet::DropRuleCollection()
   }
 }
 
 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 (Inner()->mSheets.Length() != 1) {
+  if (mInner->mSheets.Length() != 1) {
     return;
   }
 
   Inner()->mOrderedRules.EnumerateForwards(SetStyleSheetReference, nullptr);
   Inner()->mOrderedRules.Clear();
 
   // Have to be a bit careful with child sheets, because we want to
   // drop their mNext pointers and null out their mParent and
@@ -477,17 +463,17 @@ CSSStyleSheet::UnlinkInner()
   }
 }
 
 void
 CSSStyleSheet::TraverseInner(nsCycleCollectionTraversalCallback &cb)
 {
   // 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 (Inner()->mSheets.Length() != 1) {
+  if (mInner->mSheets.Length() != 1) {
     return;
   }
 
   StyleSheet* childSheet = GetFirstChild();
   while (childSheet) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "child sheet");
     cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIDOMCSSStyleSheet*, childSheet));
     childSheet = childSheet->mNext;
@@ -657,25 +643,25 @@ CSSStyleSheet::GetStyleRuleAt(int32_t aI
   return Inner()->mOrderedRules.SafeObjectAt(aIndex);
 }
 
 void
 CSSStyleSheet::EnsureUniqueInner()
 {
   mDirty = true;
 
-  MOZ_ASSERT(Inner()->mSheets.Length() != 0,
+  MOZ_ASSERT(mInner->mSheets.Length() != 0,
              "unexpected number of outers");
-  if (Inner()->mSheets.Length() == 1) {
+  if (mInner->mSheets.Length() == 1) {
     // already unique
     return;
   }
   CSSStyleSheetInner* clone = Inner()->CloneFor(this);
   MOZ_ASSERT(clone);
-  Inner()->RemoveSheet(this);
+  mInner->RemoveSheet(this);
   mInner = clone;
 
   // otherwise the rule processor has pointers to the old rules
   ClearRuleCascades();
 
   // let our containing style sets know that if we call
   // nsPresContext::EnsureSafeToHandOutCSSRules we will need to restyle the
   // document
--- 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;
+  DropRuleList();
 }
 
 // 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)
@@ -91,23 +90,16 @@ ServoStyleSheet::ParseSheet(css::Loader*
 
 void
 ServoStyleSheet::LoadFailed()
 {
   mSheet = Servo_StyleSheet_Empty(mParsingMode).Consume();
 }
 
 void
-ServoStyleSheet::DropSheet()
-{
-  mSheet = nullptr;
-  DropRuleList();
-}
-
-void
 ServoStyleSheet::DropRuleList()
 {
   if (mRuleList) {
     mRuleList->DropReference();
     mRuleList = nullptr;
   }
 }
 
--- a/layout/style/ServoStyleSheet.h
+++ b/layout/style/ServoStyleSheet.h
@@ -77,17 +77,16 @@ protected:
   dom::CSSRuleList* GetCssRulesInternal(ErrorResult& aRv);
   uint32_t InsertRuleInternal(const nsAString& aRule,
                               uint32_t aIndex, ErrorResult& aRv);
   void DeleteRuleInternal(uint32_t aIndex, ErrorResult& aRv);
 
   void EnabledStateChangedInternal() {}
 
 private:
-  void DropSheet();
   void DropRuleList();
 
   RefPtr<RawServoStyleSheet> mSheet;
   RefPtr<ServoCSSRuleList> mRuleList;
 
   friend class StyleSheet;
 };
 
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -40,25 +40,33 @@ StyleSheet::StyleSheet(const StyleSheet&
   , 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(aCopy.mInner) // Shallow copy, but concrete subclasses will fix up.
 {
+  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,53 @@ 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.
+#ifdef DEBUG
+  , mPrincipalSet(aCopy.mPrincipalSet)
+#endif
+{
+  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;
@@ -44,16 +52,18 @@ struct StyleSheetInfo
   bool                   mComplete;
 
   // Pointer to start of linked list of child sheets. This is all fundamentally
   // 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;
+  AutoTArray<StyleSheet*, 8> mSheets;
+
 #ifdef DEBUG
   bool                   mPrincipalSet;
 #endif
 };
 
 } // namespace mozilla
 
 #endif // mozilla_StyleSheetInfo_h