Bug 1251797 - Don't fault struct out of rule tree if all of the potential physical property destinations already have a winning value in the cascade. r?heycam
I believe this is useful for cases like having logical properties in the
UA style sheet that are commonly overridden (e.g., margins on lists).
MozReview-Commit-ID: HCnzmhRmN0I
--- a/layout/style/nsCSSDataBlock.cpp
+++ b/layout/style/nsCSSDataBlock.cpp
@@ -290,28 +290,55 @@ EnsurePhysicalProperty(nsCSSProperty aPr
// the physical properties in the logical group array.
static_assert(NS_SIDE_TOP == 0 && NS_SIDE_RIGHT == 1 &&
NS_SIDE_BOTTOM == 2 && NS_SIDE_LEFT == 3,
"unexpected side constant values");
index = side;
}
const nsCSSProperty* props = nsCSSProps::LogicalGroup(aProperty);
+ // Table-length is 1 for single prop, 2 for axis prop, 4 for block prop.
+ size_t len = isSingleProperty ? 1 : (isAxisProperty ? 2 : 4);
#ifdef DEBUG
{
- // Table-length is 1 for single prop, 2 for axis prop, 4 for block prop.
- size_t len = isSingleProperty ? 1 : (isAxisProperty ? 2 : 4);
for (size_t i = 0; i < len; i++) {
MOZ_ASSERT(props[i] != eCSSProperty_UNKNOWN,
"unexpected logical group length");
}
MOZ_ASSERT(props[len] == eCSSProperty_UNKNOWN,
"unexpected logical group length");
}
#endif
+
+ for (size_t i = 0; i < len; i++) {
+ if (aRuleData->ValueFor(props[i])->GetUnit() == eCSSUnit_Null) {
+ // A declaration of one of the logical properties in this logical
+ // group would be the winning declaration in the cascade. This
+ // means that it's reasonably likely that this logical property
+ // could be the winning declaration in the cascade for some values
+ // of writing-mode, direction, and text-orientation. (It doesn't
+ // mean that for sure, though. For example, if this is a
+ // block-start logical property, and all but the bottom physical
+ // property were set. But the common case we want to hit here is
+ // logical declarations that are completely overridden by a
+ // shorthand.)
+ //
+ // If this logical property could be the winning declaration in
+ // the cascade for some values of writing-mode, direction, and
+ // text-orientation, then we have to fault the resulting style
+ // struct out of the rule tree. We can't cache anything on the
+ // rule tree if it depends on data from the style context, since
+ // data cached in the rule tree could be used with a style context
+ // with a different value of the depended-upon data.
+ uint8_t wm = WritingMode(aRuleData->mStyleContext).GetBits();
+ aRuleData->mConditions.SetWritingModeDependency(wm);
+ break;
+ }
+ }
+
return props[index];
}
void
nsCSSCompressedDataBlock::MapRuleInfoInto(nsRuleData *aRuleData) const
{
// If we have no data for these structs, then return immediately.
// This optimization should make us return most of the time, so we
@@ -324,23 +351,16 @@ nsCSSCompressedDataBlock::MapRuleInfoInt
// right property when one can be expressed using both logical and
// physical property names.
for (uint32_t i = mNumProps; i-- > 0; ) {
nsCSSProperty iProp = PropertyAtIndex(i);
if (nsCachedStyleData::GetBitForSID(nsCSSProps::kSIDTable[iProp]) &
aRuleData->mSIDs) {
nsCSSProperty physicalProp = EnsurePhysicalProperty(iProp,
aRuleData);
- if (physicalProp != iProp) {
- // We can't cache anything on the rule tree if we use any data from
- // the style context, since data cached in the rule tree could be
- // used with a style context with a different value.
- uint8_t wm = WritingMode(aRuleData->mStyleContext).GetBits();
- aRuleData->mConditions.SetWritingModeDependency(wm);
- }
nsCSSValue* target = aRuleData->ValueFor(physicalProp);
if (target->GetUnit() == eCSSUnit_Null) {
const nsCSSValue *val = ValueAtIndex(i);
// In order for variable resolution to have the right information
// about the stylesheet level of a value, that level needs to be
// stored on the token stream. We can't do that at creation time
// because the CSS parser (which creates the object) has no idea
// about the stylesheet level, so we do it here instead, where
@@ -787,20 +807,16 @@ nsCSSExpandedDataBlock::MapRuleInfoInto(
nsRuleData* aRuleData) const
{
MOZ_ASSERT(!nsCSSProps::IsShorthand(aPropID));
const nsCSSValue* src = PropertyAt(aPropID);
MOZ_ASSERT(src->GetUnit() != eCSSUnit_Null);
nsCSSProperty physicalProp = EnsurePhysicalProperty(aPropID, aRuleData);
- if (physicalProp != aPropID) {
- uint8_t wm = WritingMode(aRuleData->mStyleContext).GetBits();
- aRuleData->mConditions.SetWritingModeDependency(wm);
- }
nsCSSValue* dest = aRuleData->ValueFor(physicalProp);
MOZ_ASSERT(dest->GetUnit() == eCSSUnit_TokenStream &&
dest->GetTokenStreamValue()->mPropertyID == aPropID);
CSSVariableImageTable::ReplaceAll(aRuleData->mStyleContext, aPropID, [=] {
MapSinglePropertyInto(aPropID, src, physicalProp, dest, aRuleData);
});