cc: add DCHECKs of tile in GPUMemoryUsageInBytes and PushPropertiesTo

cc: add DCHECKs of tile in GPUMemoryUsageInBytes and PushPropertiesTo

BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282090

[hyunki, 2014-06-30 09:41:38]

Please take a look.

[reveman, 2014-06-30 12:51:31]

can you add a test for this?

[hyunki, 2014-06-30 13:07:43]

Sure. I'll make it.

[hyunki, 2014-07-09 14:48:58]

On 2014/06/30 12:51:31, reveman wrote:
> can you add a test for this?

Sorry for late reply and asking again.
I couldn't find where I add the test for this null check.
This is null check for the null tile to avoid subsequent function calls to
access tile(DrawableTile).
Also, I found same null check in TiledLayerImpl::GPUMemoryUsageInBytes()
fuction.

Do I need to add unittest of the function "TiledLayerImpl::PushPropertiesTo"
which includes this null check?

[reveman, 2014-07-09 15:04:11]

On 2014/07/09 14:48:58, hyunki wrote:
> On 2014/06/30 12:51:31, reveman wrote:
> > can you add a test for this?
> 
> Sorry for late reply and asking again.
> I couldn't find where I add the test for this null check.
> This is null check for the null tile to avoid subsequent function calls to
> access tile(DrawableTile).
> Also, I found same null check in TiledLayerImpl::GPUMemoryUsageInBytes()
> fuction.
> 
> Do I need to add unittest of the function "TiledLayerImpl::PushPropertiesTo"
> which includes this null check?

It makes sense to make the code in ::GPUMemoryUsageInBytes() and
::PushPropertiesTo() consistent. Can we just change this to a DCHECK(tile) in
both cases instead? If not, can you explain what might cause |tile| to be NULL?

Also, I'm not sure the the "!tile->resource_id()" check should be in
::PushPropertiesTo(). It makes sense in ::GPUMemoryUsageInBytes() but not clear
that tiles without resources should be ignored in ::PushPropertiesTo().

[hyunki, 2014-07-09 15:12:01]

Thanks, PTAL.

> It makes sense to make the code in ::GPUMemoryUsageInBytes() and
> ::PushPropertiesTo() consistent. Can we just change this to a DCHECK(tile) in
> both cases instead? If not, can you explain what might cause |tile| to be
NULL?
Done.

> Also, I'm not sure the the "!tile->resource_id()" check should be in
> ::PushPropertiesTo(). It makes sense in ::GPUMemoryUsageInBytes() but not
clear
> that tiles without resources should be ignored in ::PushPropertiesTo().
Done.

[reveman, 2014-07-09 15:13:45]

please update the description to match the new version of this patch.

[hyunki, 2014-07-09 15:18:01]

On 2014/07/09 15:13:45, reveman wrote:
> please update the description to match the new version of this patch.

Done. Thanks.

[hyunki, 2014-07-09 15:50:11]

On 2014/07/09 15:18:01, hyunki wrote:
> On 2014/07/09 15:13:45, reveman wrote:
> > please update the description to match the new version of this patch.
> 
> Done. Thanks.

Sorry. I have changed the description again. PTAL. Thanks.

[reveman, 2014-07-09 15:52:23]

lgtm

[hyunki, 2014-07-09 15:54:33]

The CQ bit was checked by hyunki.baik@samsung.com

[commit-bot: I haz the power, 2014-07-09 15:57:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hyunki.baik@samsung.com/353093004/60001

[enne (OOO), 2014-07-09 17:01:18]

The CQ bit was unchecked by enne@chromium.org

[enne (OOO), 2014-07-09 17:07:08]

The CQ bit was checked by enne@chromium.org

[commit-bot: I haz the power, 2014-07-09 17:08:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hyunki.baik@samsung.com/353093004/60001

[commit-bot: I haz the power, 2014-07-09 18:42:45]

Change committed as 282090