Fix SVG {text, foreignObject} transform for css animation

No Implement to check whether needs transform or not on the
LayoutSVGBlock.
Implement this one which check hasTransform. If has them,
should change transform matrix using setNeedsTransformUpdate.

BUG=537121
Committed: https://crrev.com/9471053555d605a19e0c3ec604d69aa81fe5ffde
Cr-Commit-Position: refs/heads/master@{#368232}

[hyunjunekim2, 2015-12-29 02:21:59]

Description was changed from

==========
WIP

BUG=537121
==========

to

==========
WIP

BUG=537121
==========

[hyunjunekim2, 2015-12-29 02:22:00]

hyunjune.kim@samsung.com changed reviewers:
+ fs@opera.com, pdr@chromium.org

[hyunjunekim2, 2016-01-02 15:03:26]

Description was changed from

==========
WIP

BUG=537121
==========

to

==========
No Implement styleDidChange for LayoutSVGText.
After override styleDidChange from LayoutSVGBlock, Implement
this one which check needsFullLayout and hasTransform.
If has them, should change transform matrix.

BUG=537121
==========

[hyunjunekim2, 2016-01-02 15:04:00]

fs, pdr
Could you check this patch and give me advice?
Thank you.

[fs, 2016-01-02 16:41:13]

https://codereview.chromium.org/1553473002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/repaint/transform-text.html (right):

https://codereview.chromium.org/1553473002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/repaint/transform-text.html:10: ani =
text.animate([{transform: 'translate(0px, 50px)'},{transform: 'translate(200px,
50px)'}], { duration: 2000, iterations: 1});
Does this bug actually need an animation to reproduce - doesn't just setting the
style suffice?

I.e:

myTextElement.style.transform = 'translate(200px,50px)";

That would make for a simpler test, and preferbly a reftest. If this bug affects
<foreignObject> too, then adding a test for that too would be good.

https://codereview.chromium.org/1553473002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp (right):

https://codereview.chromium.org/1553473002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp:474: if
(style()->hasTransform())
Since LayoutSVGBlock doesn't do this, then presumably <foreignObject> suffer
from the same issue. If so the fix should probably be in LayoutSVGBlock.

https://codereview.chromium.org/1553473002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp:478:
SVGResourcesCache::clientStyleChanged(this, diff, styleRef());
The call to the super-class (just above) would've achieved this I think.

[hyunjunekim2, 2016-01-04 15:07:06]

fs, Could you check PS6? Thank you.

[fs, 2016-01-04 19:29:57]

On 2016/01/04 at 15:07:06, hyunjune.kim wrote:
> fs, Could you check PS6? Thank you.

Code change LG, but I'd still prefer the test to be split into two (one for
<text> and one for <foreignObject> and made reftests (or at least non-pixel
tests.)

[hyunjunekim2, 2016-01-05 13:50:49]

Description was changed from

==========
No Implement styleDidChange for LayoutSVGText.
After override styleDidChange from LayoutSVGBlock, Implement
this one which check needsFullLayout and hasTransform.
If has them, should change transform matrix.

BUG=537121
==========

to

==========
No Implement to check whether needs transform or not on the
LayoutSVGBlock.
Implement this one which check hasTransform. If has them, 
should change transform matrix using setNeedsTransformUpdate.

BUG=537121
==========

[hyunjunekim2, 2016-01-05 13:52:07]

fs, Could you check PS7? Thank you for your review :).

[hyunjunekim2, 2016-01-06 13:35:53]

fs, If you are possible, Could you review PS 19? Thank you. :)

[fs, 2016-01-07 11:38:46]

I'd still prefer reftests, but I guess we can roll like this...

https://codereview.chromium.org/1553473002/diff/350001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/svg/repaint/transform-text-element.svg
(right):

https://codereview.chromium.org/1553473002/diff/350001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/repaint/transform-text-element.svg:2: <script
xlink:href="../../fast/repaint/resources/text-based-repaint.js"></script>
You should also include the resources/ahem.js script here as well, to guard
against flakiness.

https://codereview.chromium.org/1553473002/diff/350001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/repaint/transform-text-element.svg:4:
<![CDATA[
(If you make these tests HTML w/ inline SVG instead you wouldn't need this
abomination/uglyness.)

https://codereview.chromium.org/1553473002/diff/350001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/repaint/transform-text-element.svg:11: <text
x="10" y="90" font-size="100" font-family="Ahem">Test</text>
IIRC, the "Ahem" font has an ascent of 800 (of 1000 units-per-em), so using y=80
would be the more "natural choice".

(You could also make this behave the same as the foreignObject test by using
x=100 and make the transform cancel that out - which I find to be a good
strategy. It should make for simpler to check repaint rects. It'd also be
preferable to avoid clipping here - i.e either use a larger viewport or shorter
text.)

[hyunjunekim2, 2016-01-07 16:05:35]

fs, Could you check PS21? Thank you.

[fs, 2016-01-07 16:18:29]

LGTM w/ nits

https://codereview.chromium.org/1553473002/diff/390001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/svg/repaint/transform-foreign-object.html
(right):

https://codereview.chromium.org/1553473002/diff/390001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/repaint/transform-foreign-object.html:3:
<script src="../../resources/run-after-layout-and-paint.js"></script>
Nit: Not needed/used.

https://codereview.chromium.org/1553473002/diff/390001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/svg/repaint/transform-text-element.html
(right):

https://codereview.chromium.org/1553473002/diff/390001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/repaint/transform-text-element.html:3:
<script src="../../resources/run-after-layout-and-paint.js"></script>
Ditto.

[hyunjunekim2, 2016-01-07 23:01:09]

I changed it. I am going to land this patch. Thank you.

[hyunjunekim2, 2016-01-07 23:01:24]

The CQ bit was checked by hyunjune.kim@samsung.com

[hyunjunekim2, 2016-01-07 23:01:24]

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/1553473002/#ps410001
(title: " ")

[commit-bot: I haz the power, 2016-01-07 23:02:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1553473002/410001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1553473002/410001

[hyunjunekim2, 2016-01-07 23:26:14]

The CQ bit was unchecked by hyunjune.kim@samsung.com

[hyunjunekim2, 2016-01-08 01:21:22]

The CQ bit was checked by hyunjune.kim@samsung.com

[commit-bot: I haz the power, 2016-01-08 01:22:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1553473002/410001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1553473002/410001

[commit-bot: I haz the power, 2016-01-08 01:27:41]

Description was changed from

==========
No Implement to check whether needs transform or not on the
LayoutSVGBlock.
Implement this one which check hasTransform. If has them, 
should change transform matrix using setNeedsTransformUpdate.

BUG=537121
==========

to

==========
No Implement to check whether needs transform or not on the
LayoutSVGBlock.
Implement this one which check hasTransform. If has them, 
should change transform matrix using setNeedsTransformUpdate.

BUG=537121
==========

[commit-bot: I haz the power, 2016-01-08 01:27:42]

Committed patchset #22 (id:410001)

[commit-bot: I haz the power, 2016-01-08 01:28:38]

Description was changed from

==========
No Implement to check whether needs transform or not on the
LayoutSVGBlock.
Implement this one which check hasTransform. If has them, 
should change transform matrix using setNeedsTransformUpdate.

BUG=537121
==========

to

==========
No Implement to check whether needs transform or not on the
LayoutSVGBlock.
Implement this one which check hasTransform. If has them,
should change transform matrix using setNeedsTransformUpdate.

BUG=537121

Committed: https://crrev.com/9471053555d605a19e0c3ec604d69aa81fe5ffde
Cr-Commit-Position: refs/heads/master@{#368232}
==========

[commit-bot: I haz the power, 2016-01-08 01:28:39]

Patchset 22 (id:??) landed as
https://crrev.com/9471053555d605a19e0c3ec604d69aa81fe5ffde
Cr-Commit-Position: refs/heads/master@{#368232}