cc: Fix crash in MVRAR with missing recordings

cc: Fix crash in MVRAR with missing recordings

Add a missing null check for the case where missing recordings create
null tiles.

R=reveman@chromium.org
BUG=243839
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202960

[danakj, 2013-05-29 00:11:47]

The code changes lgtm and some random questions about the test you can ignore if
needed

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
File cc/layers/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
cc/layers/picture_layer_impl_unittest.cc:817: pending_pile->AddRecordingAt(0,
0);
From holiday: do you need to add recording? Or could you just not do anything
here?

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
cc/layers/picture_layer_impl_unittest.cc:825: // It should be safe to call this
(and MarkVisibleResourcesAsRequired)
Can you verify that the tilings were not added? Num_tilings is 0 or something?

[enne (OOO), 2013-05-29 00:14:29]

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
File cc/layers/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
cc/layers/picture_layer_impl_unittest.cc:817: pending_pile->AddRecordingAt(0,
0);
On 2013/05/29 00:11:47, danakj wrote:
> From holiday: do you need to add recording? Or could you just not do anything
> here?

Yes.  CanHaveTilings() is false if the pile's recorded region is empty.

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
cc/layers/picture_layer_impl_unittest.cc:825: // It should be safe to call this
(and MarkVisibleResourcesAsRequired)
On 2013/05/29 00:11:47, danakj wrote:
> Can you verify that the tilings were not added? Num_tilings is 0 or something?

The tilings are supposed to be there, it's that the tiles aren't.  Are you
asking that I verify that iterating over each tiling has both null and non-null
tiles?

[danakj, 2013-05-29 10:39:33]

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
File cc/layers/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
cc/layers/picture_layer_impl_unittest.cc:817: pending_pile->AddRecordingAt(0,
0);
On 2013/05/29 00:14:29, enne wrote:
> On 2013/05/29 00:11:47, danakj wrote:
> > From holiday: do you need to add recording? Or could you just not do
anything
> > here?
>  
> Yes.  CanHaveTilings() is false if the pile's recorded region is empty.

I see. I guess its just nonobvious that there are in fact null tiles being hit
from reading the test alone. Or that UpdateDrawProps didn't just early out or
something.

https://codereview.chromium.org/16086003/diff/1/cc/layers/picture_layer_impl_...
cc/layers/picture_layer_impl_unittest.cc:825: // It should be safe to call this
(and MarkVisibleResourcesAsRequired)
On 2013/05/29 00:14:29, enne wrote:
> On 2013/05/29 00:11:47, danakj wrote:
> > Can you verify that the tilings were not added? Num_tilings is 0 or
something?
> 
> The tilings are supposed to be there, it's that the tiles aren't.  Are you
> asking that I verify that iterating over each tiling has both null and
non-null
> tiles?

Oh sorry, right.

[commit-bot: I haz the power, 2013-05-29 16:14:00]

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

[commit-bot: I haz the power, 2013-05-29 20:55:17]

Change committed as 202960