Bug 1377158 - (Part 4) Reduce the assertion cost to check reconstruction frame hint. draft
authorKuoE0 <kuoe0.tw@gmail.com>
Fri, 28 Jul 2017 00:23:05 +0800
changeset 643234 d9e0a149a1b48d9fb83f978bdacbf0dfccf827eb
parent 643233 0cf7234a06c1e1d5035fcdb3c0a5cd335742f4b9
child 643235 3bfb2c2f613c5223a1eed703792eb654065245fb
push id73025
push userbmo:kuoe0@mozilla.com
push dateWed, 09 Aug 2017 12:29:24 +0000
bugs1377158
milestone57.0a1
Bug 1377158 - (Part 4) Reduce the assertion cost to check reconstruction frame hint. The original assertion took too long time to check nsStyleChangeList. It caused the test case with many elements timed-out and failed. MozReview-Commit-ID: FpNZvdQFTtR
layout/base/nsStyleChangeList.cpp
--- a/layout/base/nsStyleChangeList.cpp
+++ b/layout/base/nsStyleChangeList.cpp
@@ -36,27 +36,35 @@ nsStyleChangeList::AppendChange(nsIFrame
               aHint & nsChangeHint_ReconstructFrame),
              "Shouldn't be trying to restyle non-elements directly, "
              "except if it's a display:contents child or a text node "
              "doing lazy frame construction");
   MOZ_ASSERT(!(aHint & nsChangeHint_AllReflowHints) ||
              (aHint & nsChangeHint_NeedReflow),
              "Reflow hint bits set without actually asking for a reflow");
 
-  // If Servo fires reconstruct at a node, it is the only change hint fired at
-  // that node.
-  if (IsServo()) {
-    for (size_t i = 0; i < Length(); ++i) {
-      MOZ_ASSERT(!aContent || !((aHint | (*this)[i].mHint) & nsChangeHint_ReconstructFrame) ||
-                 (*this)[i].mContent != aContent);
-    }
-  } else {
-    // Filter out all other changes for same content for Gecko (Servo asserts against this
-    // case above).
-    if (aContent && (aHint & nsChangeHint_ReconstructFrame)) {
+  if (aContent && (aHint & nsChangeHint_ReconstructFrame)) {
+    // If Servo fires reconstruct at a node, it is the only change hint fired at
+    // that node.
+    if (IsServo()) {
+      // Note: Because we check whether |aHint| is a reconstruct above (which is
+      // necessary to avoid debug test timeouts on certain crashtests), this check
+      // will not find bugs where we add a non-reconstruct hint for an element after
+      // adding a reconstruct. This is ok though, since ProcessRestyledFrames will
+      // handle that case via mDestroyedFrames.
+      for (size_t i = 0; i < Length(); ++i) {
+        MOZ_ASSERT(aContent != (*this)[i].mContent ||
+                   !((*this)[i].mHint & nsChangeHint_ReconstructFrame),
+                   "Should not append a non-ReconstructFrame hint after \
+                   appending a ReconstructFrame hint for the same \
+                   content.");
+      }
+    } else {
+      // Filter out all other changes for same content for Gecko (Servo asserts against this
+      // case above).
       // NOTE: This is captured by reference to please static analysis.
       // Capturing it by value as a pointer should be fine in this case.
       RemoveElementsBy([&](const nsStyleChangeData& aData) {
         return aData.mContent == aContent;
       });
     }
   }