Bug 983175 - Part 1: Refactor ParseGridLineNames (CSS Grid). r=dholbert
authorSimon Sapin <simon.sapin@exyr.org>
Thu, 27 Mar 2014 11:54:40 -0400
changeset 175655 63d2474f84f8d67744b7b3c829cf309d9b6e003a
parent 175654 acba6fd6fb2c1843beb5626308ceb31a6303fd0a
child 175656 038c28096bfe5e0bf958f83d5aaee40f9ecc015e
push id26496
push userkwierso@gmail.com
push dateFri, 28 Mar 2014 02:28:34 +0000
treeherdermozilla-central@3c09159e01da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs983175
milestone31.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 983175 - Part 1: Refactor ParseGridLineNames (CSS Grid). r=dholbert Return a CSSParseResult rather than a bool to distinguish between an empty list of names '()' and the lack of a list.
layout/style/nsCSSParser.cpp
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -641,24 +641,27 @@ protected:
   // For "flex" shorthand property, defined in CSS Flexbox spec
   bool ParseFlex();
   // For "flex-flow" shorthand property, defined in CSS Flexbox spec
   bool ParseFlexFlow();
 
   // CSS Grid
   bool ParseGridAutoFlow();
 
-  // Parse an optional <line-names> expression.
-  // If successful, leave aValue untouched,
+  // Parse a <line-names> expression.
+  // If successful, either leave aValue untouched,
   // to indicate that we parsed the empty list,
   // or set it to a eCSSUnit_List of eCSSUnit_Ident.
-  // Not finding an open paren is considered the same as an empty list.
+  //
+  // To parse an optional <line-names> (ie. if not finding an open paren
+  // is considered the same as an empty list),
+  // treat CSSParseResult::NotFound the same as CSSParseResult::Ok.
   //
   // If aValue is already a eCSSUnit_List, append to that list.
-  bool ParseGridLineNames(nsCSSValue& aValue);
+  CSSParseResult ParseGridLineNames(nsCSSValue& aValue);
   bool ParseGridTrackBreadth(nsCSSValue& aValue);
   CSSParseResult ParseGridTrackSize(nsCSSValue& aValue);
   bool ParseGridAutoColumnsRows(nsCSSProperty aPropID);
 
   // Assuming a [ <line-names>? ] has already been parsed,
   // parse the rest of a <track-list>.
   //
   // This exists because [ <line-names>? ] is ambiguous in the
@@ -6953,26 +6956,26 @@ CSSParserImpl::ParseGridAutoFlow()
       bitField != NS_STYLE_GRID_AUTO_FLOW_NONE) {
     return false;
   }
 
   AppendValue(eCSSProperty_grid_auto_flow, value);
   return true;
 }
 
-bool
+CSSParseResult
 CSSParserImpl::ParseGridLineNames(nsCSSValue& aValue)
 {
   if (!ExpectSymbol('(', true)) {
-    return true;
+    return CSSParseResult::NotFound;
   }
   if (!GetToken(true) || mToken.IsSymbol(')')) {
-    return true;
-  }
-  // 'return true' so far keeps eCSSUnit_Null, to represent an empty list.
+    return CSSParseResult::Ok;
+  }
+  // 'return' so far leaves aValue untouched, to represent an empty list.
 
   nsCSSValueList* item;
   if (aValue.GetUnit() == eCSSUnit_List) {
     // Find the end of an existing list.
     // The grid-template shorthand uses this, at most once for a given list.
 
     // NOTE: we could avoid this traversal by somehow keeping around
     // a pointer to the last item from the previous call.
@@ -6987,20 +6990,20 @@ CSSParserImpl::ParseGridLineNames(nsCSSV
     MOZ_ASSERT(aValue.GetUnit() == eCSSUnit_Null, "Unexpected unit");
     item = aValue.SetListValue();
   }
   for (;;) {
     if (!(eCSSToken_Ident == mToken.mType &&
           ParseCustomIdent(item->mValue, mToken.mIdent))) {
       UngetToken();
       SkipUntil(')');
-      return false;
+      return CSSParseResult::Error;
     }
     if (!GetToken(true) || mToken.IsSymbol(')')) {
-      return true;
+      return CSSParseResult::Ok;
     }
     item->mNext = new nsCSSValueList;
     item = item->mNext;
   }
 }
 
 // Parse a <track-breadth>
 bool
@@ -7080,17 +7083,17 @@ CSSParserImpl::ParseGridTrackListWithFir
     // so even CSSParseResult::NotFound is an error here.
     return false;
   }
 
   nsCSSValueList* item = firstTrackSizeItem;
   for (;;) {
     item->mNext = new nsCSSValueList;
     item = item->mNext;
-    if (!ParseGridLineNames(item->mValue)) {
+    if (ParseGridLineNames(item->mValue) == CSSParseResult::Error) {
       return false;
     }
 
     // FIXME: add repeat()
     nsCSSValue trackSize;
     CSSParseResult result = ParseGridTrackSize(trackSize);
     if (result == CSSParseResult::Error) {
       return false;
@@ -7121,18 +7124,18 @@ CSSParserImpl::ParseGridTemplateColumnsR
   nsCSSValue value;
   if (ParseVariant(value, VARIANT_INHERIT | VARIANT_NONE, nullptr)) {
     AppendValue(aPropID, value);
     return true;
   }
   // FIXME (bug 983175): add subgrid parsing
 
   nsCSSValue firstLineNames;
-  if (!(ParseGridLineNames(firstLineNames) &&
-        ParseGridTrackListWithFirstLineNames(value, firstLineNames))) {
+  if (ParseGridLineNames(firstLineNames) == CSSParseResult::Error ||
+      !ParseGridTrackListWithFirstLineNames(value, firstLineNames)) {
     return false;
   }
   AppendValue(aPropID, value);
   return true;
 }
 
 bool
 CSSParserImpl::ParseGridTemplateAreasLine(const nsAutoString& aInput,
@@ -7279,18 +7282,18 @@ CSSParserImpl::ParseGridTemplate()
 
   // TODO (bug 983175): add parsing for subgrid
   // as part of <'grid-template-columns'>
 
   // [ <line-names>? ] here is ambiguous:
   // it can be either the start of a <track-list>,
   // or the start of [ <line-names>? <string> <track-size>? <line-names>? ]+
   nsCSSValue firstLineNames;
-  if (!(ParseGridLineNames(firstLineNames) &&
-        GetToken(true))) {
+  if (ParseGridLineNames(firstLineNames) == CSSParseResult::Error ||
+      !GetToken(true)) {
     return false;
   }
   if (mToken.mType == eCSSToken_String) {
     // [ <track-list> / ]? was omitted
     // Parse [ <line-names>? <string> <track-size>? <line-names>? ]+
     value.SetNoneValue();
     AppendValue(eCSSProperty_grid_template_columns, value);
     return ParseGridTemplateAfterString(firstLineNames);
@@ -7310,18 +7313,18 @@ bool
 CSSParserImpl::ParseGridTemplateAfterSlash(bool aColumnsIsTrackList)
 {
   nsCSSValue rowsValue;
   if (!ParseVariant(rowsValue, VARIANT_NONE, nullptr)) {
     // TODO (bug 983175): add parsing for subgrid
     // as part of <'grid-template-rows'>
 
     nsCSSValue firstLineNames;
-    if (!(ParseGridLineNames(firstLineNames) &&
-          GetToken(true))) {
+    if (ParseGridLineNames(firstLineNames) == CSSParseResult::Error ||
+        !GetToken(true)) {
       return false;
     }
     if (aColumnsIsTrackList && mToken.mType == eCSSToken_String) {
       return ParseGridTemplateAfterString(firstLineNames);
     }
     UngetToken();
 
     if (!ParseGridTrackListWithFirstLineNames(rowsValue, firstLineNames)) {
@@ -7367,27 +7370,27 @@ CSSParserImpl::ParseGridTemplateAfterStr
       return false;
     }
     if (result == CSSParseResult::NotFound) {
       rowsItem->mValue.SetAutoValue();
     }
 
     rowsItem->mNext = new nsCSSValueList;
     rowsItem = rowsItem->mNext;
-    if (!ParseGridLineNames(rowsItem->mValue)) {
+    if (ParseGridLineNames(rowsItem->mValue) == CSSParseResult::Error) {
       return false;
     }
 
     if (CheckEndProperty()) {
       break;
     }
 
     // Append to the same list as the previous call to ParseGridLineNames.
-    if (!(ParseGridLineNames(rowsItem->mValue) &&
-          GetToken(true))) {
+    if (ParseGridLineNames(rowsItem->mValue) == CSSParseResult::Error ||
+        !GetToken(true)) {
       return false;
     }
     if (eCSSToken_String != mToken.mType) {
       UngetToken();
       return false;
     }
   }