Bug 1366735 part 3 - Remove SetCounterStyleDirty. r=heycam draft
authorXidorn Quan <me@upsuper.org>
Tue, 23 May 2017 10:08:50 +1000
changeset 584113 2adf7b64484746a5f416b7902df2ac718a3fdeab
parent 584112 8216769c7ee2a323d2af2080dd97cac3489af82d
child 630288 1fc35ef72a70075a90146b1a6b7e83a1206c00ef
push id60638
push userxquan@mozilla.com
push dateWed, 24 May 2017 22:57:13 +0000
reviewersheycam
bugs1366735
milestone55.0a1
Bug 1366735 part 3 - Remove SetCounterStyleDirty. r=heycam When the counter style in the style struct changes, CalcDifference would return ReconstructFrame, which should cause corresponding use node to be reconstructed. That means a use node with retired counter style should always be destroyed in the next flush, so it makes no sense to reset them anymore. However, we would still need to mark counter lists dirty because otherwise their content may not get update when there are changes to counter styles which are still valid. MozReview-Commit-ID: FnBPx81StzG
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCounterManager.cpp
layout/base/nsCounterManager.h
layout/style/nsStyleStruct.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -9021,17 +9021,17 @@ nsCSSFrameConstructor::RecalcQuotesAndCo
   NS_ASSERTION(!mQuotesDirty, "Quotes updates will be lost");
   NS_ASSERTION(!mCountersDirty, "Counter updates will be lost");
 }
 
 void
 nsCSSFrameConstructor::NotifyCounterStylesAreDirty()
 {
   NS_PRECONDITION(mUpdateCount != 0, "Should be in an update");
-  mCounterManager.SetAllCounterStylesDirty();
+  mCounterManager.SetAllDirty();
   CountersDirty();
 }
 
 void
 nsCSSFrameConstructor::WillDestroyFrameTree()
 {
 #if defined(DEBUG_dbaron_off)
   mCounterManager.Dump();
--- a/layout/base/nsCounterManager.cpp
+++ b/layout/base/nsCounterManager.cpp
@@ -250,32 +250,20 @@ nsCounterManager::RecalcAll()
         nsCounterList* list = iter.UserData();
         if (list->IsDirty()) {
             list->RecalcAll();
         }
     }
 }
 
 void
-nsCounterManager::SetAllCounterStylesDirty()
+nsCounterManager::SetAllDirty()
 {
   for (auto iter = mNames.Iter(); !iter.Done(); iter.Next()) {
-    nsCounterList* list = iter.UserData();
-    bool changed = false;
-
-    for (nsCounterNode* node = list->First(); node; node = list->Next(node)) {
-      if (node->mType == nsCounterNode::USE) {
-        node->UseNode()->SetCounterStyleDirty();
-        changed = true;
-      }
-    }
-
-    if (changed) {
-      list->SetDirty();
-    }
+    iter.UserData()->SetDirty();
   }
 }
 
 bool
 nsCounterManager::DestroyNodesFor(nsIFrame *aFrame)
 {
   bool destroyedAny = false;
   for (auto iter = mNames.Iter(); !iter.Done(); iter.Next()) {
--- a/layout/base/nsCounterManager.h
+++ b/layout/base/nsCounterManager.h
@@ -91,21 +91,16 @@ struct nsCounterUseNode : public nsCount
         , mAllCounters(aAllCounters)
     {
         NS_ASSERTION(aContentIndex <= INT32_MAX, "out of range");
     }
 
     virtual bool InitTextFrame(nsGenConList* aList,
             nsIFrame* aPseudoFrame, nsIFrame* aTextFrame) override;
 
-    void SetCounterStyleDirty()
-    {
-        mCounterStyle = nullptr;
-    }
-
     // assign the correct |mValueAfter| value to a node that has been inserted
     // Should be called immediately after calling |Insert|.
     void Calc(nsCounterList* aList);
 
     // The text that should be displayed for this counter.
     void GetText(nsString& aResult);
 };
 
@@ -218,18 +213,18 @@ public:
 
     // Gets the appropriate counter list, creating it if necessary.
     // Guaranteed to return non-null. (Uses an infallible hashtable API.)
     nsCounterList* CounterListFor(const nsSubstring& aCounterName);
 
     // Clean up data in any dirty counter lists.
     void RecalcAll();
 
-    // Set all counter styles dirty
-    void SetAllCounterStylesDirty();
+    // Set all counter lists dirty
+    void SetAllDirty();
 
     // Destroy nodes for the frame in any lists, and return whether any
     // nodes were destroyed.
     bool DestroyNodesFor(nsIFrame *aFrame);
 
     // Clear all data.
     void Clear() { mNames.Clear(); }
 
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3922,16 +3922,20 @@ nsStyleContent::CalcDifference(const nsS
   // the optimization in ElementRestyler::Restyle is ok.  But if we ever
   // change this code to not reconstruct frames on changes to the
   // 'content' property, then we will need to revisit the optimization
   // in ElementRestyler::Restyle.
 
   // Unfortunately we need to reframe even if the content lengths are the same;
   // a simple reflow will not pick up different text or different image URLs,
   // since we set all that up in the CSSFrameConstructor
+  //
+  // Also note that we also rely on this to return ReconstructFrame when
+  // content changes to ensure that nsCounterUseNode wouldn't reference
+  // to stale counter stylex.
   if (mContents != aNewData.mContents ||
       mIncrements != aNewData.mIncrements ||
       mResets != aNewData.mResets) {
     return nsChangeHint_ReconstructFrame;
   }
 
   return nsChangeHint(0);
 }