support function in custom paint input arguments

third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/css/parser/CSSPropertyParserHelpers.h"
 
 #include "core/css/CSSCalculationValue.h"
 #include "core/css/CSSColorValue.h"
 #include "core/css/CSSCrossfadeValue.h"
 #include "core/css/CSSGradientValue.h"
@@ skipping 1148 matching lines @@
   if (!RuntimeEnabledFeatures::cssPaintAPIArgumentsEnabled()) {
     // Arguments not enabled, but exists. Invalid.
     return nullptr;
   }
 
   // Begin parse paint arguments.
   if (!consumeCommaIncludingWhitespace(args))
     return nullptr;
 
   // Consume arguments. Currently does not support complicated arguments
   // like function calls.

[meade_UTC10, 2017/02/16 02:44:59]

Sounds like this comment needs to be updated.

[renjieliu1, 2017/02/16 04:38:42]

Done.

   // TODO(renjieliu): We may want to optimize the implementation by resolve
   // variables early if paint function is registered.
   Vector<CSSParserToken> argumentTokens;
   Vector<RefPtr<CSSVariableData>> variableData;
   while (!args.atEnd()) {
     if (args.peek().type() != CommaToken) {
-      argumentTokens.push_back(args.consumeIncludingWhitespace());
+      if (args.peek().getBlockType() != CSSParserToken::BlockStart) {

[meade_UTC10, 2017/02/16 02:44:59]

There's a lot of nested ifs in this while loop now. Would it be better to split
this out into two functions? Something like this (incorrect) idea:

while(!args.atEnd()) {
  if (args.peek().getBlockType() != CSSParserToken::BlockStart) {
    // Handle function input
  } else {
    // Handle list input
  }
}

This might make it clearer where the function name is getting parsed - it's not
clear to me now

[renjieliu1, 2017/02/16 04:38:42]

Done.

+        argumentTokens.push_back(args.consumeIncludingWhitespace());
+      } else {
+        // Encounter function block.
+        argumentTokens.push_back(args.peek());

[meade_UTC10, 2017/02/16 02:44:59]

So this pushes an open brace, then the contents of the block, then a closing
brace?

Why not

CSSParserTokenRange contents = args.consumeBlock();
argumentTokens.push_back(LeftBracketToken);
while (!contents.end())
  argumentTokens.push_back(contents.consume()
argumentTokens.push_back(RightBracketToken);

[renjieliu1, 2017/02/16 04:38:42]

I totally agree that's what we should do, however, CSSParserToken treats
function name along with parenthesis as a single token.

Example rgb(1, 2, 3) => rgb(1, 2, 3 after the consumeBlock function.

+        CSSParserTokenRange contents = args.consumeBlock();
+        while (!contents.atEnd()) {
+          argumentTokens.push_back(contents.consume());
+        }
+        // Assume it is a function.

[meade_UTC10, 2017/02/16 02:44:59]

This comment is confusing, suggest removing it.

[renjieliu1, 2017/02/16 04:38:42]

Done.

+        argumentTokens.push_back(
+            CSSParserToken(RightParenthesisToken, CSSParserToken::BlockEnd));
+      }
     } else {
       if (!addCSSPaintArgument(argumentTokens, &variableData))
         return nullptr;
       argumentTokens.clear();
       if (!consumeCommaIncludingWhitespace(args))
         return nullptr;
     }
   }
   if (!addCSSPaintArgument(argumentTokens, &variableData))
     return nullptr;
@@ skipping 131 matching lines @@
 
 // https://drafts.csswg.org/css-shapes-1/#typedef-shape-box
 CSSIdentifierValue* consumeShapeBox(CSSParserTokenRange& range) {
   return consumeIdent<CSSValueContentBox, CSSValuePaddingBox, CSSValueBorderBox,
                       CSSValueMarginBox>(range);
 }
 
 }  // namespace CSSPropertyParserHelpers
 
 }  // namespace blink