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 288602 718aa12e555b11e5980ea59929bf37c7416fd1db
parent 288601 404a3f74b2793339294cbdfd522df47d612192a3
child 288603 d0fa0d3838807de31e2d4dbb1deb6cfd981bcb82
push id30087
push usercbook@mozilla.com
push dateTue, 15 Mar 2016 09:43:43 +0000
treeherdermozilla-central@5e14887312d4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1251797
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);
   });