Bug 1209603 patch 10 - Make PeekStyle* exact, i.e., guaranteed to return null if we haven't computed the data for this context. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Mon, 19 Oct 2015 20:42:29 -0700
changeset 303639 b2c58421d657b4de854528dcd1c81fda17acddc5
parent 303638 0f3e25c8e4b1a37d809d4e777edfa4d10df5cf60
child 303640 bb679653777502d9099a403520ed8b17da936e3f
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [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 10 - Make PeekStyle* exact, i.e., guaranteed to return null if we haven't computed the data for this context. r=heycam This fixes the bug because it prevents a cache conditions check for a later PeekStyle* in nsStyleContext::CalcStyleDifference from computing a struct that was null when we checked it earlier in CalcStyleDifference. This, in turn, could allow the old context to be retained (and reparented to the new parent) even though it now has incorrect data in the font or visibility struct that was computed while checking the conditions for another struct. This should also improve performance in some cases of style changes on not-yet-presented frames because we have fewer change hints to process.
layout/reftests/bugs/reftest.list
layout/style/nsStyleContext.h
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1931,9 +1931,9 @@ skip-if(B2G||Mulet) == 1150021-1.xul 115
 == 1169331-1.html 1169331-1-ref.html
 fuzzy(1,74) fuzzy-if(gtkWidget,6,79) == 1174332-1.html 1174332-1-ref.html
 == 1179078-1.html 1179078-1-ref.html
 == 1179288-1.html 1179288-1-ref.html
 == 1190635-1.html 1190635-1-ref.html
 == 1202512-1.html 1202512-1-ref.html
 == 1202512-2.html 1202512-2-ref.html
 != 1207326-1.html about:blank
-fails == 1209603-1.html 1209603-1-ref.html # bug 1209603
+== 1209603-1.html 1209603-1-ref.html
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -528,16 +528,21 @@ private:
   #define STYLE_STRUCT_INHERITED(name_, checkdata_cb_)                  \
     template<bool aComputeData>                                         \
     const nsStyle##name_ * DoGetStyle##name_() {                        \
       const nsStyle##name_ * cachedData =                               \
         static_cast<nsStyle##name_*>(                                   \
           mCachedInheritedData.mStyleStructs[eStyleStruct_##name_]);    \
       if (cachedData) /* Have it cached already, yay */                 \
         return cachedData;                                              \
+      if (!aComputeData) {                                              \
+        /* We always cache inherited structs on the context when we */  \
+        /* compute them. */                                             \
+        return nullptr;                                                 \
+      }                                                                 \
       /* Have the rulenode deal */                                      \
       AUTO_CHECK_DEPENDENCY(eStyleStruct_##name_);                      \
       const nsStyle##name_ * newData =                                  \
         mRuleNode->GetStyle##name_<aComputeData>(this, mBits);          \
       /* always cache inherited data on the style context; the rule */  \
       /* node set the bit in mBits for us if needed. */                 \
       mCachedInheritedData.mStyleStructs[eStyleStruct_##name_] =        \
         const_cast<nsStyle##name_ *>(newData);                          \
@@ -548,16 +553,21 @@ private:
     const nsStyle##name_ * DoGetStyle##name_() {                        \
       if (mCachedResetData) {                                           \
         const nsStyle##name_ * cachedData =                             \
           static_cast<nsStyle##name_*>(                                 \
             mCachedResetData->mStyleStructs[eStyleStruct_##name_]);     \
         if (cachedData) /* Have it cached already, yay */               \
           return cachedData;                                            \
       }                                                                 \
+      if (!aComputeData && !(mBits & NS_STYLE_INHERIT_BIT(name_))) {    \
+        /* When we compute reset structs, we either cache them on */    \
+        /* the style context or set the bit in mBits. */                \
+        return nullptr;                                                 \
+      }                                                                 \
       /* Have the rulenode deal */                                      \
       AUTO_CHECK_DEPENDENCY(eStyleStruct_##name_);                      \
       return mRuleNode->GetStyle##name_<aComputeData>(this, mBits);     \
     }
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT_RESET
   #undef STYLE_STRUCT_INHERITED