[CSS Typed OM] Implement support for CSSValue -> CSSCalcLength

Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

For example:
testElement.style.width = "calc(4 * (5em - 2px))";
testElmenet.styleMap.get('width');
// Returns CSSCalcLength({px: -8, em: 20})


Also removes redundant test typedcssom/inlinestyle/left-top-right-bottom.html 
(superceded by typedcssom/inlinestyle/properties/bottom.html; will
add the others later)


BUG=545318
Review-Url: https://codereview.chromium.org/2371673002
Cr-Commit-Position: refs/heads/master@{#443133}
Committed: https://chromium.googlesource.com/chromium/src/+/857445ffbc21f9cac8a018128a3a92574fe3d784

[meade_UTC10, 2016-10-07 04:35:56]

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

[commit-bot: I haz the power, 2016-10-07 04:36:09]

Dry run: 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, 2016-10-07 06:11:49]

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

[commit-bot: I haz the power, 2016-10-07 06:11:50]

Dry run: This issue passed the CQ dry run.

[meade_UTC10, 2016-10-10 03:26:54]

meade@chromium.org changed reviewers:
+ rjwright@chromium.org

[meade_UTC10, 2016-10-10 03:26:55]

I haven't updated this or rebased it yet, but PTAL?

[Timothy Loh, 2016-10-10 04:23:50]

(cc-ing alancutter, who is the maintainer of calc)

[meade_UTC10, 2016-10-13 06:33:13]

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

[commit-bot: I haz the power, 2016-10-13 06:33:31]

Dry run: 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, 2016-10-13 07:50:05]

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

[commit-bot: I haz the power, 2016-10-13 07:50:06]

Dry run: This issue passed the CQ dry run.

[alancutter (OOO until 2018), 2016-10-13 22:19:04]

alancutter@chromium.org changed reviewers:
+ alancutter@chromium.org

[alancutter (OOO until 2018), 2016-10-13 22:19:05]

https://codereview.chromium.org/2371673002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSCalculationValue.cpp (right):

https://codereview.chromium.org/2371673002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSCalculationValue.cpp:259: CalcOperator
operatorType() const { return CalcAdd; }
Can these just be NOTREACHED()?

https://codereview.chromium.org/2371673002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp (right):

https://codereview.chromium.org/2371673002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp:144: const
CSSCalcExpressionNode* expressionNode) {
This UnitData type would be a great representation for CSSCalculationValues
(though I'd name it something less generic). Please consider performing this
tree flattening at calc() parse time, I expect the array based implementation to
simplify multiple areas of code dealing with calc() including typed OM.

[meade_UTC10, 2016-11-16 19:28:20]

meade@chromium.org changed reviewers:
- alancutter@chromium.org

[meade_UTC10, 2016-11-16 19:28:21]

Renee: PTAL?

https://codereview.chromium.org/2371673002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSCalculationValue.cpp (right):

https://codereview.chromium.org/2371673002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSCalculationValue.cpp:259: CalcOperator
operatorType() const { return CalcAdd; }
On 2016/10/13 22:19:05, alancutter wrote:
> Can these just be NOTREACHED()?

Done.

https://codereview.chromium.org/2371673002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp (right):

https://codereview.chromium.org/2371673002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/cssom/CSSCalcLength.cpp:144: const
CSSCalcExpressionNode* expressionNode) {
On 2016/10/13 22:19:05, alancutter wrote:
> This UnitData type would be a great representation for CSSCalculationValues
> (though I'd name it something less generic). Please consider performing this
> tree flattening at calc() parse time, I expect the array based implementation
to
> simplify multiple areas of code dealing with calc() including typed OM.

I agree, but I don't want to block typed om on refactoring CSSCalculationValue.
I've added this to our backlog instead.

[meade_UTC10, 2016-11-28 04:53:48]

Renee: Friendly ping?

[meade_UTC10, 2016-11-29 05:34:09]

Description was changed from

==========
Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

BUG=545318
==========

to

==========
Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

BUG=545318
==========

[rjwright, 2016-12-15 05:19:35]

On 2016/11/28 04:53:48, Eddy (OOO until 19 Dec) wrote:
> Renee: Friendly ping?

LGTM :)

[meade_UTC10, 2016-12-19 01:14:43]

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

[commit-bot: I haz the power, 2016-12-19 01:14:49]

Dry run: 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, 2016-12-19 02:26:28]

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

[commit-bot: I haz the power, 2016-12-19 02:26:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[meade_UTC10, 2016-12-19 05:16:05]

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

[commit-bot: I haz the power, 2016-12-19 05:16:14]

Dry run: 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, 2016-12-19 06:26:12]

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

[commit-bot: I haz the power, 2016-12-19 06:26:13]

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_...)

[meade_UTC10, 2016-12-21 00:55:16]

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

[commit-bot: I haz the power, 2016-12-21 00:55:50]

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

[meade_UTC10, 2016-12-21 02:20:39]

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

[commit-bot: I haz the power, 2016-12-21 02:20:57]

Dry run: 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, 2016-12-21 03:52:01]

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

[commit-bot: I haz the power, 2016-12-21 03:52:02]

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

[meade_UTC10, 2016-12-21 04:08:42]

Description was changed from

==========
Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

BUG=545318
==========

to

==========
Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

For example:
testElement.style.width = "calc(4 * (5em - 2px))";
testElmenet.styleMap.get('width');
// Returns CSSCalcLength({px: -8, em: 20})


Also removes redundant test typedcssom/inlinestyle/left-top-right-bottom.html
(superceded by typedcssom/inlinestyle/properties/bottom.html; will add the
others later)


BUG=545318
==========

[meade_UTC10, 2016-12-21 04:09:05]

Description was changed from

==========
Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

For example:
testElement.style.width = "calc(4 * (5em - 2px))";
testElmenet.styleMap.get('width');
// Returns CSSCalcLength({px: -8, em: 20})


Also removes redundant test typedcssom/inlinestyle/left-top-right-bottom.html
(superceded by typedcssom/inlinestyle/properties/bottom.html; will add the
others later)


BUG=545318
==========

to

==========
Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

For example:
testElement.style.width = "calc(4 * (5em - 2px))";
testElmenet.styleMap.get('width');
// Returns CSSCalcLength({px: -8, em: 20})


Also removes redundant test typedcssom/inlinestyle/left-top-right-bottom.html 
(superceded by typedcssom/inlinestyle/properties/bottom.html; will
add the others later)


BUG=545318
==========

[meade_UTC10, 2016-12-21 04:09:31]

meade@chromium.org changed reviewers:
+ sashab@chromium.org

[meade_UTC10, 2016-12-21 04:09:32]

+Sasha for OWNERS. PTAL, thank you!

[meade_UTC10, 2016-12-21 04:09:38]

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

[commit-bot: I haz the power, 2016-12-21 04:10:00]

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

[sashab, 2016-12-21 04:15:35]

It feels weird having duplicate implementations for all this stuff in both
CSSValue and StyleValue. Is any of this stuff common to both that you could pull
out?

https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/typedcssom/inlinestyle/left-top-right-bottom.html
(left):

https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/typedcssom/inlinestyle/left-top-right-bottom.html:1:
<!DOCTYPE html>
Why remove all these tests? :)

https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp (right):

https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp:30: // NOTE:
Won't work for non-length units. Perhaps check on propertyID?
TODO?

[meade_UTC10, 2016-12-21 04:46:59]

On 2016/12/21 04:15:35, sashab wrote:
> It feels weird having duplicate implementations for all this stuff in both
> CSSValue and StyleValue. Is any of this stuff common to both that you could
pull
> out?
AFAIK this isn't supported in CSSValue though? 

> 
>
https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Lay...
> File
>
third_party/WebKit/LayoutTests/typedcssom/inlinestyle/left-top-right-bottom.html
> (left):
> 
>
https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/typedcssom/inlinestyle/left-top-right-bottom.html:1:
> <!DOCTYPE html>
> Why remove all these tests? :)
> 
>
https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp (right):
> 
>
https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp:30: // NOTE:
> Won't work for non-length units. Perhaps check on propertyID?
> TODO?

[meade_UTC10, 2016-12-21 04:47:09]

https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/typedcssom/inlinestyle/left-top-right-bottom.html
(left):

https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/typedcssom/inlinestyle/left-top-right-bottom.html:1:
<!DOCTYPE html>
On 2016/12/21 04:15:35, sashab wrote:
> Why remove all these tests? :)

It's in the CL description :) I'm replacing these with the property-suite tests;
these are now in typedcssom/inlinestyle/properties/bottom.html.

https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp (right):

https://codereview.chromium.org/2371673002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp:30: // NOTE:
Won't work for non-length units. Perhaps check on propertyID?
On 2016/12/21 04:15:35, sashab wrote:
> TODO?

Done.

[commit-bot: I haz the power, 2016-12-21 05:36:26]

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

[commit-bot: I haz the power, 2016-12-21 05:36:27]

Dry run: This issue passed the CQ dry run.

[sashab, 2016-12-21 23:30:21]

LGTM

I'd like to see class comments on the StyleValue subclasses, maybe you can add
some of those in follow up patches. :)

[meade_UTC10, 2017-01-05 05:31:18]

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

[commit-bot: I haz the power, 2017-01-05 05:31:41]

Dry run: 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-05 07:08:01]

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

[commit-bot: I haz the power, 2017-01-05 07:08:05]

Dry run: This issue passed the CQ dry run.

[meade_UTC10, 2017-01-10 03:11:31]

The CQ bit was checked by meade@chromium.org

[meade_UTC10, 2017-01-10 03:11:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjwright@chromium.org, sashab@chromium.org
Link to the patchset: https://codereview.chromium.org/2371673002/#ps320001
(title: "Remove unneccessary expectation file \o/")

[commit-bot: I haz the power, 2017-01-10 03:12:19]

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-10 06:24:31]

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

[commit-bot: I haz the power, 2017-01-10 06:24:33]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[meade_UTC10, 2017-01-12 00:47:07]

The CQ bit was checked by meade@chromium.org

[meade_UTC10, 2017-01-12 00:47:08]

The patchset sent to the CQ was uploaded after l-g-t-m from sashab@chromium.org,
rjwright@chromium.org
Link to the patchset: https://codereview.chromium.org/2371673002/#ps340001
(title: "sync")

[commit-bot: I haz the power, 2017-01-12 00:48:29]

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-12 02:58:51]

CQ is committing da patch.

Bot data: {"patchset_id": 340001, "attempt_start_ts": 1484182027517580,
"parent_rev": "f76945dbb3afd556549b72544644a2e14fd32271", "commit_rev":
"857445ffbc21f9cac8a018128a3a92574fe3d784"}

[commit-bot: I haz the power, 2017-01-12 02:59:22]

Description was changed from

==========
Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

For example:
testElement.style.width = "calc(4 * (5em - 2px))";
testElmenet.styleMap.get('width');
// Returns CSSCalcLength({px: -8, em: 20})


Also removes redundant test typedcssom/inlinestyle/left-top-right-bottom.html 
(superceded by typedcssom/inlinestyle/properties/bottom.html; will
add the others later)


BUG=545318
==========

to

==========
Implement support for CSSValue -> CSSCalcLength

The CSSValue must be a CSSPrimitiveValue containing a CSSCalcValue.

For example:
testElement.style.width = "calc(4 * (5em - 2px))";
testElmenet.styleMap.get('width');
// Returns CSSCalcLength({px: -8, em: 20})


Also removes redundant test typedcssom/inlinestyle/left-top-right-bottom.html 
(superceded by typedcssom/inlinestyle/properties/bottom.html; will
add the others later)


BUG=545318

Review-Url: https://codereview.chromium.org/2371673002
Cr-Commit-Position: refs/heads/master@{#443133}
Committed:
https://chromium.googlesource.com/chromium/src/+/857445ffbc21f9cac8a018128a3a...
==========

[commit-bot: I haz the power, 2017-01-12 02:59:25]

Committed patchset #18 (id:340001) as
https://chromium.googlesource.com/chromium/src/+/857445ffbc21f9cac8a018128a3a...