[cc] Store CCLayerImpls as scoped_ptrs

[cc] Store CCLayerImpls as scoped_ptrs

BUG=154451
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=160631

[jamesr, 2012-10-07 22:27:19]

lgtm

https://codereview.chromium.org/11076013/diff/1/cc/CCLayerTreeHostImplTest.cpp
File cc/CCLayerTreeHostImplTest.cpp (right):

https://codereview.chromium.org/11076013/diff/1/cc/CCLayerTreeHostImplTest.cp...
cc/CCLayerTreeHostImplTest.cpp:681: static scoped_ptr<CCLayerImpl> create(int
id) { return make_scoped_ptr(new DidDrawCheckLayer(id)).PassAs<CCLayerImpl>(); }
just "return scoped_ptr<T>(...)" as below would be terser than the
make_...().PassAs<T>() version

https://codereview.chromium.org/11076013/diff/1/cc/TreeSynchronizer.cpp
File cc/TreeSynchronizer.cpp (right):

https://codereview.chromium.org/11076013/diff/1/cc/TreeSynchronizer.cpp#newco...
cc/TreeSynchronizer.cpp:99: CCScrollbarLayerImpl* ccScrollbarLayerImpl = iter !=
newLayers.end() ? static_cast<CCScrollbarLayerImpl*>(iter->second) : NULL;
do we really need the iterator? what about just doing
CCScrollbarLayerImpl* ccScrollbarLayerImpl = newLayers[scrollbarLayer->id()];
?

if the entry isn't in the map this will default-construct a null
CCScrollbarLayerImpl*, but we ASSERT() that we don't hit that case anyway

https://codereview.chromium.org/11076013/diff/1/cc/TreeSynchronizer.cpp#newco...
cc/TreeSynchronizer.cpp:101: CCLayerImpl* ccScrollLayerImpl = iter !=
newLayers.end() ? iter->second : NULL;
same here

[enne (OOO), 2012-10-07 22:44:57]

https://codereview.chromium.org/11076013/diff/1/cc/CCLayerTreeHostImplTest.cpp
File cc/CCLayerTreeHostImplTest.cpp (right):

https://codereview.chromium.org/11076013/diff/1/cc/CCLayerTreeHostImplTest.cp...
cc/CCLayerTreeHostImplTest.cpp:681: static scoped_ptr<CCLayerImpl> create(int
id) { return make_scoped_ptr(new DidDrawCheckLayer(id)).PassAs<CCLayerImpl>(); }
On 2012/10/07 22:27:19, jamesr wrote:
> just "return scoped_ptr<T>(...)" as below would be terser than the
> make_...().PassAs<T>() version

Done.

https://codereview.chromium.org/11076013/diff/1/cc/TreeSynchronizer.cpp
File cc/TreeSynchronizer.cpp (right):

https://codereview.chromium.org/11076013/diff/1/cc/TreeSynchronizer.cpp#newco...
cc/TreeSynchronizer.cpp:99: CCScrollbarLayerImpl* ccScrollbarLayerImpl = iter !=
newLayers.end() ? static_cast<CCScrollbarLayerImpl*>(iter->second) : NULL;
On 2012/10/07 22:27:19, jamesr wrote:
> do we really need the iterator? what about just doing
> CCScrollbarLayerImpl* ccScrollbarLayerImpl = newLayers[scrollbarLayer->id()];
> ?
> 
> if the entry isn't in the map this will default-construct a null
> CCScrollbarLayerImpl*, but we ASSERT() that we don't hit that case anyway

Done.  I think it's just personal style to never use [] unless I'm assigning
because of that behavior.

https://codereview.chromium.org/11076013/diff/1/cc/TreeSynchronizer.cpp#newco...
cc/TreeSynchronizer.cpp:101: CCLayerImpl* ccScrollLayerImpl = iter !=
newLayers.end() ? iter->second : NULL;
On 2012/10/07 22:27:19, jamesr wrote:
> same here

Done.

[enne (OOO), 2012-10-07 23:40:23]

https://codereview.chromium.org/11076013/diff/1/cc/TreeSynchronizer.cpp
File cc/TreeSynchronizer.cpp (right):

https://codereview.chromium.org/11076013/diff/1/cc/TreeSynchronizer.cpp#newco...
cc/TreeSynchronizer.cpp:99: CCScrollbarLayerImpl* ccScrollbarLayerImpl = iter !=
newLayers.end() ? static_cast<CCScrollbarLayerImpl*>(iter->second) : NULL;
On 2012/10/07 22:44:57, enne wrote:
> On 2012/10/07 22:27:19, jamesr wrote:
> > do we really need the iterator? what about just doing
> > CCScrollbarLayerImpl* ccScrollbarLayerImpl =
newLayers[scrollbarLayer->id()];
> > ?
> > 
> > if the entry isn't in the map this will default-construct a null
> > CCScrollbarLayerImpl*, but we ASSERT() that we don't hit that case anyway
> 
> Done.  I think it's just personal style to never use [] unless I'm assigning
> because of that behavior.

Ok, I lied.  You can't operator[] on a const hash_map.  Reverted back to
original.

[commit-bot: I haz the power, 2012-10-08 04:09:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/11076013/6001

[commit-bot: I haz the power, 2012-10-08 05:58:44]

Change committed as 160631