Fold SMIL animation value application helpers and simplify

Fold SMIL animation value application helpers and simplify

Folds the following helper functions:

  applyCSSPropertyToTargetAndInstances
  removeCSSPropertyFromTargetAndInstances
  notifyTargetAndInstancesAboutAnimValChange

into their users, hoisting common predicates and simplifies
accordingly.

BUG=640676
Committed: https://crrev.com/cfa5adc96879fea39db27d190261e82ae3b85624
Cr-Commit-Position: refs/heads/master@{#414808}

[fs, 2016-08-25 21:15:30]

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

[commit-bot: I haz the power, 2016-08-25 21:15:59]

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

[fs, 2016-08-25 22:21:49]

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

[commit-bot: I haz the power, 2016-08-25 22:22:37]

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

[fs, 2016-08-25 23:12:15]

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

[commit-bot: I haz the power, 2016-08-25 23:52:13]

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

[commit-bot: I haz the power, 2016-08-25 23:52:14]

Dry run: This issue passed the CQ dry run.

[pdr., 2016-08-26 01:28:12]

Much simpler! Just a small suggestion

https://codereview.chromium.org/2285473002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGAnimateElement.cpp (right):

https://codereview.chromium.org/2285473002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGAnimateElement.cpp:243: if
(targetIsUsable(targetElement, attributeName()))
The attribute check is sort of unrelated to whether the target is usable. WDYT
of pulling "attribute != anyQName()" out and relying on shouldApply which has a
similar check?

Something like:
if (shouldApply != DontApplyAnimation && targetIsUsable(targetElement))
   ...

[fs, 2016-08-26 08:45:27]

https://codereview.chromium.org/2285473002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGAnimateElement.cpp (right):

https://codereview.chromium.org/2285473002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGAnimateElement.cpp:243: if
(targetIsUsable(targetElement, attributeName()))
On 2016/08/26 at 01:28:12, pdr. wrote:
> The attribute check is sort of unrelated to whether the target is usable. WDYT
of pulling "attribute != anyQName()" out and relying on shouldApply which has a
similar check?
> 
> Something like:
> if (shouldApply != DontApplyAnimation && targetIsUsable(targetElement))
>    ...

Well, we could make that "shouldApply != DontApplyAnimation &&
targetElement->parentNode()" even (shouldApplyAnimation checks inActiveDocument
too), but technically speaking it's a behavioral change. I'd be happy to do this
as a follow up though (probably by merging targetIsUsable into
shouldApplyAnimation and replace checks with the first part of the conjunction
above.)

(In general I consider "target" in this context to be the <element, attribute>
pair.)

[pdr., 2016-08-26 21:05:24]

On 2016/08/26 at 08:45:27, fs wrote:
>
https://codereview.chromium.org/2285473002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/svg/SVGAnimateElement.cpp (right):
> 
>
https://codereview.chromium.org/2285473002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/svg/SVGAnimateElement.cpp:243: if
(targetIsUsable(targetElement, attributeName()))
> On 2016/08/26 at 01:28:12, pdr. wrote:
> > The attribute check is sort of unrelated to whether the target is usable.
WDYT of pulling "attribute != anyQName()" out and relying on shouldApply which
has a similar check?
> > 
> > Something like:
> > if (shouldApply != DontApplyAnimation && targetIsUsable(targetElement))
> >    ...
> 
> Well, we could make that "shouldApply != DontApplyAnimation &&
targetElement->parentNode()" even (shouldApplyAnimation checks inActiveDocument
too), but technically speaking it's a behavioral change. I'd be happy to do this
as a follow up though (probably by merging targetIsUsable into
shouldApplyAnimation and replace checks with the first part of the conjunction
above.)
> 
> (In general I consider "target" in this context to be the <element, attribute>
pair.)

OK LGTM

[fs, 2016-08-26 21:13:47]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-08-26 21:14:39]

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-08-26 21:20:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-26 21:22:36]

Description was changed from

==========
Fold SMIL animation value application helpers and simplify

Folds the following helper functions:

  applyCSSPropertyToTargetAndInstances
  removeCSSPropertyFromTargetAndInstances
  notifyTargetAndInstancesAboutAnimValChange

into their users, hoisting common predicates and simplifies
accordingly.

BUG=640676
==========

to

==========
Fold SMIL animation value application helpers and simplify

Folds the following helper functions:

  applyCSSPropertyToTargetAndInstances
  removeCSSPropertyFromTargetAndInstances
  notifyTargetAndInstancesAboutAnimValChange

into their users, hoisting common predicates and simplifies
accordingly.

BUG=640676

Committed: https://crrev.com/cfa5adc96879fea39db27d190261e82ae3b85624
Cr-Commit-Position: refs/heads/master@{#414808}
==========

[commit-bot: I haz the power, 2016-08-26 21:22:37]

Patchset 2 (id:??) landed as
https://crrev.com/cfa5adc96879fea39db27d190261e82ae3b85624
Cr-Commit-Position: refs/heads/master@{#414808}