[tracing] Don't use dot-based path expansion in TracedValue

[tracing] Don't use dot-based path expansion in TracedValue

The current implementation of TracedValue still relies on a conversion
to base::Value in the last stage to create the JSON.
The current implementation can degenerate the dictionary if key names
contain dots, thanks to the default implementation of base::Value.
This fixes the issue by using the *WithoutPathExpansion methods.

BUG=532838
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/986ddd21b0329b365b5cf95e71b0d46a797ec00a
Cr-Commit-Position: refs/heads/master@{#349640}

[Primiano Tucci (use gerrit), 2015-09-17 10:54:19]

primiano@chromium.org changed reviewers:
+ dsinclair@chromium.org, nduca@chromium.org

[Primiano Tucci (use gerrit), 2015-09-17 10:54:19]

dsinclair, nduca: the patch itself it's trivial.
The main question is: can I assume that nothing in tracing is relying on the
auto dot-path-expansion behavior of trace args?
sounds to me insane if we did, but worth doublechecking

[Primiano Tucci (use gerrit), 2015-09-17 11:13:41]

On 2015/09/17 10:54:19, Primiano Tucci wrote:
> dsinclair, nduca: the patch itself it's trivial.
> The main question is: can I assume that nothing in tracing is relying on the
> auto dot-path-expansion behavior of trace args?
> sounds to me insane if we did, but worth doublechecking

Hmm self NOT LGTM. Looks like something in cc.debug.display_items is depending
on this. After this CL I get:

[4779:4779:0917/121101:ERROR:CONSOLE(664)] "TypeError: Cannot read property
'layerRect' of undefined
    at Object.PictureSnapshot.initialize (chrome://tracing/tracing.js:3682:52)
    at Object.DisplayItemListSnapshot.initialize
(chrome://tracing/tracing.js:3685:147)
    at ObjectInstance.initialize (chrome://tracing/tracing.js:619:19)
    at Object.<anonymous> (chrome://tracing/tracing.js:1450:695)
    at Array.forEach (native)
    at Object.<anonymous> (chrome://tracing/tracing.js:1450:197)
    at Object.iterItems (chrome://tracing/tracing.js:340:139)
    at ObjectCollection.iterObjectInstances
(chrome://tracing/tracing.js:1450:127)
    at ObjectCollection.initializeAllObjects
(chrome://tracing/tracing.js:1450:647)
    at Object.ProcessBase.initializeObjects
(chrome://tracing/tracing.js:1561:502)", source: chrome://tracing/tracing.js
(664)

[Primiano Tucci (use gerrit), 2015-09-17 14:19:53]

primiano@chromium.org changed reviewers:
+ caseq@chromium.org, enne@chromium.org

[Primiano Tucci (use gerrit), 2015-09-17 14:19:54]

Ok the fix for  cc.debug.display_items  seems trivial.

Adding +enne/caseq for cc/ changes.

I have only one major doubt, I see another "params.layer_rect" in the codebase
in playback/picture.cc
"scoped_refptr<Picture> Picture::CreateFromValue"

Is that related with tracing as well? Or does it happen to use the same name for
the serialization/playback and has nothing to do with tracing?

[dsinclair, 2015-09-17 14:28:18]

base/trace_event lgtm.

It looks like the picture call is used in tests and the
skia_benchmarking_extension.

cc/playback/picture.cc:scoped_refptr<Picture> Picture::CreateFromValue(const
base::Value* raw_value) {
cc/playback/picture_unittest.cc:      Picture::CreateFromValue(tmp.get());
cc/playback/picture_unittest.cc:     
Picture::CreateFromValue(serialized_one_rect.get());
cc/playback/picture_unittest.cc:     
Picture::CreateFromValue(serialized_two_rect.get());
content/renderer/skia_benchmarking_extension.cc:  return
cc::Picture::CreateFromValue(picture_value.get());


Th tests seem fine, I have no idea what the extension is doing .....

[caseq, 2015-09-17 16:39:43]

On 2015/09/17 14:19:54, Primiano Tucci wrote:
> Ok the fix for  cc.debug.display_items  seems trivial.
> 
> Adding +enne/caseq for cc/ changes.
> 
> I have only one major doubt, I see another "params.layer_rect" in the codebase
> in playback/picture.cc
> "scoped_refptr<Picture> Picture::CreateFromValue"
> 
> Is that related with tracing as well? Or does it happen to use the same name
for

It is, and we actually use the value at least in the DevTools' frame viewer
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
and possibly in the trace viewer as well, so please retain the trace format.

[Primiano Tucci (use gerrit), 2015-09-17 16:48:10]

On 2015/09/17 16:39:43, caseq wrote:
> On 2015/09/17 14:19:54, Primiano Tucci wrote:
> > Ok the fix for  cc.debug.display_items  seems trivial.
> > 
> > Adding +enne/caseq for cc/ changes.
> > 
> > I have only one major doubt, I see another "params.layer_rect" in the
codebase
> > in playback/picture.cc
> > "scoped_refptr<Picture> Picture::CreateFromValue"
> > 
> > Is that related with tracing as well? Or does it happen to use the same name
> for
> 
> It is, and we actually use the value at least in the DevTools' frame viewer
>
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
> and possibly in the trace viewer as well, so please retain the trace format.

I am retaining the trace format (I hope) but I'd like the tracing code to NOT
rely on the fact "params.items" magically expands into params { items [] } and
instead make it explicit (see the change introduced here)

I checked the frameviewer in tracing and seems happy with this change.
How can I test that I am not breaking anything else?

[enne (OOO), 2015-09-17 16:59:42]

cc lgtm

[Primiano Tucci (use gerrit), 2015-09-18 08:33:12]

On 2015/09/17 16:59:42, enne wrote:
> cc lgtm

Looks like the DevTools frameviewer is happy with this change. After all this is
not altering the data format, just making the dictionary creation explicit

[Primiano Tucci (use gerrit), 2015-09-18 08:33:20]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2015-09-18 08:33:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1354493003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1354493003/40001

[commit-bot: I haz the power, 2015-09-18 09:57:47]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-09-18 09:58:27]

Patchset 3 (id:??) landed as
https://crrev.com/986ddd21b0329b365b5cf95e71b0d46a797ec00a
Cr-Commit-Position: refs/heads/master@{#349640}