Bug 1209603 patch 11 - Assert that PeekStyle* results don't change during difference computation. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Mon, 19 Oct 2015 20:42:29 -0700
changeset 301865 bb679653777502d9099a403520ed8b17da936e3f
parent 301864 b2c58421d657b4de854528dcd1c81fda17acddc5
child 301866 48fff3ec4d8113bff05b4dd484f2e5a5481f2f9d
push id5392
push userraliiev@mozilla.com
push dateMon, 14 Dec 2015 20:08:23 +0000
treeherdermozilla-beta@16ce8562a975 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1209603
milestone44.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 1209603 patch 11 - Assert that PeekStyle* results don't change during difference computation. r=heycam This assertion catches the condition that led to the bug. I confirmed that without patch 10 on this bug, the assert fires on the reftest added in patch 4, but does not fire on slight modifications of that testcase that don't show the bug.
layout/style/nsStyleContext.cpp
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -831,37 +831,41 @@ nsStyleContext::CalcStyleDifference(nsSt
   // parent, we might produce these same noninherited hints on this
   // style context's frame due to 'inherit' values, so we do need to
   // compare.
   // (Things like 'em' units are handled by the change hint produced
   // by font-size changing, so we don't need to worry about them like
   // we worry about 'inherit' values.)
   bool compare = mRuleNode != aOther->mRuleNode;
 
+  DebugOnly<uint32_t> structsFound = 0;
+
   // If we had any change in variable values, then we'll need to examine
   // all of the other style structs too, even if the new style context has
   // the same rule node as the old one.
   const nsStyleVariables* thisVariables = PeekStyleVariables();
   if (thisVariables) {
+    structsFound |= NS_STYLE_INHERIT_BIT(Variables);
     const nsStyleVariables* otherVariables = aOther->StyleVariables();
     if (thisVariables->mVariables == otherVariables->mVariables) {
-      *aEqualStructs |= nsCachedStyleData::GetBitForSID(eStyleStruct_Variables);
+      *aEqualStructs |= NS_STYLE_INHERIT_BIT(Variables);
     } else {
       compare = true;
     }
   } else {
-    *aEqualStructs |= nsCachedStyleData::GetBitForSID(eStyleStruct_Variables);
+    *aEqualStructs |= NS_STYLE_INHERIT_BIT(Variables);
   }
 
   DebugOnly<int> styleStructCount = 1;  // count Variables already
 
 #define DO_STRUCT_DIFFERENCE(struct_)                                         \
   PR_BEGIN_MACRO                                                              \
     const nsStyle##struct_* this##struct_ = PeekStyle##struct_();             \
     if (this##struct_) {                                                      \
+      structsFound |= NS_STYLE_INHERIT_BIT(struct_);                          \
       const nsStyle##struct_* other##struct_ = aOther->Style##struct_();      \
       nsChangeHint maxDifference = nsStyle##struct_::MaxDifference();         \
       nsChangeHint differenceAlwaysHandledForDescendants =                    \
         nsStyle##struct_::DifferenceAlwaysHandledForDescendants();            \
       if (this##struct_ == other##struct_) {                                  \
         /* The very same struct, so we know that there will be no */          \
         /* differences.                                           */          \
         *aEqualStructs |= NS_STYLE_INHERIT_BIT(struct_);                      \
@@ -931,16 +935,26 @@ nsStyleContext::CalcStyleDifference(nsSt
   DO_STRUCT_DIFFERENCE(Color);
 #undef EXTRA_DIFF_ARGS
 
 #undef DO_STRUCT_DIFFERENCE
 
   MOZ_ASSERT(styleStructCount == nsStyleStructID_Length,
              "missing a call to DO_STRUCT_DIFFERENCE");
 
+#ifdef DEBUG
+  #define STYLE_STRUCT(name_, callback_)                                      \
+    MOZ_ASSERT(!!(structsFound & NS_STYLE_INHERIT_BIT(name_)) ==              \
+               !!PeekStyle##name_(),                                          \
+               "PeekStyleData results must not change in the middle of "      \
+               "difference calculation.");
+  #include "nsStyleStructList.h"
+  #undef STYLE_STRUCT
+#endif
+
   // We check for struct pointer equality here rather than as part of the
   // DO_STRUCT_DIFFERENCE calls, since those calls can result in structs
   // we previously examined and found to be null on this style context
   // getting computed by later DO_STRUCT_DIFFERENCE calls (which can
   // happen when the nsRuleNode::ComputeXXXData method looks up another
   // struct.)  This is important for callers in RestyleManager that
   // need to know the equality or not of the final set of cached struct
   // pointers.