Bug 1188721 - Part 14: Stop storing style struct ownership data in nsStyleContext::mBits. r?dbaron
MozReview-Commit-ID: 2ykmOezStOp
--- 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