Add Layer Id information to the LayerTree json string

Add Layer Id information to the LayerTree json string

LayerTreeAsJson is useful during debugging of layer tree operations.
Adding the layer id into the information for the layer currently being
serialized. This helps the function be more usable as user knows which
layer he is looking when checking the json.

BUG=None

R=danakj,enne

Committed: https://crrev.com/fd17d0e9e5ca003cb323c30225681f5719fc64db
Cr-Commit-Position: refs/heads/master@{#330306}

[r.kasibhatla, 2015-05-13 12:05:34]

r.kasibhatla@samsung.com changed reviewers:
+ enne@chromium.org

[r.kasibhatla, 2015-05-13 12:06:15]

PTAL. A small change.

[danakj, 2015-05-13 16:50:05]

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

https://codereview.chromium.org/1140913002/diff/1/cc/layers/layer_impl.cc#new...
cc/layers/layer_impl.cc:659: result->SetString("Layer", base::StringPrintf("%p",
this));
What does the pointer do for you that the id doesn't?

[r.kasibhatla, 2015-05-14 03:51:56]

On 2015/05/13 at 16:50:05, danakj wrote:
> https://codereview.chromium.org/1140913002/diff/1/cc/layers/layer_impl.cc
> File cc/layers/layer_impl.cc (right):
> 
>
https://codereview.chromium.org/1140913002/diff/1/cc/layers/layer_impl.cc#new...
> cc/layers/layer_impl.cc:659: result->SetString("Layer",
base::StringPrintf("%p", this));
> What does the pointer do for you that the id doesn't?

I agree, generally id is enough to identify the layer. I added the pointer for
some corner cases like if we are trying to check some crashing or use-after-free
of layers type scenarios?
Though with id we can achieve the same, but with pointer we can catch very
quickly those type of scenarios.

[danakj, 2015-05-14 16:52:49]

On Wed, May 13, 2015 at 8:51 PM, <r.kasibhatla@samsung.com> wrote:

> On 2015/05/13 at 16:50:05, danakj wrote:
>
>> https://codereview.chromium.org/1140913002/diff/1/cc/layers/layer_impl.cc
>> File cc/layers/layer_impl.cc (right):
>>
>
>
>
>
https://codereview.chromium.org/1140913002/diff/1/cc/layers/layer_impl.cc#new...
>
>> cc/layers/layer_impl.cc:659: result->SetString("Layer",
>>
> base::StringPrintf("%p", this));
>
>> What does the pointer do for you that the id doesn't?
>>
>
> I agree, generally id is enough to identify the layer. I added the pointer
> for
> some corner cases like if we are trying to check some crashing or
> use-after-free
> of layers type scenarios?
> Though with id we can achieve the same, but with pointer we can catch very
> quickly those type of scenarios.
>

I think the id sounds like enough. The pointer is only useful if you're
running a debugger pretty much? And tracing/debugging don't really go
together. LGTM with just the id.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[r.kasibhatla, 2015-05-15 04:45:16]

On 2015/05/14 at 16:52:49, danakj wrote:
> On Wed, May 13, 2015 at 8:51 PM, <r.kasibhatla@samsung.com> wrote:
> 
> > On 2015/05/13 at 16:50:05, danakj wrote:
> >
> >> https://codereview.chromium.org/1140913002/diff/1/cc/layers/layer_impl.cc
> >> File cc/layers/layer_impl.cc (right):
> >>
> >
> >
> >
> >
https://codereview.chromium.org/1140913002/diff/1/cc/layers/layer_impl.cc#new...
> >
> >> cc/layers/layer_impl.cc:659: result->SetString("Layer",
> >>
> > base::StringPrintf("%p", this));
> >
> >> What does the pointer do for you that the id doesn't?
> >>
> >
> > I agree, generally id is enough to identify the layer. I added the pointer
> > for
> > some corner cases like if we are trying to check some crashing or
> > use-after-free
> > of layers type scenarios?
> > Though with id we can achieve the same, but with pointer we can catch very
> > quickly those type of scenarios.
> >
> 
> I think the id sounds like enough. The pointer is only useful if you're
> running a debugger pretty much? And tracing/debugging don't really go
> together. LGTM with just the id.
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email to chromium-reviews+unsubscribe@chromium.org.
> 

Sure. Uploaded the patch with only id added to the json information.

[r.kasibhatla, 2015-05-15 04:47:51]

@danakj: Removed the pointer info from json. Can you please stamp the change?
Thanks :)

[danakj, 2015-05-15 17:18:04]

LGTM

[r.kasibhatla, 2015-05-17 09:59:01]

The CQ bit was checked by r.kasibhatla@samsung.com

[commit-bot: I haz the power, 2015-05-17 09:59:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1140913002/20001

[commit-bot: I haz the power, 2015-05-17 11:12:15]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-18 11:32:41]

Patchset 2 (id:??) landed as
https://crrev.com/fd17d0e9e5ca003cb323c30225681f5719fc64db
Cr-Commit-Position: refs/heads/master@{#330306}