Support calc(...) in ConsumeAngleOrPercent / for conic-gradient

Support calc(...) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages. CSSGradientValue also wasn't
checking for calc() values when evaluating stops.
Rewrite ConsumeAngleOrPercent in a vein similar to
ConsumeLengthOrPercent.
Make CSSPrimitiveValue::IsAngle() consider resolved type, and update
CSSRotation TypedOM implementation to counter this.

BUG=709730
Review-Url: https://codereview.chromium.org/2813583002
Cr-Commit-Position: refs/heads/master@{#463585}
Committed: https://chromium.googlesource.com/chromium/src/+/cc50825cf637990a4edfa90c9610ad072885c74d

[fs, 2017-04-10 11:23:08]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 11:23:21]

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

[fs, 2017-04-10 12:27:55]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 12:28:08]

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

[fs, 2017-04-10 12:29:59]

Description was changed from

==========
Support calc(<percentage>) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages.
Rewrite ConsumeAngleOrPercent in a vain similar to
ConsumeLengthOrPercent.

BUG=709730
==========

to

==========
Support calc(<percentage>) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages.
Rewrite ConsumeAngleOrPercent in a vein similar to
ConsumeLengthOrPercent.

BUG=709730
==========

[fs, 2017-04-10 12:30:26]

Description was changed from

==========
Support calc(<percentage>) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages.
Rewrite ConsumeAngleOrPercent in a vein similar to
ConsumeLengthOrPercent.

BUG=709730
==========

to

==========
Support calc(<percentage>) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages.
Rewrite ConsumeAngleOrPercent in a vein similar to
ConsumeLengthOrPercent.
Make CSSPrimitiveValue::IsAngle() consider resolved type.

BUG=709730
==========

[fs, 2017-04-10 12:30:38]

fs@opera.com changed reviewers:
+ alancutter@chromium.org, fmalita@chromium.org

[commit-bot: I haz the power, 2017-04-10 13:28:44]

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

[commit-bot: I haz the power, 2017-04-10 13:28:45]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[f(malita), 2017-04-10 14:07:03]

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp
(right):

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp:948:
CalcParser calc_parser(range, value_range);
Both ConsumePercent and ConsumeAngle perform type checks and handle calc values
internally.  So I *think* it's safe to just cascade-invoke, like this
https://codereview.chromium.org/2814453002/diff/1/third_party/WebKit/Source/c...

[fs, 2017-04-10 14:09:08]

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp
(right):

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp:948:
CalcParser calc_parser(range, value_range);
On 2017/04/10 at 14:07:03, f(malita) wrote:
> Both ConsumePercent and ConsumeAngle perform type checks and handle calc
values internally.  So I *think* it's safe to just cascade-invoke, like this
https://codereview.chromium.org/2814453002/diff/1/third_party/WebKit/Source/c...

Yes, but then we can end up parsing calc() twice (depending on order and actual
type.)

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp:951: //
TODO(fs): Add and support kCalcPercentAngle?
+alancutter for thoughts on this

[f(malita), 2017-04-10 14:16:26]

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSPrimitiveValue.h (right):

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSPrimitiveValue.h:167: bool IsAngle() const
{ return IsAngle(TypeWithCalcResolved()); }
All IsFoo() helpers seem to ignore calc values at this point.  For consistency,
I suggest we leave IsAngle() as-is for now and update the test in
CSSGradientValue::AddStops() instead:

  stop.offset_->IsAngle() =>
CSSPrimitiveValue::IsAngle(stop.offset_->TypeWithCalcResolved())

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp
(right):

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp:948:
CalcParser calc_parser(range, value_range);
On 2017/04/10 14:09:08, fs wrote:
> On 2017/04/10 at 14:07:03, f(malita) wrote:
> > Both ConsumePercent and ConsumeAngle perform type checks and handle calc
> values internally.  So I *think* it's safe to just cascade-invoke, like this
>
https://codereview.chromium.org/2814453002/diff/1/third_party/WebKit/Source/c...
> 
> Yes, but then we can end up parsing calc() twice (depending on order and
actual
> type.)

Ack, this approach is more efficient.

[fs, 2017-04-10 14:22:58]

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSPrimitiveValue.h (right):

https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSPrimitiveValue.h:167: bool IsAngle() const
{ return IsAngle(TypeWithCalcResolved()); }
On 2017/04/10 at 14:16:26, f(malita) wrote:
> All IsFoo() helpers seem to ignore calc values at this point.  For
consistency, I suggest we leave IsAngle() as-is for now and update the test in
CSSGradientValue::AddStops() instead:
> 
>   stop.offset_->IsAngle() =>
CSSPrimitiveValue::IsAngle(stop.offset_->TypeWithCalcResolved())

Hmm, IsLength, IsNumber and IsPercentage (the non-static versions) all use
TypeWithCalcResolved()?

[f(malita), 2017-04-10 14:25:09]

On 2017/04/10 14:22:58, fs wrote:
>
https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/CSSPrimitiveValue.h (right):
> 
>
https://codereview.chromium.org/2813583002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/CSSPrimitiveValue.h:167: bool IsAngle()
const
> { return IsAngle(TypeWithCalcResolved()); }
> On 2017/04/10 at 14:16:26, f(malita) wrote:
> > All IsFoo() helpers seem to ignore calc values at this point.  For
> consistency, I suggest we leave IsAngle() as-is for now and update the test in
> CSSGradientValue::AddStops() instead:
> > 
> >   stop.offset_->IsAngle() =>
> CSSPrimitiveValue::IsAngle(stop.offset_->TypeWithCalcResolved())
> 
> Hmm, IsLength, IsNumber and IsPercentage (the non-static versions) all use
> TypeWithCalcResolved()?

Heh, let me rephrase: "all helpers in my narrow editor view" :)

LGTM

[fs, 2017-04-10 14:29:57]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 14:30:05]

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

[fs, 2017-04-10 15:12:15]

fs@opera.com changed reviewers:
+ meade@chromium.org

[commit-bot: I haz the power, 2017-04-10 16:27:03]

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

[commit-bot: I haz the power, 2017-04-10 16:27:04]

Dry run: This issue passed the CQ dry run.

[meade_UTC10, 2017-04-11 05:03:28]

lgtm

[alancutter (OOO until 2018), 2017-04-11 05:28:55]

lgtm

[fs, 2017-04-11 09:14:43]

Description was changed from

==========
Support calc(<percentage>) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages.
Rewrite ConsumeAngleOrPercent in a vein similar to
ConsumeLengthOrPercent.
Make CSSPrimitiveValue::IsAngle() consider resolved type.

BUG=709730
==========

to

==========
Support calc(...) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages. CSSGradientValue also wasn't
checking for calc() values when evaluating stops.
Rewrite ConsumeAngleOrPercent in a vein similar to
ConsumeLengthOrPercent.
Make CSSPrimitiveValue::IsAngle() consider resolved type, and update
CSSRotation TypedOM implementation to counter this.

BUG=709730
==========

[fs, 2017-04-11 09:14:49]

The CQ bit was checked by fs@opera.com

[fs, 2017-04-11 09:14:49]

The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/2813583002/#ps40001
(title: "TypedOM CSSRotation fixup")

[commit-bot: I haz the power, 2017-04-11 09:15:11]

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-04-11 09:23:48]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491902089044900,
"parent_rev": "61312491232439beddbbd347cf86bf95f99aedb2", "commit_rev":
"cc50825cf637990a4edfa90c9610ad072885c74d"}

[commit-bot: I haz the power, 2017-04-11 09:24:35]

Description was changed from

==========
Support calc(...) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages. CSSGradientValue also wasn't
checking for calc() values when evaluating stops.
Rewrite ConsumeAngleOrPercent in a vein similar to
ConsumeLengthOrPercent.
Make CSSPrimitiveValue::IsAngle() consider resolved type, and update
CSSRotation TypedOM implementation to counter this.

BUG=709730
==========

to

==========
Support calc(...) in ConsumeAngleOrPercent / for conic-gradient

Because of the explicit check for <percentage-token>, calc(...) would
not be properly handled for percentages. CSSGradientValue also wasn't
checking for calc() values when evaluating stops.
Rewrite ConsumeAngleOrPercent in a vein similar to
ConsumeLengthOrPercent.
Make CSSPrimitiveValue::IsAngle() consider resolved type, and update
CSSRotation TypedOM implementation to counter this.

BUG=709730

Review-Url: https://codereview.chromium.org/2813583002
Cr-Commit-Position: refs/heads/master@{#463585}
Committed:
https://chromium.googlesource.com/chromium/src/+/cc50825cf637990a4edfa90c9610...
==========

[commit-bot: I haz the power, 2017-04-11 09:24:37]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/cc50825cf637990a4edfa90c9610...