Bug 1188074 part 1 - Refactor ParseImageLayersItem a bit to check whether a slot is filled before checking whether the keyword matches. r=heycam draft
authorXidorn Quan <me@upsuper.org>
Thu, 25 May 2017 13:38:47 +1000
changeset 584325 fb142b903a098621f5db442f4f43ac4adf1dfa04
parent 584270 3f15d3bf1562e175ad11ae7d29d9bbbe3d2b3d48
child 584326 dab47404b74e3a5bb74488ac2e8ac656afb967b1
push id60691
push userxquan@mozilla.com
push dateThu, 25 May 2017 08:48:10 +0000
reviewersheycam
bugs1188074
milestone55.0a1
Bug 1188074 part 1 - Refactor ParseImageLayersItem a bit to check whether a slot is filled before checking whether the keyword matches. r=heycam This matches how Servo handles this, which should be faster when the value is valid, because we can skip checking lots of keyword tables in many cases. MozReview-Commit-ID: C1pGwbKTi0c
layout/style/nsCSSParser.cpp
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -12325,72 +12325,66 @@ CSSParserImpl::ParseImageLayersItem(
 
     if (tt == eCSSToken_Ident) {
       nsCSSKeyword keyword = nsCSSKeywords::LookupKeyword(mToken.mIdent);
       int32_t dummy;
       if (keyword == eCSSKeyword_inherit ||
           keyword == eCSSKeyword_initial ||
           keyword == eCSSKeyword_unset) {
         return false;
-      } else if (keyword == eCSSKeyword_none) {
-        if (haveImage)
-          return false;
+      } else if (!haveImage && keyword == eCSSKeyword_none) {
         haveImage = true;
         if (ParseSingleValueProperty(aState.mImage->mValue,
                                      aTable[nsStyleImageLayers::image]) !=
             CSSParseResult::Ok) {
           NS_NOTREACHED("should be able to parse");
           return false;
         }
-      } else if (aTable[nsStyleImageLayers::attachment] !=
+      } else if (!haveAttach &&
+                 aTable[nsStyleImageLayers::attachment] !=
                    eCSSProperty_UNKNOWN &&
-                 nsCSSProps::FindKeyword(keyword,
-                   nsCSSProps::kImageLayerAttachmentKTable, dummy)) {
-        if (haveAttach)
-          return false;
+                 nsCSSProps::FindKeyword(
+                   keyword, nsCSSProps::kImageLayerAttachmentKTable, dummy)) {
         haveAttach = true;
         if (ParseSingleValueProperty(aState.mAttachment->mValue,
                                      aTable[nsStyleImageLayers::attachment]) !=
             CSSParseResult::Ok) {
           NS_NOTREACHED("should be able to parse");
           return false;
         }
-      } else if (nsCSSProps::FindKeyword(keyword,
-                   nsCSSProps::kImageLayerRepeatKTable, dummy)) {
-        if (haveRepeat)
-          return false;
+      } else if (!haveRepeat &&
+                 nsCSSProps::FindKeyword(
+                   keyword, nsCSSProps::kImageLayerRepeatKTable, dummy)) {
         haveRepeat = true;
         nsCSSValuePair scratch;
         if (!ParseImageLayerRepeatValues(scratch)) {
           NS_NOTREACHED("should be able to parse");
           return false;
         }
         aState.mRepeat->mXValue = scratch.mXValue;
         aState.mRepeat->mYValue = scratch.mYValue;
-      } else if (nsCSSProps::FindKeyword(keyword,
+      } else if (!havePositionAndSize &&
+                 nsCSSProps::FindKeyword(keyword,
                    nsCSSProps::kImageLayerPositionKTable, dummy)) {
-        if (havePositionAndSize)
-          return false;
         havePositionAndSize = true;
 
         if (!ParsePositionValueSeparateCoords(aState.mPositionX->mValue,
                                               aState.mPositionY->mValue)) {
           return false;
         }
         if (ExpectSymbol('/', true)) {
           nsCSSValuePair scratch;
           if (!ParseImageLayerSizeValues(scratch)) {
             return false;
           }
           aState.mSize->mXValue = scratch.mXValue;
           aState.mSize->mYValue = scratch.mYValue;
         }
-      } else if (nsCSSProps::FindKeyword(keyword, originTable, dummy)) {
-        if (haveOrigin)
-          return false;
+      } else if (!haveOrigin &&
+                 nsCSSProps::FindKeyword(keyword, originTable, dummy)) {
         haveOrigin = true;
         if (ParseSingleValueProperty(aState.mOrigin->mValue,
                                      aTable[nsStyleImageLayers::origin]) !=
             CSSParseResult::Ok) {
           NS_NOTREACHED("should be able to parse");
           return false;
         }
 
@@ -12413,43 +12407,41 @@ CSSParserImpl::ParseImageLayersItem(
                    "how can failing to parse a single background-clip value "
                    "consume tokens?");
         if (result == CSSParseResult::NotFound) {
           // When exactly one <box> value is set, it is used for both
           // 'background-origin' and 'background-clip'.
           // See assertions above showing these values are compatible.
           aState.mClip->mValue = aState.mOrigin->mValue;
         }
-      } else if (aTable[nsStyleImageLayers::composite] != eCSSProperty_UNKNOWN &&
-                 nsCSSProps::FindKeyword(keyword,
-                   nsCSSProps::kImageLayerCompositeKTable, dummy)) {
-        if (haveComposite)
-          return false;
+      } else if (!haveComposite &&
+                 aTable[nsStyleImageLayers::composite] !=
+                   eCSSProperty_UNKNOWN &&
+                 nsCSSProps::FindKeyword(
+                   keyword, nsCSSProps::kImageLayerCompositeKTable, dummy)) {
         haveComposite = true;
         if (ParseSingleValueProperty(aState.mComposite->mValue,
                                      aTable[nsStyleImageLayers::composite]) !=
             CSSParseResult::Ok) {
           NS_NOTREACHED("should be able to parse");
           return false;
         }
-      } else if (aTable[nsStyleImageLayers::maskMode] != eCSSProperty_UNKNOWN &&
-                 nsCSSProps::FindKeyword(keyword,
-                   nsCSSProps::kImageLayerModeKTable, dummy)) {
-        if (haveMode)
-          return false;
+      } else if (!haveMode &&
+                 aTable[nsStyleImageLayers::maskMode] != eCSSProperty_UNKNOWN &&
+                 nsCSSProps::FindKeyword(
+                   keyword, nsCSSProps::kImageLayerModeKTable, dummy)) {
         haveMode = true;
         if (ParseSingleValueProperty(aState.mMode->mValue,
                                      aTable[nsStyleImageLayers::maskMode]) !=
             CSSParseResult::Ok) {
           NS_NOTREACHED("should be able to parse");
           return false;
         }
-      } else if (aTable[nsStyleImageLayers::color] != eCSSProperty_UNKNOWN) {
-        if (haveColor)
-          return false;
+      } else if (!haveColor &&
+                 aTable[nsStyleImageLayers::color] != eCSSProperty_UNKNOWN) {
         haveColor = true;
         if (ParseSingleValueProperty(aState.mColor,
                                      aTable[nsStyleImageLayers::color]) !=
                                        CSSParseResult::Ok) {
           return false;
         }
       } else {
         return false;