Bug 1209603 patch 6 - Prepare to use a different meaning of mBits when cached style data pointer is null. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Mon, 19 Oct 2015 20:42:28 -0700
changeset 301859 565c13fd9230597dd9cda4cb1c2fe66bc4f89f34
parent 301858 415be6e995fdac614a00a85554076e2f3073d1ec
child 301860 fcd8749d260ccb60fe7f8e07303ce40391652a29
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 6 - Prepare to use a different meaning of mBits when cached style data pointer is null. r=heycam We currently only use the style struct bits in mBits when the style context has the relevant struct cached. The bit being set indicates whether or not the context owns the struct. This patch conditions the necessary tests on a cached struct being present so that we can use (for reset structs, i.e., those with non-inherited properties) mBits to mean something different when the cached storage is null.
layout/style/nsStyleContext.cpp
layout/style/nsStyleContext.h
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -1351,17 +1351,17 @@ nsStyleContext::SwapStyleData(nsStyleCon
        i < nsStyleStructID_Inherited_Start + nsStyleStructID_Inherited_Count;
        i = nsStyleStructID(i + 1)) {
     uint32_t bit = nsCachedStyleData::GetBitForSID(i);
     if (!(aStructs & bit)) {
       continue;
     }
     void*& thisData = mCachedInheritedData.mStyleStructs[i];
     void*& otherData = aNewContext->mCachedInheritedData.mStyleStructs[i];
-    if (mBits & bit) {
+    if ((mBits & bit) && thisData) {
       if (thisData == otherData) {
         thisData = nullptr;
       }
     } else if (!(aNewContext->mBits & bit) && thisData && otherData) {
       std::swap(thisData, otherData);
     }
   }
 
@@ -1376,17 +1376,17 @@ nsStyleContext::SwapStyleData(nsStyleCon
       mCachedResetData = new (mRuleNode->PresContext()) nsResetStyleData;
     }
     if (!aNewContext->mCachedResetData) {
       aNewContext->mCachedResetData =
         new (mRuleNode->PresContext()) nsResetStyleData;
     }
     void*& thisData = mCachedResetData->mStyleStructs[i];
     void*& otherData = aNewContext->mCachedResetData->mStyleStructs[i];
-    if (mBits & bit) {
+    if ((mBits & bit) && thisData) {
       if (thisData == otherData) {
         thisData = nullptr;
       }
     } else if (!(aNewContext->mBits & bit) && thisData && otherData) {
       std::swap(thisData, otherData);
     }
   }
 }
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -277,21 +277,25 @@ public:
       slot = aStruct;                                                       \
     }
 #define STYLE_STRUCT_RESET(name_, checkdata_cb_) /* nothing */
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT_RESET
   #undef STYLE_STRUCT_INHERITED
 
   /**
-   * Returns whether this style context has cached, inherited style data for a
-   * given style struct.
+   * Returns whether this style context has cached style data for a
+   * given style struct and it does NOT own that struct.  This can
+   * happen because it was inherited from the parent style context, or
+   * because it was stored conditionally on the rule node.
    */
-  bool HasCachedInheritedStyleData(nsStyleStructID aSID)
-    { return mBits & nsCachedStyleData::GetBitForSID(aSID); }
+  bool HasCachedInheritedStyleData(nsStyleStructID aSID) {
+    return (mBits & nsCachedStyleData::GetBitForSID(aSID)) &&
+           GetCachedStyleData(aSID);
+  }
 
   nsRuleNode* RuleNode() { return mRuleNode; }
   void AddStyleBit(const uint64_t& aBit) { mBits |= aBit; }
 
   /*
    * Mark this style context's rule node (and its ancestors) to prevent
    * it from being garbage collected.
    */
@@ -597,25 +601,33 @@ private:
   // root of the rule tree, and the most specific rule matched is the
   // |mRule| member of |mRuleNode|.
   nsRuleNode* const       mRuleNode;
 
   // mCachedInheritedData and mCachedResetData point to both structs that
   // are owned by this style context and structs that are owned by one of
   // this style context's ancestors (which are indirectly owned since this
   // style context owns a reference to its parent).  If the bit in |mBits|
-  // is set for a struct, that means that the pointer for that struct is
-  // owned by an ancestor or by mRuleNode rather than by this style context.
-  // Since style contexts typically have some inherited data but only sometimes
-  // have reset data, we always allocate the mCachedInheritedData, but only
-  // sometimes allocate the mCachedResetData.
+  // is set for a non-null struct, that means that the pointer for that
+  // struct is owned by an ancestor or by mRuleNode rather than by this
+  // style context.  Since style contexts typically have some inherited
+  // data but only sometimes have reset data, we always allocate the
+  // mCachedInheritedData, but only sometimes allocate the
+  // mCachedResetData.
   nsResetStyleData*       mCachedResetData; // Cached reset style data.
   nsInheritedStyleData    mCachedInheritedData; // Cached inherited style data
-  uint64_t                mBits; // Which structs are inherited from the
-                                 // parent context or owned by mRuleNode.
+
+  // mBits stores a number of things:
+  //  - For all structs, when they are non-null in the style context's
+  //    storage, it records (using the style struct bits) which structs
+  //    are inherited from the parent context or owned by mRuleNode.
+  //  - It also stores the additional bits listed at the top of
+  //    nsStyleStruct.h.
+  uint64_t                mBits;
+
   uint32_t                mRefCnt;
 
 #ifdef DEBUG
   uint32_t                mFrameRefCnt; // number of frames that use this
                                         // as their style context
 
   nsStyleStructID         mComputingStruct;