SVG: Use path attribute for animateMotion

SVG: Use path attribute for animateMotion

Avoids a bug where the path attribute was updated but the style had not
yet been recalculated. The SMIL animateMotion element was notified of
the update but then read the now-invalid style.

BUG=626935
Committed: https://crrev.com/403a0d6c1bcfa02363f764acd1ba56145263bafa
Cr-Commit-Position: refs/heads/master@{#411264}

[Eric Willigers, 2016-08-08 02:51:04]

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

[Eric Willigers, 2016-08-08 03:53:47]

Patchset #1 (id:1) has been deleted

[fs, 2016-08-08 15:45:39]

LGTM w/ test pondering

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html
(right):

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:8:
<svg width="200" height="200" xmlns="http://www.w3.org/2000/svg"
Nit: Don't need to declare NS's (the HTML parser open them automatically.)

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:24:
requestAnimationFrame(function() {
If we can do the thing below, then I think we could use the
run-after-layout-and-paint.js helper here, then it'd just be:

runAfterLayoutAndPaint(function() {
  document.getElementById('track').setAttribute('d', ...);
}, true);

(whether that is bound to happen after onload might be debatable, but equally so
I think to using rAF.)

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:28:
setTimeout(function(){testRunner.notifyDone();}, 100);
Could we avoid the 100ms delay by pausing the timeline and using the point at
t=0?

[Eric Willigers, 2016-08-10 01:13:15]

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html
(right):

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:8:
<svg width="200" height="200" xmlns="http://www.w3.org/2000/svg"
On 2016/08/08 15:45:38, fs (OoO until Aug 15) wrote:
> Nit: Don't need to declare NS's (the HTML parser open them automatically.)

Done.

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:24:
requestAnimationFrame(function() {
On 2016/08/08 15:45:38, fs (OoO until Aug 15) wrote:
> If we can do the thing below, then I think we could use the
> run-after-layout-and-paint.js helper here, then it'd just be:
> 
> runAfterLayoutAndPaint(function() {
>   document.getElementById('track').setAttribute('d', ...);
> }, true);
> 
> (whether that is bound to happen after onload might be debatable, but equally
so
> I think to using rAF.)

Done.

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:28:
setTimeout(function(){testRunner.notifyDone();}, 100);
On 2016/08/08 15:45:38, fs (OoO until Aug 15) wrote:
> Could we avoid the 100ms delay by pausing the timeline and using the point at
> t=0?

The animation has fill="freeze", so now I simply update current time to when the
animation completes.

[fs, 2016-08-10 15:32:16]

https://codereview.chromium.org/2227463002/diff/20001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:28:
setTimeout(function(){testRunner.notifyDone();}, 100);
> On 2016/08/08 15:45:38, fs (OoO until Aug 15) wrote:
> > Could we avoid the 100ms delay by pausing the timeline and using the point
at
> > t=0?
> 
> The animation has fill="freeze", so now I simply update current time to when
the animation completes.

Great! LGTM++

https://codereview.chromium.org/2227463002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html
(right):

https://codereview.chromium.org/2227463002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:22:
testRunner.waitUntilDone();
Nit: Not needed (done by the 'true' second argument to runAfter...)

[Eric Willigers, 2016-08-10 23:58:24]

https://codereview.chromium.org/2227463002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html
(right):

https://codereview.chromium.org/2227463002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/animateMotion_changingPath.html:22:
testRunner.waitUntilDone();
On 2016/08/10 15:32:16, fs (OoO until Aug 15) wrote:
> Nit: Not needed (done by the 'true' second argument to runAfter...)

Done.

[Eric Willigers, 2016-08-11 03:18:49]

The CQ bit was checked by ericwilligers@chromium.org

[Eric Willigers, 2016-08-11 03:18:49]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/2227463002/#ps60001
(title: "implicitWait")

[commit-bot: I haz the power, 2016-08-11 03:19:07]

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-11 04:28:04]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-08-11 05:08:06]

Description was changed from

==========
SVG: Use path attribute for animateMotion

Avoids a bug where the path attribute was updated but the style had not
yet been recalculated. The SMIL animateMotion element was notified of
the update but then read the now-invalid style.

BUG=626935
==========

to

==========
SVG: Use path attribute for animateMotion

Avoids a bug where the path attribute was updated but the style had not
yet been recalculated. The SMIL animateMotion element was notified of
the update but then read the now-invalid style.

BUG=626935

Committed: https://crrev.com/403a0d6c1bcfa02363f764acd1ba56145263bafa
Cr-Commit-Position: refs/heads/master@{#411264}
==========

[commit-bot: I haz the power, 2016-08-11 05:08:08]

Patchset 3 (id:??) landed as
https://crrev.com/403a0d6c1bcfa02363f764acd1ba56145263bafa
Cr-Commit-Position: refs/heads/master@{#411264}