Added CSSPropertyShapeUtils which holds shape parsing methods.

Added CSSPropertyShapeUtils which holds shape parsing methods.

This allows CSSPropertyShapeOutside as well as CSSPropertyClipPath to
use the consumeBasicShape method and for CSSPropertyShapeOutside and
CSSPropertyBorderRadius to use the consumeRadii method.

Moved complete4Sides into CSSPropertyParserHelpers as that method is
quite generic and simple, and used by several parser methods.

This patch:
- Adds CSSPropertyShapeUtils (.h and .cpp files) to the BUILD.gn
  file.
- Moves consumeBasicShape() and consumeRadii() from
  CSSPropertyParser.cpp to CSSPropertyShapeUtils.cpp, as well as helper
  methods consumeShapeRadius, consumeBasicShapeEllipse,
  consumeBasicShapePolygon, and consumeBasicShapeInset that are placed
  in anonymous namespace.
- Move complete4Sides method from CSSPropertyParser.cpp to
  CSSPropertyParserHelpers.cpp

BUG=668012
Review-Url: https://codereview.chromium.org/2645683003
Cr-Commit-Position: refs/heads/master@{#444885}
Committed: https://chromium.googlesource.com/chromium/src/+/29aa23c34afa7f5879aa6563a025c388b80c96a5

[ktyliu, 2017-01-19 02:55:08]

The CQ bit was checked by ktyliu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 02:55:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ktyliu, 2017-01-19 02:56:40]

ktyliu@chromium.org changed reviewers:
+ aazzam@google.com

[ktyliu, 2017-01-19 02:56:41]

Hi Anna,

Thanks for your review of 2642013002.

I've now moved just the utility part into this CL.
PTAL.

Thanks,
Kevin

[aazzam, 2017-01-19 03:35:49]

aazzam@google.com changed reviewers:
+ sashab@chromium.org

[aazzam, 2017-01-19 03:35:50]

awesome kevin, lgtm, sasha ptal :)

[sashab, 2017-01-19 04:12:14]

LGTM with nit: Anna, are we calling the files 'Util' or 'Utils'? Would just be
nice to keep it consistent. :-)

[aazzam, 2017-01-19 04:16:18]

On 2017/01/19 at 04:12:14, sashab wrote:
> LGTM with nit: Anna, are we calling the files 'Util' or 'Utils'? Would just be
nice to keep it consistent. :-)

my bad, didn't notice, ktyliu@ please change it to CSSPropertyShapeUtils!

[ktyliu, 2017-01-19 04:20:33]

The CQ bit was checked by ktyliu@chromium.org to run a CQ dry run

[ktyliu, 2017-01-19 04:20:33]

On 2017/01/19 at 04:16:18, aazzam wrote:
> On 2017/01/19 at 04:12:14, sashab wrote:
> > LGTM with nit: Anna, are we calling the files 'Util' or 'Utils'? Would just
be nice to keep it consistent. :-)
> 
> my bad, didn't notice, ktyliu@ please change it to CSSPropertyShapeUtils!

good catch!

Done

[commit-bot: I haz the power, 2017-01-19 04:20:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ktyliu, 2017-01-19 04:21:20]

Description was changed from

==========
Added CSSPropertyShapeUtil which holds shape parsing methods.

This allows CSSPropertyShapeOutside as well as CSSPropertyClipPath to
use the consumeBasicShape method and for CSSPropertyShapeOutside and
CSSPropertyBorderRadius to use the consumeRadii method.

Moved complete4Sides into CSSPropertyParserHelpers as that method is
quite generic and simple, and used by several parser methods.

This patch:
- Adds CSSPropertyShapeUtil (.h and .cpp files) to the BUILD.gn
  file.
- Moves consumeBasicShape() and consumeRadii() from
  CSSPropertyParser.cpp to CSSPropertyShapeUtil.cpp, as well as helper
  methods consumeShapeRadius, consumeBasicShapeEllipse,
  consumeBasicShapePolygon, and consumeBasicShapeInset that are placed
  in anonymous namespace.
- Move complete4Sides method from CSSPropertyParser.cpp to
  CSSPropertyParserHelpers.cpp

BUG=668012
==========

to

==========
Added CSSPropertyShapeUtils which holds shape parsing methods.

This allows CSSPropertyShapeOutside as well as CSSPropertyClipPath to
use the consumeBasicShape method and for CSSPropertyShapeOutside and
CSSPropertyBorderRadius to use the consumeRadii method.

Moved complete4Sides into CSSPropertyParserHelpers as that method is
quite generic and simple, and used by several parser methods.

This patch:
- Adds CSSPropertyShapeUtils (.h and .cpp files) to the BUILD.gn
  file.
- Moves consumeBasicShape() and consumeRadii() from
  CSSPropertyParser.cpp to CSSPropertyShapeUtils.cpp, as well as helper
  methods consumeShapeRadius, consumeBasicShapeEllipse,
  consumeBasicShapePolygon, and consumeBasicShapeInset that are placed
  in anonymous namespace.
- Move complete4Sides method from CSSPropertyParser.cpp to
  CSSPropertyParserHelpers.cpp

BUG=668012
==========

[sashab, 2017-01-19 04:22:22]

Cool :) LGTM, thanks for updating CL desc too

[commit-bot: I haz the power, 2017-01-19 04:49:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 04:49:53]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[ktyliu, 2017-01-19 04:52:14]

The CQ bit was checked by ktyliu@chromium.org

[ktyliu, 2017-01-19 04:52:14]

The patchset sent to the CQ was uploaded after l-g-t-m from aazzam@google.com,
sashab@chromium.org
Link to the patchset: https://codereview.chromium.org/2645683003/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2017-01-19 04:52:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 07:49:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 07:49:11]

Failed to apply patch for
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:43
  error: third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp: patch
does not apply

Patch:       third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
Index: third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
index
17294ba2bc4d3b80ff0c26a3a658e43de5fb44ab..5591ab64daffdbd3c784826179f2608a77681d82
100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -43,6 +43,7 @@
 #include "core/css/parser/CSSVariableParser.h"
 #include "core/css/properties/CSSPropertyAlignmentUtils.h"
 #include "core/css/properties/CSSPropertyDescriptor.h"
+#include "core/css/properties/CSSPropertyShapeUtils.h"
 #include "core/frame/UseCounter.h"
 #include "core/layout/LayoutTheme.h"
 #include "core/svg/SVGPathUtilities.h"
@@ -1767,214 +1768,13 @@ static CSSValue*
consumeScrollSnapPoints(CSSParserTokenRange& range,
   return nullptr;
 }
 
-static CSSValue* consumeShapeRadius(CSSParserTokenRange& args,
-                                    CSSParserMode cssParserMode) {
-  if (identMatches<CSSValueClosestSide,
CSSValueFarthestSide>(args.peek().id()))
-    return consumeIdent(args);
-  return consumeLengthOrPercent(args, cssParserMode, ValueRangeNonNegative);
-}
-
-static CSSBasicShapeCircleValue* consumeBasicShapeCircle(
-    CSSParserTokenRange& args,
-    const CSSParserContext* context) {
-  // spec: https://drafts.csswg.org/css-shapes/#supported-basic-shapes
-  // circle( [<shape-radius>]? [at <position>]? )
-  CSSBasicShapeCircleValue* shape = CSSBasicShapeCircleValue::create();
-  if (CSSValue* radius = consumeShapeRadius(args, context->mode()))
-    shape->setRadius(radius);
-  if (consumeIdent<CSSValueAt>(args)) {
-    CSSValue* centerX = nullptr;
-    CSSValue* centerY = nullptr;
-    if (!consumePosition(args, context->mode(), UnitlessQuirk::Forbid, centerX,
-                         centerY))
-      return nullptr;
-    shape->setCenterX(centerX);
-    shape->setCenterY(centerY);
-  }
-  return shape;
-}
-
-static CSSBasicShapeEllipseValue* consumeBasicShapeEllipse(
-    CSSParserTokenRange& args,
-    const CSSParserContext* context) {
-  // spec: https://drafts.csswg.org/css-shapes/#supported-basic-shapes
-  // ellipse( [<shape-radius>{2}]? [at <position>]? )
-  CSSBasicShapeEllipseValue* shape = CSSBasicShapeEllipseValue::create();
-  if (CSSValue* radiusX = consumeShapeRadius(args, context->mode())) {
-    shape->setRadiusX(radiusX);
-    if (CSSValue* radiusY = consumeShapeRadius(args, context->mode()))
-      shape->setRadiusY(radiusY);
-  }
-  if (consumeIdent<CSSValueAt>(args)) {
-    CSSValue* centerX = nullptr;
-    CSSValue* centerY = nullptr;
-    if (!consumePosition(args, context->mode(), UnitlessQuirk::Forbid, centerX,
-                         centerY))
-      return nullptr;
-    shape->setCenterX(centerX);
-    shape->setCenterY(centerY);
-  }
-  return shape;
-}
-
-static CSSBasicShapePolygonValue* consumeBasicShapePolygon(
-    CSSParserTokenRange& args,
-    const CSSParserContext* context) {
-  CSSBasicShapePolygonValue* shape = CSSBasicShapePolygonValue::create();
-  if (identMatches<CSSValueEvenodd, CSSValueNonzero>(args.peek().id())) {
-    shape->setWindRule(args.consumeIncludingWhitespace().id() ==
CSSValueEvenodd
-                           ? RULE_EVENODD
-                           : RULE_NONZERO);
-    if (!consumeCommaIncludingWhitespace(args))
-      return nullptr;
-  }
-
-  do {
-    CSSPrimitiveValue* xLength =
-        consumeLengthOrPercent(args, context->mode(), ValueRangeAll);
-    if (!xLength)
-      return nullptr;
-    CSSPrimitiveValue* yLength =
-        consumeLengthOrPercent(args, context->mode(), ValueRangeAll);
-    if (!yLength)
-      return nullptr;
-    shape->appendPoint(xLength, yLength);
-  } while (consumeCommaIncludingWhitespace(args));
-  return shape;
-}
-
-static void complete4Sides(CSSValue* side[4]) {
-  if (side[3])
-    return;
-  if (!side[2]) {
-    if (!side[1])
-      side[1] = side[0];
-    side[2] = side[0];
-  }
-  side[3] = side[1];
-}
-
-static bool consumeRadii(CSSValue* horizontalRadii[4],
-                         CSSValue* verticalRadii[4],
-                         CSSParserTokenRange& range,
-                         CSSParserMode cssParserMode,
-                         bool useLegacyParsing) {
-  unsigned i = 0;
-  for (; i < 4 && !range.atEnd() && range.peek().type() != DelimiterToken;
-       ++i) {
-    horizontalRadii[i] =
-        consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative);
-    if (!horizontalRadii[i])
-      return false;
-  }
-  if (!horizontalRadii[0])
-    return false;
-  if (range.atEnd()) {
-    // Legacy syntax: -webkit-border-radius: l1 l2; is equivalent to
-    // border-radius: l1 / l2;
-    if (useLegacyParsing && i == 2) {
-      verticalRadii[0] = horizontalRadii[1];
-      horizontalRadii[1] = nullptr;
-    } else {
-      complete4Sides(horizontalRadii);
-      for (unsigned i = 0; i < 4; ++i)
-        verticalRadii[i] = horizontalRadii[i];
-      return true;
-    }
-  } else {
-    if (!consumeSlashIncludingWhitespace(range))
-      return false;
-    for (i = 0; i < 4 && !range.atEnd(); ++i) {
-      verticalRadii[i] =
-          consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative);
-      if (!verticalRadii[i])
-        return false;
-    }
-    if (!verticalRadii[0] || !range.atEnd())
-      return false;
-  }
-  complete4Sides(horizontalRadii);
-  complete4Sides(verticalRadii);
-  return true;
-}
-
-static CSSBasicShapeInsetValue* consumeBasicShapeInset(
-    CSSParserTokenRange& args,
-    const CSSParserContext* context) {
-  CSSBasicShapeInsetValue* shape = CSSBasicShapeInsetValue::create();
-  CSSPrimitiveValue* top =
-      consumeLengthOrPercent(args, context->mode(), ValueRangeAll);
-  if (!top)
-    return nullptr;
-  CSSPrimitiveValue* right =
-      consumeLengthOrPercent(args, context->mode(), ValueRangeAll);
-  CSSPrimitiveValue* bottom = nullptr;
-  CSSPrimitiveValue* left = nullptr;
-  if (right) {
-    bottom = consumeLengthOrPercent(args, context->mode(), ValueRangeAll);
-    if (bottom)
-      left = consumeLengthOrPercent(args, context->mode(), ValueRangeAll);
-  }
-  if (left)
-    shape->updateShapeSize4Values(top, right, bottom, left);
-  else if (bottom)
-    shape->updateShapeSize3Values(top, right, bottom);
-  else if (right)
-    shape->updateShapeSize2Values(top, right);
-  else
-    shape->updateShapeSize1Value(top);
-
-  if (consumeIdent<CSSValueRound>(args)) {
-    CSSValue* horizontalRadii[4] = {0};
-    CSSValue* verticalRadii[4] = {0};
-    if (!consumeRadii(horizontalRadii, verticalRadii, args, context->mode(),
-                      false))
-      return nullptr;
-    shape->setTopLeftRadius(
-        CSSValuePair::create(horizontalRadii[0], verticalRadii[0],
-                             CSSValuePair::DropIdenticalValues));
-    shape->setTopRightRadius(
-        CSSValuePair::create(horizontalRadii[1], verticalRadii[1],
-                             CSSValuePair::DropIdenticalValues));
-    shape->setBottomRightRadius(
-        CSSValuePair::create(horizontalRadii[2], verticalRadii[2],
-                             CSSValuePair::DropIdenticalValues));
-    shape->setBottomLeftRadius(
-        CSSValuePair::create(horizontalRadii[3], verticalRadii[3],
-                             CSSValuePair::DropIdenticalValues));
-  }
-  return shape;
-}
-
-static CSSValue* consumeBasicShape(CSSParserTokenRange& range,
-                                   const CSSParserContext* context) {
-  CSSValue* shape = nullptr;
-  if (range.peek().type() != FunctionToken)
-    return nullptr;
-  CSSValueID id = range.peek().functionId();
-  CSSParserTokenRange rangeCopy = range;
-  CSSParserTokenRange args = consumeFunction(rangeCopy);
-  if (id == CSSValueCircle)
-    shape = consumeBasicShapeCircle(args, context);
-  else if (id == CSSValueEllipse)
-    shape = consumeBasicShapeEllipse(args, context);
-  else if (id == CSSValuePolygon)
-    shape = consumeBasicShapePolygon(args, context);
-  else if (id == CSSValueInset)
-    shape = consumeBasicShapeInset(args, context);
-  if (!shape || !args.atEnd())
-    return nullptr;
-  range = rangeCopy;
-  return shape;
-}
-
 static CSSValue* consumeClipPath(CSSParserTokenRange& range,
                                  const CSSParserContext* context) {
   if (range.peek().id() == CSSValueNone)
     return consumeIdent(range);
   if (CSSURIValue* url = consumeUrl(range, context))
     return url;
-  return consumeBasicShape(range, context);
+  return CSSPropertyShapeUtils::consumeBasicShape(range, context);
 }
 
 static CSSValue* consumeShapeOutside(CSSParserTokenRange& range,
@@ -1984,7 +1784,8 @@ static CSSValue* consumeShapeOutside(CSSParserTokenRange&
range,
   CSSValueList* list = CSSValueList::createSpaceSeparated();
   if (CSSValue* boxValue = consumeShapeBox(range))
     list->append(*boxValue);
-  if (CSSValue* shapeValue = consumeBasicShape(range, context)) {
+  if (CSSValue* shapeValue =
+          CSSPropertyShapeUtils::consumeBasicShape(range, context)) {
     list->append(*shapeValue);
     if (list->length() < 2) {
       if (CSSValue* boxValue = consumeShapeBox(range))
@@ -4603,7 +4404,7 @@ bool CSSPropertyParser::parseShorthand(CSSPropertyID
unresolvedProperty,
     case CSSPropertyBorderRadius: {
       CSSValue* horizontalRadii[4] = {0};
       CSSValue* verticalRadii[4] = {0};
-      if (!consumeRadii(
+      if (!CSSPropertyShapeUtils::consumeRadii(
               horizontalRadii, verticalRadii, m_range, …
(message too large)

[ktyliu, 2017-01-19 21:38:52]

The CQ bit was checked by ktyliu@chromium.org

[ktyliu, 2017-01-19 21:38:52]

The patchset sent to the CQ was uploaded after l-g-t-m from sashab@chromium.org,
aazzam@google.com
Link to the patchset: https://codereview.chromium.org/2645683003/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2017-01-19 21:39:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 23:22:06]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484861932310870,
"parent_rev": "2c430e0be3b1aab8409b61c3485f41fd041d2d1e", "commit_rev":
"29aa23c34afa7f5879aa6563a025c388b80c96a5"}

[commit-bot: I haz the power, 2017-01-19 23:22:39]

Description was changed from

==========
Added CSSPropertyShapeUtils which holds shape parsing methods.

This allows CSSPropertyShapeOutside as well as CSSPropertyClipPath to
use the consumeBasicShape method and for CSSPropertyShapeOutside and
CSSPropertyBorderRadius to use the consumeRadii method.

Moved complete4Sides into CSSPropertyParserHelpers as that method is
quite generic and simple, and used by several parser methods.

This patch:
- Adds CSSPropertyShapeUtils (.h and .cpp files) to the BUILD.gn
  file.
- Moves consumeBasicShape() and consumeRadii() from
  CSSPropertyParser.cpp to CSSPropertyShapeUtils.cpp, as well as helper
  methods consumeShapeRadius, consumeBasicShapeEllipse,
  consumeBasicShapePolygon, and consumeBasicShapeInset that are placed
  in anonymous namespace.
- Move complete4Sides method from CSSPropertyParser.cpp to
  CSSPropertyParserHelpers.cpp

BUG=668012
==========

to

==========
Added CSSPropertyShapeUtils which holds shape parsing methods.

This allows CSSPropertyShapeOutside as well as CSSPropertyClipPath to
use the consumeBasicShape method and for CSSPropertyShapeOutside and
CSSPropertyBorderRadius to use the consumeRadii method.

Moved complete4Sides into CSSPropertyParserHelpers as that method is
quite generic and simple, and used by several parser methods.

This patch:
- Adds CSSPropertyShapeUtils (.h and .cpp files) to the BUILD.gn
  file.
- Moves consumeBasicShape() and consumeRadii() from
  CSSPropertyParser.cpp to CSSPropertyShapeUtils.cpp, as well as helper
  methods consumeShapeRadius, consumeBasicShapeEllipse,
  consumeBasicShapePolygon, and consumeBasicShapeInset that are placed
  in anonymous namespace.
- Move complete4Sides method from CSSPropertyParser.cpp to
  CSSPropertyParserHelpers.cpp

BUG=668012

Review-Url: https://codereview.chromium.org/2645683003
Cr-Commit-Position: refs/heads/master@{#444885}
Committed:
https://chromium.googlesource.com/chromium/src/+/29aa23c34afa7f5879aa6563a025...
==========

[commit-bot: I haz the power, 2017-01-19 23:22:40]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/29aa23c34afa7f5879aa6563a025...