Bug 1121768 - Part 3: Look at all subproperties (not just content-visible ones) in nsCSSExpandedDataBlock methods. r=dbaron
authorCameron McCormack <cam@mcc.id.au>
Sat, 17 Jan 2015 15:55:07 +1100
changeset 224391 a1c3ce647646572441f5a17ad9a57ce1274920c4
parent 224390 9a6692c19d67d50cded6b118d057d5362afee622
child 224392 cf403ce5f085b682b80e7751dbcc18a55ae85310
push id54202
push usercmccormack@mozilla.com
push dateSat, 17 Jan 2015 04:56:55 +0000
treeherdermozilla-inbound@cf403ce5f085 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1121768
milestone38.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 1121768 - Part 3: Look at all subproperties (not just content-visible ones) in nsCSSExpandedDataBlock methods. r=dbaron We need to ensure we transfer (or clear) all subproperties of a shorthand that is only enabled in UA style sheets or in certified apps. Otherwise, the shorthand will parse correctly and get stored on the nsCSSExpandedDataBlock but the transfer (or clear) method will skip all of the subproperties.
layout/style/nsCSSDataBlock.cpp
layout/style/nsCSSDataBlock.h
layout/style/nsCSSParser.cpp
--- a/layout/style/nsCSSDataBlock.cpp
+++ b/layout/style/nsCSSDataBlock.cpp
@@ -583,17 +583,17 @@ nsCSSExpandedDataBlock::Clear()
     AssertInitialState();
 }
 
 void
 nsCSSExpandedDataBlock::ClearProperty(nsCSSProperty aPropID)
 {
   if (nsCSSProps::IsShorthand(aPropID)) {
     CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, aPropID,
-                                         nsCSSProps::eEnabledForAllContent) {
+                                         nsCSSProps::eIgnoreEnabledState) {
       ClearLonghandProperty(*p);
     }
   } else {
     ClearLonghandProperty(aPropID);
   }
 }
 
 void
@@ -604,16 +604,17 @@ nsCSSExpandedDataBlock::ClearLonghandPro
     ClearPropertyBit(aPropID);
     ClearImportantBit(aPropID);
     PropertyAt(aPropID)->Reset();
 }
 
 bool
 nsCSSExpandedDataBlock::TransferFromBlock(nsCSSExpandedDataBlock& aFromBlock,
                                           nsCSSProperty aPropID,
+                                          nsCSSProps::EnabledState aEnabledState,
                                           bool aIsImportant,
                                           bool aOverrideImportant,
                                           bool aMustCallValueAppended,
                                           css::Declaration* aDeclaration)
 {
     if (!nsCSSProps::IsShorthand(aPropID)) {
         return DoTransferFromBlock(aFromBlock, aPropID,
                                    aIsImportant, aOverrideImportant,
@@ -621,18 +622,17 @@ nsCSSExpandedDataBlock::TransferFromBloc
     }
 
     // We can pass eIgnoreEnabledState (here, and in ClearProperty above) rather
     // than a value corresponding to whether we're parsing a UA style sheet or
     // certified app because we assert in nsCSSProps::AddRefTable that shorthand
     // properties available in these contexts also have all of their
     // subproperties available in these contexts.
     bool changed = false;
-    CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, aPropID,
-                                         nsCSSProps::eEnabledForAllContent) {
+    CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, aPropID, aEnabledState) {
         changed |= DoTransferFromBlock(aFromBlock, *p,
                                        aIsImportant, aOverrideImportant,
                                        aMustCallValueAppended, aDeclaration);
     }
     return changed;
 }
 
 bool
--- a/layout/style/nsCSSDataBlock.h
+++ b/layout/style/nsCSSDataBlock.h
@@ -237,28 +237,31 @@ public:
      */
     void ClearLonghandProperty(nsCSSProperty aPropID);
 
     /**
      * Transfer the state for |aPropID| (which may be a shorthand)
      * from |aFromBlock| to this block.  The property being transferred
      * is !important if |aIsImportant| is true, and should replace an
      * existing !important property regardless of its own importance
-     * if |aOverrideImportant| is true.
+     * if |aOverrideImportant| is true.  |aEnabledState| is used to
+     * determine which longhand components of |aPropID| (if it is a
+     * shorthand) to transfer.
      *
      * Returns true if something changed, false otherwise.  Calls
      * |ValueAppended| on |aDeclaration| if the property was not
      * previously set, or in any case if |aMustCallValueAppended| is true.
      */
     bool TransferFromBlock(nsCSSExpandedDataBlock& aFromBlock,
-                             nsCSSProperty aPropID,
-                             bool aIsImportant,
-                             bool aOverrideImportant,
-                             bool aMustCallValueAppended,
-                             mozilla::css::Declaration* aDeclaration);
+                           nsCSSProperty aPropID,
+                           nsCSSProps::EnabledState aEnabledState,
+                           bool aIsImportant,
+                           bool aOverrideImportant,
+                           bool aMustCallValueAppended,
+                           mozilla::css::Declaration* aDeclaration);
 
     /**
      * Copies the values for aPropID into the specified aRuleData object.
      *
      * This is used for copying parsed-at-computed-value-time properties
      * that had variable references.  aPropID must be a longhand property.
      */
     void MapRuleInfoInto(nsCSSProperty aPropID, nsRuleData* aRuleData) const;
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -297,28 +297,32 @@ public:
                                            nsRuleData* aRuleData,
                                            nsIURI* aDocURL,
                                            nsIURI* aBaseURL,
                                            nsIPrincipal* aDocPrincipal,
                                            CSSStyleSheet* aSheet,
                                            uint32_t aLineNumber,
                                            uint32_t aLineOffset);
 
-  nsCSSProperty LookupEnabledProperty(const nsAString& aProperty) {
+  nsCSSProps::EnabledState PropertyEnabledState() const {
     static_assert(nsCSSProps::eEnabledForAllContent == 0,
                   "nsCSSProps::eEnabledForAllContent should be zero for "
                   "this bitfield to work");
     nsCSSProps::EnabledState enabledState = nsCSSProps::eEnabledForAllContent;
     if (mUnsafeRulesEnabled) {
       enabledState |= nsCSSProps::eEnabledInUASheets;
     }
     if (mIsChromeOrCertifiedApp) {
       enabledState |= nsCSSProps::eEnabledInChromeOrCertifiedApp;
     }
-    return nsCSSProps::LookupProperty(aProperty, enabledState);
+    return enabledState;
+  }
+
+  nsCSSProperty LookupEnabledProperty(const nsAString& aProperty) {
+    return nsCSSProps::LookupProperty(aProperty, PropertyEnabledState());
   }
 
 protected:
   class nsAutoParseCompoundProperty;
   friend class nsAutoParseCompoundProperty;
 
   class nsAutoFailingSupportsRule;
   friend class nsAutoFailingSupportsRule;
@@ -1526,17 +1530,18 @@ CSSParserImpl::ParseProperty(const nsCSS
     // already a value for this property in the declaration at the
     // same importance level, then we can just copy our parsed value
     // directly into the declaration without going through the whole
     // expand/compress thing.
     if (!aDeclaration->TryReplaceValue(aPropID, aIsImportant, mTempData,
                                        aChanged)) {
       // Do it the slow way
       aDeclaration->ExpandTo(&mData);
-      *aChanged = mData.TransferFromBlock(mTempData, aPropID, aIsImportant,
+      *aChanged = mData.TransferFromBlock(mTempData, aPropID,
+                                          PropertyEnabledState(), aIsImportant,
                                           true, false, aDeclaration);
       aDeclaration->CompressFrom(&mData);
     }
     CLEAR_ERROR();
   }
 
   mTempData.AssertInitialState();
 
@@ -6589,16 +6594,17 @@ CSSParserImpl::ParseDeclaration(css::Dec
     MOZ_ASSERT(Substring(propertyName, 0,
                          CSS_CUSTOM_NAME_PREFIX_LENGTH).EqualsLiteral("--"));
     // remove '--'
     nsDependentString varName(propertyName, CSS_CUSTOM_NAME_PREFIX_LENGTH);
     aDeclaration->AddVariableDeclaration(varName, variableType, variableValue,
                                          status == ePriority_Important, false);
   } else {
     *aChanged |= mData.TransferFromBlock(mTempData, propID,
+                                         PropertyEnabledState(),
                                          status == ePriority_Important,
                                          false, aMustCallValueAppended,
                                          aDeclaration);
   }
 
   return true;
 }