cc : Stop pushing transform origin from Layer and LayerImpl.

cc :  Stop pushing transform origin from Layer and LayerImpl.

This CL also adds a new LayerImplTestProperties struct and moves the
LayerImpl's transform origin to that.

BUG=602438
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/38be7a828db674dd70d332c62f313b6170bcbd01
Cr-Commit-Position: refs/heads/master@{#388782}

[jaydasika, 2016-04-20 17:35:05]

Description was changed from

==========
cc :  Stop pushing transform origin from Layer and LayerImpl.

This CL also adds a new LayerImplTestProperties struct and moves the
LayerImpl's transform origin to that.

BUG=602438
==========

to

==========
cc :  Stop pushing transform origin from Layer and LayerImpl.

This CL also adds a new LayerImplTestProperties struct and moves the
LayerImpl's transform origin to that.

BUG=602438
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-20 17:37:50]

jaydasika@chromium.org changed reviewers:
+ ajuma@chromium.org, vollick@chromium.org, weiliangc@chromium.org

[jaydasika, 2016-04-20 17:37:50]

PTAL

[ajuma, 2016-04-20 18:09:53]

https://codereview.chromium.org/1908593002/diff/20001/cc/layers/layer_impl.h
File cc/layers/layer_impl.h (right):

https://codereview.chromium.org/1908593002/diff/20001/cc/layers/layer_impl.h#...
cc/layers/layer_impl.h:243: gfx::Point3F transform_origin_for_testing() const {
This approach is fine for this CL, but as we move (or after we move) more things
into LayerImplTestProperties, we should move these getters/setters onto that
class, and only expose TestProperties() getter on LayerImpl (that returns a
lazily allocated LayerImplTestProperties instance).

https://codereview.chromium.org/1908593002/diff/20001/cc/layers/layer_impl.h#...
cc/layers/layer_impl.h:644: struct LayerImplTestProperties* test_properties_;
This needs to be a unique_ptr.

[jaydasika, 2016-04-20 18:46:06]

https://codereview.chromium.org/1908593002/diff/20001/cc/layers/layer_impl.h
File cc/layers/layer_impl.h (right):

https://codereview.chromium.org/1908593002/diff/20001/cc/layers/layer_impl.h#...
cc/layers/layer_impl.h:243: gfx::Point3F transform_origin_for_testing() const {
On 2016/04/20 18:09:52, ajuma wrote:
> This approach is fine for this CL, but as we move (or after we move) more
things
> into LayerImplTestProperties, we should move these getters/setters onto that
> class, and only expose TestProperties() getter on LayerImpl (that returns a
> lazily allocated LayerImplTestProperties instance).

Done.

https://codereview.chromium.org/1908593002/diff/20001/cc/layers/layer_impl.h#...
cc/layers/layer_impl.h:644: struct LayerImplTestProperties* test_properties_;
On 2016/04/20 18:09:52, ajuma wrote:
> This needs to be a unique_ptr.

Done.

[ajuma, 2016-04-20 18:51:03]

Thanks! lgtm

[jaydasika, 2016-04-20 18:57:33]

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

[commit-bot: I haz the power, 2016-04-20 18:57:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1908593002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1908593002/40001

[jaydasika, 2016-04-20 19:01:11]

On 2016/04/20 18:57:53, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1908593002/40001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1908593002/40001

vollick : Can you review changes to
third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp ?

[commit-bot: I haz the power, 2016-04-20 20:28:43]

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

[commit-bot: I haz the power, 2016-04-20 20:28:44]

Dry run: This issue passed the CQ dry run.

[ajuma, 2016-04-20 22:01:12]

A couple more comments:

https://codereview.chromium.org/1908593002/diff/40001/cc/layers/layer_impl_un...
File cc/layers/layer_impl_unittest.cc (right):

https://codereview.chromium.org/1908593002/diff/40001/cc/layers/layer_impl_un...
cc/layers/layer_impl_unittest.cc:248: root->test_properties()->transform_origin
= arbitrary_point_3f);
Given that this has now moved to test_properties, I think we can delete this
line altogether.

https://codereview.chromium.org/1908593002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp
(right):

https://codereview.chromium.org/1908593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp:47:
layer->test_properties()->transform_origin = gfx::Point3F();
Perhaps this line can be removed altogether? (These tests don't change transform
origin anywhere else.)

[jaydasika, 2016-04-20 22:05:44]

https://codereview.chromium.org/1908593002/diff/40001/cc/layers/layer_impl_un...
File cc/layers/layer_impl_unittest.cc (right):

https://codereview.chromium.org/1908593002/diff/40001/cc/layers/layer_impl_un...
cc/layers/layer_impl_unittest.cc:248: root->test_properties()->transform_origin
= arbitrary_point_3f);
On 2016/04/20 22:01:12, ajuma wrote:
> Given that this has now moved to test_properties, I think we can delete this
> line altogether.

Done.

https://codereview.chromium.org/1908593002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp
(right):

https://codereview.chromium.org/1908593002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp:47:
layer->test_properties()->transform_origin = gfx::Point3F();
On 2016/04/20 22:01:12, ajuma wrote:
> Perhaps this line can be removed altogether? (These tests don't change
transform
> origin anywhere else.)

Done.

[Ian Vollick, 2016-04-21 14:29:49]

On 2016/04/20 22:05:44, jaydasika wrote:
>
https://codereview.chromium.org/1908593002/diff/40001/cc/layers/layer_impl_un...
> File cc/layers/layer_impl_unittest.cc (right):
> 
>
https://codereview.chromium.org/1908593002/diff/40001/cc/layers/layer_impl_un...
> cc/layers/layer_impl_unittest.cc:248:
root->test_properties()->transform_origin
> = arbitrary_point_3f);
> On 2016/04/20 22:01:12, ajuma wrote:
> > Given that this has now moved to test_properties, I think we can delete this
> > line altogether.
> 
> Done.
> 
>
https://codereview.chromium.org/1908593002/diff/40001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp
> (right):
> 
>
https://codereview.chromium.org/1908593002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp:47:
> layer->test_properties()->transform_origin = gfx::Point3F();
> On 2016/04/20 22:01:12, ajuma wrote:
> > Perhaps this line can be removed altogether? (These tests don't change
> transform
> > origin anywhere else.)
> 
> Done.

platform/graphics/ lgtm.

[jaydasika, 2016-04-21 14:31:02]

The CQ bit was checked by jaydasika@chromium.org

[jaydasika, 2016-04-21 14:31:03]

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

[commit-bot: I haz the power, 2016-04-21 14:31:45]

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

[commit-bot: I haz the power, 2016-04-21 16:07:22]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-22 19:35:02]

Description was changed from

==========
cc :  Stop pushing transform origin from Layer and LayerImpl.

This CL also adds a new LayerImplTestProperties struct and moves the
LayerImpl's transform origin to that.

BUG=602438
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc :  Stop pushing transform origin from Layer and LayerImpl.

This CL also adds a new LayerImplTestProperties struct and moves the
LayerImpl's transform origin to that.

BUG=602438
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/38be7a828db674dd70d332c62f313b6170bcbd01
Cr-Commit-Position: refs/heads/master@{#388782}
==========

[commit-bot: I haz the power, 2016-04-22 19:35:04]

Patchset 4 (id:??) landed as
https://crrev.com/38be7a828db674dd70d332c62f313b6170bcbd01
Cr-Commit-Position: refs/heads/master@{#388782}