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
authorL. David Baron <dbaron@dbaron.org>
Mon, 14 Mar 2016 10:27:05 -0700
changeset 288608 718aa12e555b11e5980ea59929bf37c7416fd1db
parent 288607 404a3f74b2793339294cbdfd522df47d612192a3
child 288609 d0fa0d3838807de31e2d4dbb1deb6cfd981bcb82
push id18174
push usercbook@mozilla.com
push dateTue, 15 Mar 2016 09:44:58 +0000
treeherderfx-team@dd0baa33759d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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: KxojbfMYq0f
layout/style/nsCSSDataBlock.cpp
--- a/layout/style/nsCSSDataBlock.cpp
+++ b/layout/style/nsCSSDataBlock.cpp
@@ -290,28 +290,53 @@ 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,
+  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 (but maybe not aProperty) 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 +349,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 +805,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);
   });