cc: Add id->LayerImpl map to LayerTreeImpl

cc: Add id->LayerImpl map to LayerTreeImpl

This is a prerequisite to two LayerTreeImpls, as PictureLayerImpls in the
active tree may need to ask about their pending tree sibilings with the
same id.  Once this lands, functionality like findScrollingLayerbyId can
be wrapped into this too.

LayerImpl now registers itself with its tree on creation and destruction.
As there are now asserts that a given id is not already in use, a number
of tests have been updated to not duplicate layer ids.

There doesn't technically need to be an unregister step, but as LayerImpl
doesn't necessarily have to be in the LayerImpl tree rooted in the
LayerTreeImpl, this unregister check will blow up if anybody has a layer
outliving their registered tree (which is bad).

As LayerImpl has single ownership, it's impossible to assign the same
non-NULL replica or mask to a layer, so the LayerImpl unit tests are
updated to not test this anymore.  Masks and replicas are also modified
to keep their id in sync with their pointer during tree synchronization
to prevent asserts about duplicate ids.

R=danakj@chromium.org
BUG=155209
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173110

[danakj, 2012-12-13 23:57:11]

LG mostly.

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

https://codereview.chromium.org/11575018/diff/1/cc/layer_impl.cc#newcode504
cc/layer_impl.cc:504: } else if (newLayerId == m_maskLayerId)
This is "if (!maskLayer && newLayerId == m_maskLayerId)"

I don't think you meant else if?

https://codereview.chromium.org/11575018/diff/1/cc/layer_impl.cc#newcode505
cc/layer_impl.cc:505: return;
If the ids match, we're going to free the maskLayer passed in here? This seems
super weird. We used to always assign the pointer before.

I suspect this is never called with a non-NULL pointer where the layerId ==
maskLayerId. It seems impossible without breaking a pointer out of scoped_ptr.
So this check seems weird to me.

https://codereview.chromium.org/11575018/diff/1/cc/layer_tree_impl.h
File cc/layer_tree_impl.h (right):

https://codereview.chromium.org/11575018/diff/1/cc/layer_tree_impl.h#newcode11
cc/layer_tree_impl.h:11: #if defined(COMPILER_GCC)
I would expect you to need an MSVC version too. If you do, see cc/hash_pair.h

[enne (OOO), 2012-12-14 00:21:12]

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

https://codereview.chromium.org/11575018/diff/1/cc/layer_impl.cc#newcode504
cc/layer_impl.cc:504: } else if (newLayerId == m_maskLayerId)
On 2012/12/13 23:57:11, danakj wrote:
> This is "if (!maskLayer && newLayerId == m_maskLayerId)"
> 
> I don't think you meant else if?

No, I meant else if.  This is the case where "if you don't have a mask layer and
you're assigning a null mask layer, then return and don't call
noteLayerPropertyBlahBlah".

If I didn't do the else if, then you could drop maskLayer on the floor, like
you're worried about below.

https://codereview.chromium.org/11575018/diff/1/cc/layer_tree_impl.h
File cc/layer_tree_impl.h (right):

https://codereview.chromium.org/11575018/diff/1/cc/layer_tree_impl.h#newcode11
cc/layer_tree_impl.h:11: #if defined(COMPILER_GCC)
On 2012/12/13 23:57:11, danakj wrote:
> I would expect you to need an MSVC version too. If you do, see cc/hash_pair.h

Nope.  That's just for std::pair being weird.  See layer_sorter.h or
layer_tree_host.h

[danakj, 2012-12-14 00:23:15]

LGTM!

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

https://codereview.chromium.org/11575018/diff/1/cc/layer_impl.cc#newcode504
cc/layer_impl.cc:504: } else if (newLayerId == m_maskLayerId)
On 2012/12/14 00:21:12, enne wrote:
> On 2012/12/13 23:57:11, danakj wrote:
> > This is "if (!maskLayer && newLayerId == m_maskLayerId)"
> > 
> > I don't think you meant else if?
> 
> No, I meant else if.  This is the case where "if you don't have a mask layer
and
> you're assigning a null mask layer, then return and don't call
> noteLayerPropertyBlahBlah".
> 
> If I didn't do the else if, then you could drop maskLayer on the floor, like
> you're worried about below.

OOhh. ok!

[commit-bot: I haz the power, 2012-12-14 00:44:38]

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

[commit-bot: I haz the power, 2012-12-14 07:03:26]

Change committed as 173110