Implement animation tainted custom property values

Implement animation tainted custom property values

This patch updates the CSSVariableResolver to be aware of animation tainted
custom property values and to support disallowing custom property
substitution when a referenced value has been animation tainted.

Relevant spec: https://drafts.csswg.org/css-variables/#substitute-a-var
The spec currently references the out of date notion of animation tainted
custom properties rather than tainted values. This implementation reflects
a future amendment of the spec.

Because animations are the only source of animation tainted values and our
implementation doesn't support animations setting custom property values
there is no user-visible change made by this patch.
The (failing) layout test shows the kind of behaviour this patch is
intended to support.

BUG=644148
Committed: https://crrev.com/87009415889d2164713316a5e5283e484570f6fd
Cr-Commit-Position: refs/heads/master@{#423469}

[alancutter (OOO until 2018), 2016-10-05 04:28:39]

Description was changed from

==========
[WIP] animationTaintedCustomProperties

BUG=
==========

to

==========
Implement animation tainted custom property values

This patch updates the CSSVariableResolver to be aware of animation tainted
custom property values and to support omitting custom property substitution
when the value has been animation tainted.

Relevant spec: https://drafts.csswg.org/css-variables/#substitute-a-var
The spec currently references the out of date notion of animation tainted
custom properties rather than tainted values. This implementation reflects
a future amendment of the spec.

Because animations are the only source of animation tainted values and our
implementation doesn't support animations setting custom property values
there is no user-visible change made by this patch.
The (failing) layout test shows the kind of behaviour this patch is intended
to support.

BUG=644148
==========

[alancutter (OOO until 2018), 2016-10-05 04:28:39]

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

[alancutter (OOO until 2018), 2016-10-05 04:29:05]

Description was changed from

==========
Implement animation tainted custom property values

This patch updates the CSSVariableResolver to be aware of animation tainted
custom property values and to support omitting custom property substitution
when the value has been animation tainted.

Relevant spec: https://drafts.csswg.org/css-variables/#substitute-a-var
The spec currently references the out of date notion of animation tainted
custom properties rather than tainted values. This implementation reflects
a future amendment of the spec.

Because animations are the only source of animation tainted values and our
implementation doesn't support animations setting custom property values
there is no user-visible change made by this patch.
The (failing) layout test shows the kind of behaviour this patch is intended
to support.

BUG=644148
==========

to

==========
Implement animation tainted custom property values

This patch updates the CSSVariableResolver to be aware of animation tainted
custom property values and to support omitting custom property substitution
when a referenced value has been animation tainted.

Relevant spec: https://drafts.csswg.org/css-variables/#substitute-a-var
The spec currently references the out of date notion of animation tainted
custom properties rather than tainted values. This implementation reflects
a future amendment of the spec.

Because animations are the only source of animation tainted values and our
implementation doesn't support animations setting custom property values
there is no user-visible change made by this patch.
The (failing) layout test shows the kind of behaviour this patch is intended
to support.

BUG=644148
==========

[Timothy Loh, 2016-10-05 07:31:37]

lgtm

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/animations/custom-property-value-tainting.html
(right):

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/animations/custom-property-value-tainting.html:28:
animationName: 'untainted',
These don't look right, shouldn't var(--tainted) be invalid -> animationName
won't be set?

[alancutter (OOO until 2018), 2016-10-05 07:46:40]

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/animations/custom-property-value-tainting.html
(right):

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/animations/custom-property-value-tainting.html:46:
setTaintedValue(parent, '--tainted', ', , tainted');
Not sure why there's an extra comma.

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

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.cpp:133: if
(!omitAnimationTainted || !variableData->isAnimationTainted()) {
Negate this and return so it reads better and doesn't need an else.

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.cpp:137: //
TODO(alancutter): Append the registered initial custom property value.
This should be calling resolveFallback instead of discarding it.

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.h (right):

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.h:35: bool
omitAnimationTainted);
This should actually be disallowAnimationTainted.

[alancutter (OOO until 2018), 2016-10-05 21:58:15]

Description was changed from

==========
Implement animation tainted custom property values

This patch updates the CSSVariableResolver to be aware of animation tainted
custom property values and to support omitting custom property substitution
when a referenced value has been animation tainted.

Relevant spec: https://drafts.csswg.org/css-variables/#substitute-a-var
The spec currently references the out of date notion of animation tainted
custom properties rather than tainted values. This implementation reflects
a future amendment of the spec.

Because animations are the only source of animation tainted values and our
implementation doesn't support animations setting custom property values
there is no user-visible change made by this patch.
The (failing) layout test shows the kind of behaviour this patch is intended
to support.

BUG=644148
==========

to

==========
Implement animation tainted custom property values

This patch updates the CSSVariableResolver to be aware of animation tainted
custom property values and to support disallowing custom property
substitution when a referenced value has been animation tainted.

Relevant spec: https://drafts.csswg.org/css-variables/#substitute-a-var
The spec currently references the out of date notion of animation tainted
custom properties rather than tainted values. This implementation reflects
a future amendment of the spec.

Because animations are the only source of animation tainted values and our
implementation doesn't support animations setting custom property values
there is no user-visible change made by this patch.
The (failing) layout test shows the kind of behaviour this patch is
intended to support.

BUG=644148
==========

[alancutter (OOO until 2018), 2016-10-06 01:25:33]

Verified that these tests actually pass in the custom property animation
implementation.

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/animations/custom-property-value-tainting.html
(right):

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/animations/custom-property-value-tainting.html:28:
animationName: 'untainted',
On 2016/10/05 at 07:31:37, Timothy Loh wrote:
> These don't look right, shouldn't var(--tainted) be invalid -> animationName
won't be set?

Done.

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/animations/custom-property-value-tainting.html:46:
setTaintedValue(parent, '--tainted', ', , tainted');
On 2016/10/05 at 07:46:40, alancutter wrote:
> Not sure why there's an extra comma.

Removed commas from test values.

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

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.cpp:133: if
(!omitAnimationTainted || !variableData->isAnimationTainted()) {
On 2016/10/05 at 07:46:40, alancutter wrote:
> Negate this and return so it reads better and doesn't need an else.

Done.

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.cpp:137: //
TODO(alancutter): Append the registered initial custom property value.
On 2016/10/05 at 07:46:40, alancutter wrote:
> This should be calling resolveFallback instead of discarding it.

Done.

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.h (right):

https://codereview.chromium.org/2323633002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.h:35: bool
omitAnimationTainted);
On 2016/10/05 at 07:46:40, alancutter wrote:
> This should actually be disallowAnimationTainted.

Done.

[alancutter (OOO until 2018), 2016-10-06 01:25:35]

Verified that these tests actually pass in the custom property animation
implementation.

[alancutter (OOO until 2018), 2016-10-06 01:58:43]

The CQ bit was checked by alancutter@chromium.org

[alancutter (OOO until 2018), 2016-10-06 01:58:44]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/2323633002/#ps180001
(title: "Make tests work")

[commit-bot: I haz the power, 2016-10-06 01:59:23]

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-06 03:36:30]

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

[commit-bot: I haz the power, 2016-10-06 03:36:31]

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

[alancutter (OOO until 2018), 2016-10-06 06:25:26]

The CQ bit was checked by alancutter@chromium.org

[alancutter (OOO until 2018), 2016-10-06 06:25:26]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/2323633002/#ps200001
(title: "Rebased")

[commit-bot: I haz the power, 2016-10-06 06:25:49]

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-06 07:41:17]

Committed patchset #11 (id:200001)

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

Description was changed from

==========
Implement animation tainted custom property values

This patch updates the CSSVariableResolver to be aware of animation tainted
custom property values and to support disallowing custom property
substitution when a referenced value has been animation tainted.

Relevant spec: https://drafts.csswg.org/css-variables/#substitute-a-var
The spec currently references the out of date notion of animation tainted
custom properties rather than tainted values. This implementation reflects
a future amendment of the spec.

Because animations are the only source of animation tainted values and our
implementation doesn't support animations setting custom property values
there is no user-visible change made by this patch.
The (failing) layout test shows the kind of behaviour this patch is
intended to support.

BUG=644148
==========

to

==========
Implement animation tainted custom property values

This patch updates the CSSVariableResolver to be aware of animation tainted
custom property values and to support disallowing custom property
substitution when a referenced value has been animation tainted.

Relevant spec: https://drafts.csswg.org/css-variables/#substitute-a-var
The spec currently references the out of date notion of animation tainted
custom properties rather than tainted values. This implementation reflects
a future amendment of the spec.

Because animations are the only source of animation tainted values and our
implementation doesn't support animations setting custom property values
there is no user-visible change made by this patch.
The (failing) layout test shows the kind of behaviour this patch is
intended to support.

BUG=644148

Committed: https://crrev.com/87009415889d2164713316a5e5283e484570f6fd
Cr-Commit-Position: refs/heads/master@{#423469}
==========

[commit-bot: I haz the power, 2016-10-06 07:42:51]

Patchset 11 (id:??) landed as
https://crrev.com/87009415889d2164713316a5e5283e484570f6fd
Cr-Commit-Position: refs/heads/master@{#423469}