[cc] Fix crash when adding a child layer to its own parent.

[cc] Fix crash when adding a child layer to its own parent.

Only calculate the index after calling removeFromParent in case
it modifies the number of children.

BUG=


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

[jamesr, 2012-11-12 19:29:08]

Can you add a test? See cc/layer_unittest.cc

[Ted C, 2012-11-12 21:46:14]

Added a unittest and verified that it crashed w/o my change and passes with.

[danakj, 2012-11-12 21:47:23]

https://chromiumcodereview.appspot.com/11377108/diff/5001/cc/layer_unittest.cc
File cc/layer_unittest.cc (right):

https://chromiumcodereview.appspot.com/11377108/diff/5001/cc/layer_unittest.c...
cc/layer_unittest.cc:172: parent->setLayerTreeHost(m_layerTreeHost.get());
Do m_layerTreeHost->setRootLayer(parent) instead

[Ted C, 2012-11-12 22:15:45]

https://chromiumcodereview.appspot.com/11377108/diff/5001/cc/layer_unittest.cc
File cc/layer_unittest.cc (right):

https://chromiumcodereview.appspot.com/11377108/diff/5001/cc/layer_unittest.c...
cc/layer_unittest.cc:172: parent->setLayerTreeHost(m_layerTreeHost.get());
On 2012/11/12 21:47:23, danakj wrote:
> Do m_layerTreeHost->setRootLayer(parent) instead

Done.

[jamesr, 2012-11-12 22:22:09]

lgtm

[tfarina, 2012-11-12 22:22:47]

https://chromiumcodereview.appspot.com/11377108/diff/9001/cc/layer_unittest.cc
File cc/layer_unittest.cc (right):

https://chromiumcodereview.appspot.com/11377108/diff/9001/cc/layer_unittest.c...
cc/layer_unittest.cc:174: ASSERT_EQ(static_cast<size_t>(0),
parent->children().size());
0u

https://chromiumcodereview.appspot.com/11377108/diff/9001/cc/layer_unittest.c...
cc/layer_unittest.cc:177: ASSERT_EQ(static_cast<size_t>(1),
parent->children().size());
1u

https://chromiumcodereview.appspot.com/11377108/diff/9001/cc/layer_unittest.c...
cc/layer_unittest.cc:181: ASSERT_EQ(static_cast<size_t>(1),
parent->children().size());
1u

[Ted C, 2012-11-12 22:31:12]

https://chromiumcodereview.appspot.com/11377108/diff/9001/cc/layer_unittest.cc
File cc/layer_unittest.cc (right):

https://chromiumcodereview.appspot.com/11377108/diff/9001/cc/layer_unittest.c...
cc/layer_unittest.cc:174: ASSERT_EQ(static_cast<size_t>(0),
parent->children().size());
On 2012/11/12 22:22:47, tfarina wrote:
> 0u

Done.

https://chromiumcodereview.appspot.com/11377108/diff/9001/cc/layer_unittest.c...
cc/layer_unittest.cc:177: ASSERT_EQ(static_cast<size_t>(1),
parent->children().size());
On 2012/11/12 22:22:47, tfarina wrote:
> 1u

Done.

https://chromiumcodereview.appspot.com/11377108/diff/9001/cc/layer_unittest.c...
cc/layer_unittest.cc:181: ASSERT_EQ(static_cast<size_t>(1),
parent->children().size());
On 2012/11/12 22:22:47, tfarina wrote:
> 1u

Done.

[danakj, 2012-11-12 22:31:48]

https://chromiumcodereview.appspot.com/11377108/diff/4002/cc/layer_unittest.cc
File cc/layer_unittest.cc (right):

https://chromiumcodereview.appspot.com/11377108/diff/4002/cc/layer_unittest.c...
cc/layer_unittest.cc:174: ASSERT_EQ(static_cast<size_t>(0u),
parent->children().size());
You shouldn't need the static_casts then :)

[tfarina, 2012-11-12 22:36:27]

https://chromiumcodereview.appspot.com/11377108/diff/4002/cc/layer_unittest.cc
File cc/layer_unittest.cc (right):

https://chromiumcodereview.appspot.com/11377108/diff/4002/cc/layer_unittest.c...
cc/layer_unittest.cc:174: ASSERT_EQ(static_cast<size_t>(0u),
parent->children().size());
On 2012/11/12 22:31:48, danakj wrote:
> You shouldn't need the static_casts then :)
yep, that is what I suggested:
s/static_cast<size_t>(0)/0u :)

[Ted C, 2012-11-12 22:59:03]

https://chromiumcodereview.appspot.com/11377108/diff/4002/cc/layer_unittest.cc
File cc/layer_unittest.cc (right):

https://chromiumcodereview.appspot.com/11377108/diff/4002/cc/layer_unittest.c...
cc/layer_unittest.cc:174: ASSERT_EQ(static_cast<size_t>(0u),
parent->children().size());
On 2012/11/12 22:36:27, tfarina wrote:
> On 2012/11/12 22:31:48, danakj wrote:
> > You shouldn't need the static_casts then :)
> yep, that is what I suggested:
> s/static_cast<size_t>(0)/0u :)

Done :-)  That's what I get for trying to get a change done right before a
meeting.

[danakj, 2012-11-12 23:00:47]

lgtm2

[commit-bot: I haz the power, 2012-11-12 23:09:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedchoc@chromium.org/11377108/8002

[commit-bot: I haz the power, 2012-11-13 06:37:19]

Change committed as 167315