cc: Remove some unnecessary const scoped_refptr&.

cc: Remove some unnecessary const scoped_refptr&.

This patch makes the API a bit clearer by taking a copy of scoped_refptr
in some situations (where ownership is shared) and using a raw pointer
in situations where ownership is not needed. The only references that
remain are the ones captured in lambdas or Callbacks.

R=danakj
BUG=589044
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/4065174ac28746b10a832411426bd756d9c5979c
Cr-Commit-Position: refs/heads/master@{#381524}

[vmpstr, 2016-03-14 22:14:23]

Please take a look.

[vmpstr, 2016-03-15 20:54:22]

ping

[danakj, 2016-03-16 00:08:15]

LG except

https://codereview.chromium.org/1803863003/diff/1/cc/layers/layer_proto_conve...
File cc/layers/layer_proto_converter.cc (left):

https://codereview.chromium.org/1803863003/diff/1/cc/layers/layer_proto_conve...
cc/layers/layer_proto_converter.cc:97: [layer_id_map](Layer* layer) {
(*layer_id_map)[layer->id()] = layer; });
https://code.google.com/p/chromium/codesearch#chromium/src/cc/layers/layer_pr...

LayerIdMap is a map of refptrs. Does it need to hold ownership?

https://codereview.chromium.org/1803863003/diff/1/cc/layers/layer_proto_conve...
File cc/layers/layer_proto_converter.h (right):

https://codereview.chromium.org/1803863003/diff/1/cc/layers/layer_proto_conve...
cc/layers/layer_proto_converter.h:69: using LayerIdMap = std::unordered_map<int,
scoped_refptr<Layer>>;
Does this need to be refptrs? If so the FindAllLayers may as well take a refptr,
cuz it's going to turn the raw pointer back into one.

[vmpstr, 2016-03-16 16:38:25]

Description was changed from

==========
cc: Remove some unnecessary const scoped_refptr&.

This patch makes the API a bit clearer by taking a copy of scoped_refptr
in some situations (where ownership is shared) and using a raw pointer
in situations where ownership is not needed. The only references that
remain are the ones captured in lambdas or Callbacks.

R=danakj
BUG=589044
==========

to

==========
cc: Remove some unnecessary const scoped_refptr&.

This patch makes the API a bit clearer by taking a copy of scoped_refptr
in some situations (where ownership is shared) and using a raw pointer
in situations where ownership is not needed. The only references that
remain are the ones captured in lambdas or Callbacks.

R=danakj
BUG=589044
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[vmpstr, 2016-03-16 16:38:37]

PTAL

https://codereview.chromium.org/1803863003/diff/1/cc/layers/layer_proto_conve...
File cc/layers/layer_proto_converter.h (right):

https://codereview.chromium.org/1803863003/diff/1/cc/layers/layer_proto_conve...
cc/layers/layer_proto_converter.h:69: using LayerIdMap = std::unordered_map<int,
scoped_refptr<Layer>>;
On 2016/03/16 00:08:15, danakj wrote:
> Does this need to be refptrs? If so the FindAllLayers may as well take a
refptr,
> cuz it's going to turn the raw pointer back into one.

I think it makes sense for the map to be owning, since that's where the layer
children / masks are taken from when they exist. Those things are ref counted.
In other words, even if this map has raw pointers we'll still be creating ref
counted objects out of them and I would prefer to then have ref counting as
early as possible. 

The function itself though begins iteration with a raw pointer, hence this
change. I've added a TODO here to fix it better.

[danakj, 2016-03-16 18:04:32]

https://codereview.chromium.org/1803863003/diff/1/cc/layers/layer_proto_conve...
File cc/layers/layer_proto_converter.h (right):

https://codereview.chromium.org/1803863003/diff/1/cc/layers/layer_proto_conve...
cc/layers/layer_proto_converter.h:69: using LayerIdMap = std::unordered_map<int,
scoped_refptr<Layer>>;
On 2016/03/16 16:38:37, vmpstr wrote:
> On 2016/03/16 00:08:15, danakj wrote:
> > Does this need to be refptrs? If so the FindAllLayers may as well take a
> refptr,
> > cuz it's going to turn the raw pointer back into one.
> 
> I think it makes sense for the map to be owning, since that's where the layer
> children / masks are taken from when they exist. Those things are ref counted.
> In other words, even if this map has raw pointers we'll still be creating ref
> counted objects out of them and I would prefer to then have ref counting as
> early as possible. 
> 
> The function itself though begins iteration with a raw pointer, hence this
> change. I've added a TODO here to fix it better.

OK.

BTW if blink no longer changes cc during painting, we don't need to refcount
cc::Layer anymore. I thot we turned on strict property checking to verify blink
isn't doing this, maybe that bounced, enne would know where that's at.

[danakj, 2016-03-16 18:04:38]

LGTM

[vmpstr, 2016-03-16 18:05:16]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2016-03-16 18:05:55]

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

[commit-bot: I haz the power, 2016-03-16 20:44:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-16 20:46:25]

Description was changed from

==========
cc: Remove some unnecessary const scoped_refptr&.

This patch makes the API a bit clearer by taking a copy of scoped_refptr
in some situations (where ownership is shared) and using a raw pointer
in situations where ownership is not needed. The only references that
remain are the ones captured in lambdas or Callbacks.

R=danakj
BUG=589044
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Remove some unnecessary const scoped_refptr&.

This patch makes the API a bit clearer by taking a copy of scoped_refptr
in some situations (where ownership is shared) and using a raw pointer
in situations where ownership is not needed. The only references that
remain are the ones captured in lambdas or Callbacks.

R=danakj
BUG=589044
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/4065174ac28746b10a832411426bd756d9c5979c
Cr-Commit-Position: refs/heads/master@{#381524}
==========

[commit-bot: I haz the power, 2016-03-16 20:46:26]

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