Merge backout of changeset 24917a339f2e (bug 475128) because it didn't patch the IsRoot check nsRuleNode::Sweep, which it needs to.

layout/base/nsPresContext.h

@@ -282,7 +282,6 @@ public:
 
   void FreeToShell(size_t aSize, void* aFreeChunk)
   {
-    NS_ASSERTION(mShell, "freeing after shutdown");
     if (mShell)
       mShell->FreeFrame(aSize, aFreeChunk);
   }

layout/style/nsStyleContext.cpp

@@ -86,7 +86,7 @@ nsStyleContext::nsStyleContext(nsStyleContext* aParent,
       r1 = r1->GetParent();
     while (r2->GetParent())
       r2 = r2->GetParent();
-    NS_ASSERTION(r1 == r2, "must be in the same rule tree as parent");
+    NS_ABORT_IF_FALSE(r1 == r2, "must be in the same rule tree as parent");
 #endif
   }
 

layout/style/nsStyleSet.cpp

@@ -86,9 +86,9 @@ nsStyleSet::nsStyleSet()
     mRuleWalker(nsnull),
     mDestroyedCount(0),
     mBatching(0),
+    mOldRuleTree(nsnull),
     mInShutdown(PR_FALSE),
     mAuthorStyleDisabled(PR_FALSE),
-    mInReconstruct(PR_FALSE),
     mDirty(0)
 {
 }
@@ -126,7 +126,7 @@ nsStyleSet::Init(nsPresContext *aPresContext)
 nsresult
 nsStyleSet::BeginReconstruct()
 {
-  NS_ASSERTION(!mInReconstruct, "Unmatched begin/end?");
+  NS_ASSERTION(!mOldRuleTree, "Unmatched begin/end?");
   NS_ASSERTION(mRuleTree, "Reconstructing before first construction?");
 
   // Create a new rule tree root
@@ -141,19 +141,13 @@ nsStyleSet::BeginReconstruct()
   }
 
   // Save the old rule tree so we can destroy it later
-  if (!mOldRuleTrees.AppendElement(mRuleTree)) {
-    delete ruleWalker;
-    newTree->Destroy();
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+  mOldRuleTree = mRuleTree;
   // Delete mRuleWalker because it holds a reference to the rule tree root
   delete mRuleWalker;
+  // We don't need to clear out mRoots; NotifyStyleContextDestroyed
+  // will, and they're useful in EndReconstruct if they don't get
+  // completely cleared out.
 
-  // We need to keep mRoots so that the rule tree GC will only free the
-  // rule trees that really aren't referenced anymore (which should be
-  // all of them, if there are no bugs in reresolution code).
-
-  mInReconstruct = PR_TRUE;
   mRuleTree = newTree;
   mRuleWalker = ruleWalker;
 
@@ -163,8 +157,6 @@ nsStyleSet::BeginReconstruct()
 void
 nsStyleSet::EndReconstruct()
 {
-  NS_ASSERTION(mInReconstruct, "Unmatched begin/end?");
-  mInReconstruct = PR_FALSE;
 #ifdef DEBUG
   for (PRInt32 i = mRoots.Length() - 1; i >= 0; --i) {
     nsRuleNode *n = mRoots[i]->GetRuleNode();
@@ -178,13 +170,16 @@ nsStyleSet::EndReconstruct()
     // mRoots; we only need to check the rule nodes of mRoots
     // themselves.
 
-    NS_ASSERTION(n == mRuleTree, "style context has old rule node");
+    NS_ABORT_IF_FALSE(n == mRuleTree, "style context has old rule node");
   }
 #endif
-  // This *should* destroy the only element of mOldRuleTrees, but in
-  // case of some bugs (which would trigger the above assertions), it
-  // won't.
-  GCRuleTrees();
+  NS_ASSERTION(mOldRuleTree, "Unmatched begin/end?");
+  // Reset the destroyed count; it's no longer valid
+  mDestroyedCount = 0;
+  // Destroy the old rule tree (all the associated style contexts should have
+  // been destroyed by the caller beforehand)
+  mOldRuleTree->Destroy();
+  mOldRuleTree = nsnull;
 }
 
 void
@@ -880,15 +875,6 @@ nsStyleSet::Shutdown(nsPresContext* aPresContext)
   mRuleTree->Destroy();
   mRuleTree = nsnull;
 
-  // We can have old rule trees either because:
-  //   (1) we failed the assertions in EndReconstruct, or
-  //   (2) we're shutting down within a reconstruct (see bug 462392)
-  for (PRUint32 i = mOldRuleTrees.Length(); i > 0; ) {
-    --i;
-    mOldRuleTrees[i]->Destroy();
-  }
-  mOldRuleTrees.Clear();
-
   mDefaultStyleData.Destroy(0, aPresContext);
 }
 
@@ -909,43 +895,27 @@ nsStyleSet::NotifyStyleContextDestroyed(nsPresContext* aPresContext,
     mRoots.RemoveElement(aStyleContext);
   }
 
-  if (mInReconstruct)
+  if (mOldRuleTree)
     return;
 
   if (++mDestroyedCount == kGCInterval) {
-    GCRuleTrees();
-  }
-}
+    mDestroyedCount = 0;
 
-void
-nsStyleSet::GCRuleTrees()
-{
-  mDestroyedCount = 0;
-
-  // Mark the style context tree by marking all style contexts which
-  // have no parent, which will mark all descendants.  This will reach
-  // style contexts in the undisplayed map and "additional style
-  // contexts" since they are descendants of the roots.
-  for (PRInt32 i = mRoots.Length() - 1; i >= 0; --i) {
-    mRoots[i]->Mark();
-  }
-
-  // Sweep the rule tree.
-#ifdef DEBUG
-  PRBool deleted =
-#endif
-    mRuleTree->Sweep();
-  NS_ASSERTION(!deleted, "Root node must not be gc'd");
-
-  // Sweep the old rule trees.
-  for (PRUint32 i = mOldRuleTrees.Length(); i > 0; ) {
-    --i;
-    if (mOldRuleTrees[i]->Sweep()) {
-      // It was deleted, as it should be.
-      mOldRuleTrees.RemoveElementAt(i);
-    } else {
-      NS_NOTREACHED("old rule tree still referenced");
+    // Mark the style context tree by marking all roots, which will mark
+    // all descendants.  This will reach style contexts in the
+    // undisplayed map and "additional style contexts" since they are
+    // descendants of the root.
+    for (PRInt32 i = mRoots.Length() - 1; i >= 0; --i) {
+      mRoots[i]->Mark();
     }
+
+    // Sweep the rule tree.
+#ifdef DEBUG
+    PRBool deleted =
+#endif
+      mRuleTree->Sweep();
+
+    NS_ASSERTION(!deleted, "Root node must not be gc'd");
   }
 }
 

layout/style/nsStyleSet.h

@@ -246,9 +246,6 @@ class nsStyleSet
   // Returns false on out-of-memory.
   PRBool BuildDefaultStyleData(nsPresContext* aPresContext);
 
-  // Run mark-and-sweep GC on mRuleTree and mOldRuleTrees, based on mRoots.
-  void GCRuleTrees();
-
   // Update the rule processor list after a change to the style sheet list.
   nsresult GatherRuleProcessors(sheetType aType);
 
@@ -320,14 +317,11 @@ class nsStyleSet
 
   PRUint16 mBatching;
 
-  // Old rule trees, which should only be non-empty between
-  // BeginReconstruct and EndReconstruct, but in case of bugs that cause
-  // style contexts to exist too long, may last longer.
-  nsTArray<nsRuleNode*> mOldRuleTrees;
+  nsRuleNode* mOldRuleTree; // Old rule tree; used during tree reconstruction
+                            // (See BeginReconstruct and EndReconstruct)
 
   unsigned mInShutdown : 1;
   unsigned mAuthorStyleDisabled: 1;
-  unsigned mInReconstruct : 1;
   unsigned mDirty : 7;  // one dirty bit is used per sheet type
 
 };