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 draft
authorL. David Baron <dbaron@dbaron.org>
Sun, 13 Mar 2016 19:47:26 -0700
changeset 339875 36f81aa9a4875d56fc95e2587803935513fb2f9c
parent 339874 f0c0480732d36153e8839c7f17394d45f679f87d
child 516059 af24b5a567075ed71bd98cea49c6c6b3c8a559df
push id12811
push userdbaron@mozilla.com
push dateMon, 14 Mar 2016 02:47:45 +0000
reviewersheycam
bugs1251797
milestone48.0a1
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
layout/style/nsCSSDataBlock.cpp
--- 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);
   });