Bug 1188721 - Part 14: Stop storing style struct ownership data in nsStyleContext::mBits. r?dbaron draft
authorCameron McCormack <cam@mcc.id.au>
Wed, 23 Mar 2016 17:35:59 +1100
changeset 343764 459325bd1a2befed2801fb96c68fb03b89c579d8
parent 343763 af51dc1e4396225a81ac8f9696d5538bc80f81af
child 343765 30859ee75f010cece5a398b89548319f06b4c656
push id13680
push usercmccormack@mozilla.com
push dateWed, 23 Mar 2016 06:36:18 +0000
reviewersdbaron
bugs1188721
milestone48.0a1
Bug 1188721 - Part 14: Stop storing style struct ownership data in nsStyleContext::mBits. r?dbaron MozReview-Commit-ID: 2ykmOezStOp
layout/style/nsRuleNode.cpp
layout/style/nsRuleNode.h
layout/style/nsStyleContext.cpp
layout/style/nsStyleContext.h
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -80,18 +80,16 @@ nsConditionalResetStyleData::GetConditio
   MOZ_ASSERT(e, "if mConditionalBits bit is set, we must have at least one "
                 "conditional style struct");
   do {
     if (e->mConditions.Matches(aStyleContext)) {
       nsStyleStruct* data = e->mStyleStruct;
 
       // For reset structs with conditions, we cache the data on the
       // style context.
-      // Tell the style context that it doesn't own the data
-      aStyleContext->AddStyleBit(GetBitForSID(aSID));
       aStyleContext->SetStyle(aSID, data);
 
       return data;
     }
     e = e->mNext;
   } while (e);
   return nullptr;
 }
@@ -2367,21 +2365,16 @@ nsRuleNode::WalkRuleTree(const nsStyleSt
 
   if (detail == eRuleNone && startStruct) {
     // We specified absolutely no rule information, but a parent rule in the tree
     // specified all the rule information.  We set a bit along the branch from our
     // node in the tree to the node that specified the data that tells nodes on that
     // branch that they never need to examine their rules for this particular struct type
     // ever again.
     PropagateDependentBit(aSID, ruleNode, startStruct);
-    // For inherited structs, mark the struct (which will be set on
-    // the context by our caller) as not being owned by the context.
-    if (!isReset) {
-      aContext->AddStyleBit(nsCachedStyleData::GetBitForSID(aSID));
-    }
     return startStruct;
   }
   if ((!startStruct && !isReset &&
        (detail == eRuleNone || detail == eRulePartialInherited)) ||
       detail == eRuleFullInherited) {
     // We specified no non-inherited information and neither did any of
     // our parent rules.
 
@@ -2411,17 +2404,16 @@ nsRuleNode::WalkRuleTree(const nsStyleSt
       }
     }
     if (parentContext) {
       // We have a parent, and so we should just inherit from the parent.
       // Set the inherit bits on our context.  These bits tell the style context that
       // it never has to go back to the rule tree for data.  Instead the style context tree
       // should be walked to find the data.
       const nsStyleStruct* parentStruct = parentContext->StyleData(aSID);
-      aContext->AddStyleBit(bit); // makes const_cast OK.
       aContext->SetStyle(aSID, const_cast<nsStyleStruct*>(parentStruct));
       return parentStruct;
     }
     else
       // We are the root.  In the case of fonts, the default values just
       // come from the pres context.
       return SetDefaultOnRoot(aSID, aContext);
   }
@@ -2737,18 +2729,16 @@ nsRuleNode::SetDefaultOnRoot(const nsSty
     if (!aHighestNode->mStyleData.mInheritedData) {                           \
       aHighestNode->mStyleData.mInheritedData =                               \
         new (mPresContext) nsInheritedStyleData;                              \
     }                                                                         \
     aHighestNode->mStyleData.mInheritedData->                                 \
       SetStyleData(eStyleStruct_##type_, data_);                              \
     /* Propagate the bit down. */                                             \
     PropagateDependentBit(eStyleStruct_##type_, aHighestNode, data_);         \
-    /* Tell the style context that it doesn't own the data */                 \
-    aContext->AddStyleBit(NS_STYLE_INHERIT_BIT(type_));                       \
   }                                                                           \
   /* For inherited structs, our caller will cache the data on the */          \
   /* style context */                                                         \
                                                                               \
   return data_;
 
 /**
  * End an nsRuleNode::Compute*Data function for a reset struct.
@@ -2776,18 +2766,16 @@ nsRuleNode::SetDefaultOnRoot(const nsSty
     /* Propagate the bit down. */                                             \
     PropagateDependentBit(eStyleStruct_##type_, aHighestNode, data_);         \
   } else if (conditions.Cacheable()) {                                        \
     if (!mStyleData.mResetData) {                                             \
       mStyleData.mResetData = new (mPresContext) nsConditionalResetStyleData; \
     }                                                                         \
     mStyleData.mResetData->                                                   \
       SetStyleData(eStyleStruct_##type_, mPresContext, data_, conditions);    \
-    /* Tell the style context that it doesn't own the data */                 \
-    aContext->AddStyleBit(NS_STYLE_INHERIT_BIT(type_));                       \
     aContext->SetStyle(eStyleStruct_##type_, data_);                          \
   } else {                                                                    \
     /* We can't be cached in the rule node.  We have to be put right */       \
     /* on the style context. */                                               \
     aContext->SetStyle(eStyleStruct_##type_, data_);                          \
     if (aContext->GetParent()) {                                              \
       /* This is pessimistic; we could be uncacheable because we had a */     \
       /* relative font-weight, for example, which does not need to defeat */  \
@@ -10037,21 +10025,16 @@ nsRuleNode::GetStyleData(nsStyleStructID
 
   const nsStyleStruct* data;
 
   // Never use cached data for animated style inside a pseudo-element;
   // see comment on cacheability in AnimValuesStyleRule::MapRuleInfoInto.
   if (!(HasAnimationData() && ParentHasPseudoElementData(aContext))) {
     data = mStyleData.GetStyleData(aSID, aContext, aComputeData);
     if (MOZ_LIKELY(data != nullptr)) {
-      // For inherited structs, mark the struct (which will be set on
-      // the context by our caller) as not being owned by the context.
-      if (!nsCachedStyleData::IsReset(aSID)) {
-        aContext->AddStyleBit(nsCachedStyleData::GetBitForSID(aSID));
-      }
       return data; // We have a fully specified struct. Just return it.
     }
   }
 
   if (MOZ_UNLIKELY(!aComputeData))
     return nullptr;
 
   // Nothing is cached.  We'll have to delve further and examine our rules.
--- a/layout/style/nsRuleNode.h
+++ b/layout/style/nsRuleNode.h
@@ -950,37 +950,32 @@ public:
                                     bool aComputeData);
 
 
   // See comments in GetStyleData for an explanation of what the
   // code below does.
   #define STYLE_STRUCT_INHERITED(name_, checkdata_cb_)                        \
   template<bool aComputeData>                                                 \
   const nsStyle##name_*                                                       \
-  GetStyle##name_(nsStyleContext* aContext, uint64_t& aContextStyleBits)      \
+  GetStyle##name_(nsStyleContext* aContext)                                   \
   {                                                                           \
     NS_ASSERTION(IsUsedDirectly(),                                            \
                  "if we ever call this on rule nodes that aren't used "       \
                  "directly, we should adjust handling of mDependentBits "     \
                  "in some way.");                                             \
     MOZ_ASSERT(!ContextHasCachedData(aContext, eStyleStruct_##name_),         \
                "style context should not have cached data for struct");       \
                                                                               \
     const nsStyle##name_ *data;                                               \
                                                                               \
     /* Never use cached data for animated style inside a pseudo-element; */   \
     /* see comment on cacheability in AnimValuesStyleRule::MapRuleInfoInto */ \
     if (!(HasAnimationData() && ParentHasPseudoElementData(aContext))) {      \
       data = mStyleData.GetStyle##name_();                                    \
       if (data != nullptr) {                                                  \
-        /* For inherited structs, mark the struct (which will be set on */    \
-        /* the context by our caller) as not being owned by the context. */   \
-        /* Normally this would be aContext->AddStyleBit(), but aContext is */ \
-        /* an incomplete type here, so we work around that with a param. */   \
-        aContextStyleBits |= NS_STYLE_INHERIT_BIT(name_);                     \
         /* Our caller will cache the data on the style context. */            \
         return data;                                                          \
       }                                                                       \
     }                                                                         \
                                                                               \
     if (!aComputeData)                                                        \
       return nullptr;                                                         \
                                                                               \
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -418,18 +418,17 @@ const nsStyleStruct*
 nsStyleContext::StyleData(nsStyleStructID aSID)
 {
   const nsStyleStruct* cachedData = GetCachedStyleData(aSID);
   if (cachedData)
     return cachedData; // We have computed data stored on this node in the context tree.
   // Our rule node will take care of it for us.
   const nsStyleStruct* newData = mRuleNode->GetStyleData(aSID, this, true);
   if (!nsCachedStyleData::IsReset(aSID)) {
-    // always cache inherited data on the style context; the rule
-    // node set the bit in mBits for us if needed.
+    // Always cache inherited data on the style context.
     mCachedInheritedData.SetStyleData(aSID, const_cast<nsStyleStruct*>(newData));
   }
   return newData;
 }
 
 // This is an evil evil function, since it forces you to alloc your own separate copy of
 // style data!  Do not use this function unless you absolutely have to!  You should avoid
 // this at all costs! -dwh
@@ -478,28 +477,25 @@ nsStyleContext::GetUniqueStyleData(const
 
 #undef UNIQUE_CASE
 
   default:
     NS_ERROR("Struct type not supported.  Please find another way to do this if you can!");
     return nullptr;
   }
 
-  mBits &= ~static_cast<uint64_t>(nsCachedStyleData::GetBitForSID(aSID));
-
   return newData;
 }
 
 // This is an evil function, but less evil than GetUniqueStyleData. It
 // creates an empty style struct for this nsStyleContext.
 nsStyleStruct*
 nsStyleContext::CreateEmptyStyleData(const nsStyleStructID& aSID)
 {
   MOZ_ASSERT(!mChild && !mEmptyChild &&
-             !(mBits & nsCachedStyleData::GetBitForSID(aSID)) &&
              !GetCachedStyleData(aSID),
              "This style should not have been computed");
 
   nsStyleStruct* result;
   nsPresContext* presContext = PresContext();
   switch (aSID) {
 #define UNIQUE_CASE(c_, ...) \
     case eStyleStruct_##c_: \
@@ -511,19 +507,16 @@ nsStyleContext::CreateEmptyStyleData(con
 
 #undef UNIQUE_CASE
 
   default:
     NS_ERROR("Struct type not supported.");
     return nullptr;
   }
 
-  // The new struct is owned by this style context, but that we don't
-  // need to clear the bit in mBits because we've asserted that at the
-  // top of this function.
   SetStyle(aSID, result);
   return result;
 }
 
 void
 nsStyleContext::SetStyle(nsStyleStructID aSID, nsStyleStruct* aStruct)
 {
   // This method should only be called from nsRuleNode!  It is not a public
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -495,19 +495,18 @@ private:
       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. */                 \
+        mRuleNode->GetStyle##name_<aComputeData>(this);                 \
+      /* always cache inherited data on the style context */            \
       mCachedInheritedData.SetStyleData(eStyleStruct_##name_,           \
         const_cast<nsStyle##name_ *>(newData));                         \
       return newData;                                                   \
     }
   #define STYLE_STRUCT_RESET(name_, checkdata_cb_)                      \
     template<bool aComputeData>                                         \
     const nsStyle##name_ * DoGetStyle##name_() {                        \
       if (mCachedResetData) {                                           \
@@ -568,33 +567,22 @@ private:
   // (the "rule tree") that indicates which style rules are used to
   // compute the style data, and in what cascading order.  The least
   // specific rule matched is the one whose rule node is a child of the
   // 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.
+  // were created for this style context, inherited from the parent style
+  // context, or came from the rule node.
   nsResetStyleData*       mCachedResetData; // Cached reset style data.
   nsInheritedStyleData    mCachedInheritedData; // Cached inherited style data
 
-  // mBits stores a number of things:
-  //  - It records (using the style struct bits) which structs are
-  //    inherited from the parent context or owned by mRuleNode (i.e.,
-  //    not owned by the style context).
-  //  - It also stores the additional bits listed at the top of
-  //    nsStyleStruct.h.
+  // mBits 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