cc : Add LayerToPropertyTreeIndices map to PropertyTrees

cc : Add LayerToPropertyTreeIndices map to PropertyTrees

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index.

BUG=611936
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6f47e3b18ac6b9084799d2d5978c6ef831fa5a07
Cr-Commit-Position: refs/heads/master@{#393646}

[jaydasika, 2016-05-12 00:30:52]

Description was changed from

==========
cc : Add LayerToPropertyTreeIndices map to LayerTreeImpl

This is dead code for now. The map is populated but not used.

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

to

==========
cc : Add LayerToPropertyTreeIndices map to LayerTreeImpl

This is dead code for now. The map is populated but not used.

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

[jaydasika, 2016-05-12 00:36:20]

Description was changed from

==========
cc : Add LayerToPropertyTreeIndices map to LayerTreeImpl

This is dead code for now. The map is populated but not used.

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

to

==========
cc : Add LayerToPropertyTreeIndices map to LayerTreeImpl

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index. 

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

[jaydasika, 2016-05-12 00:36:50]

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

[jaydasika, 2016-05-12 00:36:51]

PTAL

[ajuma, 2016-05-12 00:54:56]

Is this intending to build a map from all layer ids to their property tree
index, or only from layers that own nodes? I think the latter is what we need
(that is, we need a map from owner id to node index). How about building this
map as part of property tree building (we already know the node owner ids
there), and then pushing the map over (as part of PropertyTrees, say)? Please
also add tests making sure the map gets created/updated/pushed with correct
values.

https://codereview.chromium.org/1975623002/diff/1/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/1975623002/diff/1/cc/layers/layer_impl.cc#new...
cc/layers/layer_impl.cc:185: if (layer_tree_impl_)
Is layer_tree_impl_ ever null? It's assumed to be non-null in lots of places in
LayerImpl.

[jaydasika, 2016-05-12 19:47:47]

On 2016/05/12 00:54:56, ajuma wrote:
> Is this intending to build a map from all layer ids to their property tree
> index, or only from layers that own nodes? I think the latter is what we need
> (that is, we need a map from owner id to node index). How about building this
> map as part of property tree building (we already know the node owner ids
> there), and then pushing the map over (as part of PropertyTrees, say)? Please
> also add tests making sure the map gets created/updated/pushed with correct
> values.
> 
> https://codereview.chromium.org/1975623002/diff/1/cc/layers/layer_impl.cc
> File cc/layers/layer_impl.cc (right):
> 
>
https://codereview.chromium.org/1975623002/diff/1/cc/layers/layer_impl.cc#new...
> cc/layers/layer_impl.cc:185: if (layer_tree_impl_)
> Is layer_tree_impl_ ever null? It's assumed to be non-null in lots of places
in
> LayerImpl.

I have moved the maps to property trees and they are populated during property
tree building. I have also added separate maps for each property tree. (It might
be easier to review the new patch as a new CL rather than as a diff from the
first patch).

[ajuma, 2016-05-13 14:38:53]

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree....
cc/trees/property_tree.cc:1929: scroll_node_owner_to_node_map =
from.scroll_node_owner_to_node_map;
Do these also need to be handled in ToProtobuf and FromProtobuf?

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree.h
File cc/trees/property_tree.h (right):

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree....
cc/trees/property_tree.h:671: std::unordered_map<int, int>
scroll_node_owner_to_node_map;
Nit: how about using names like "transform_id_to_index_map" so they continue to
make sense for trees built by Blink (where nodes won't have an 'owner')?

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree_...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:65: std::unordered_map<int, int>*
scroll_node_owner_to_node_map;
DataForRecursion is starting to get pretty large. In a follow-up, how about
adding PropertyTrees* to this and then removing all the fields this gives us
from DataForRecursion?

[jaydasika, 2016-05-13 19:40:39]

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree....
cc/trees/property_tree.cc:1929: scroll_node_owner_to_node_map =
from.scroll_node_owner_to_node_map;
On 2016/05/13 14:38:53, ajuma wrote:
> Do these also need to be handled in ToProtobuf and FromProtobuf?

I have added code to reconstruct the map rather than sending it as we anyway
walk all property trees in FromProtobuf

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree.h
File cc/trees/property_tree.h (right):

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree....
cc/trees/property_tree.h:671: std::unordered_map<int, int>
scroll_node_owner_to_node_map;
On 2016/05/13 14:38:53, ajuma wrote:
> Nit: how about using names like "transform_id_to_index_map" so they continue
to
> make sense for trees built by Blink (where nodes won't have an 'owner')?

Done.

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree_...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/1975623002/diff/40001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:65: std::unordered_map<int, int>*
scroll_node_owner_to_node_map;
On 2016/05/13 14:38:53, ajuma wrote:
> DataForRecursion is starting to get pretty large. In a follow-up, how about
> adding PropertyTrees* to this and then removing all the fields this gives us
> from DataForRecursion?

Acknowledged.

[jaydasika, 2016-05-13 19:41:57]

Description was changed from

==========
cc : Add LayerToPropertyTreeIndices map to LayerTreeImpl

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index. 

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

to

==========
cc : Add LayerToPropertyTreeIndices map to PropertyTrees

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index. 

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

[ajuma, 2016-05-13 20:08:52]

Thanks, lgtm

[jaydasika, 2016-05-13 20:48:17]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-05-13 20:48:42]

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

[commit-bot: I haz the power, 2016-05-13 21:39:44]

Description was changed from

==========
cc : Add LayerToPropertyTreeIndices map to PropertyTrees

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index. 

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

to

==========
cc : Add LayerToPropertyTreeIndices map to PropertyTrees

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index. 

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

[commit-bot: I haz the power, 2016-05-13 21:39:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-13 21:41:11]

Description was changed from

==========
cc : Add LayerToPropertyTreeIndices map to PropertyTrees

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index. 

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

to

==========
cc : Add LayerToPropertyTreeIndices map to PropertyTrees

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6f47e3b18ac6b9084799d2d5978c6ef831fa5a07
Cr-Commit-Position: refs/heads/master@{#393646}
==========

[commit-bot: I haz the power, 2016-05-13 21:41:13]

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

[jaydasika, 2016-05-14 01:03:30]

Description was changed from

==========
cc : Add LayerToPropertyTreeIndices map to PropertyTrees

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6f47e3b18ac6b9084799d2d5978c6ef831fa5a07
Cr-Commit-Position: refs/heads/master@{#393646}
==========

to

==========
cc : Add LayerToPropertyTreeIndices map to PropertyTrees

This is dead code for now. The map is populated but not used.
This map will be used whenever we want to get property tree
node from a layer id without de-referencing LayerImpl. This
will ultimately become a map from stable id to property tree
index.

BUG=611936
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6f47e3b18ac6b9084799d2d5978c6ef831fa5a07
Cr-Commit-Position: refs/heads/master@{#393646}
==========