Add an explicit way of making IDs local or global

base/trace_event/trace_event_impl.cc

Index: base/trace_event/trace_event_impl.cc
diff --git a/base/trace_event/trace_event_impl.cc b/base/trace_event/trace_event_impl.cc
index 510cc2ffac6b32add118eb53ed4147b4b70369d9..eb24e918164ece034efc2949f9e9af07a1f04a23 100644
--- a/base/trace_event/trace_event_impl.cc
+++ b/base/trace_event/trace_event_impl.cc
@@ -358,10 +358,17 @@ void TraceEvent::AppendAsJSON(
 
   // If id_ is set, print it out as a hex string so we don't loose any
   // bits (it might be a 64-bit pointer).
-  if (flags_ & TRACE_EVENT_FLAG_HAS_ID) {
+  unsigned int id_flags = flags_ & TRACE_EVENT_FLAG_ID_MASK;
+  if (id_flags != TRACE_EVENT_FLAG_NONE) {
     if (scope_ != trace_event_internal::kGlobalScope)
       StringAppendF(out, ",\"scope\":\"%s\"", scope_);
-    StringAppendF(out, ",\"id\":\"0x%" PRIx64 "\"", static_cast<uint64_t>(id_));
+    std::string id_field_name = "id";
+    if (id_flags == TRACE_EVENT_FLAG_HAS_LOCAL_ID)
+      id_field_name = "local_id";

[Primiano Tucci (use gerrit), 2016/08/31 19:11:22]

instead of changing the field name, which feels quite magical, can we just make
it so that:

the old-style id becomes just id="..."
LOCAL_ID turns into id="local:..."
GLOBAL_ID turns into id="global:..."

so we can use the same pattern consistently for bind_id, without having to
create bind_global_id, bind_local_id etc? I guess that would make the logic in
the importer more reusable (I can imagine some helper LookupObjectById)

The TraceEventFormat[1] doc doesn't seem to mandate that id have to be 0x1234
hex parsable, and they are strings so ... :)

[1]
https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAy...

[chiniforooshan, 2016/08/31 21:28:45]

Done.

Note that this was originally due to Andrey's suggestion that if we use a
different field here for the new IDs, then old trace consumers will break
explicitly when they read an event with a new ID as opposed to reading the ID
field as usual but handling it incorrectly.

But, since we are going to change the two most important consumers (trace viewer
and devtools) soon, and probably before we actually use these macros, I'm in
favour of the cleaner choice :) (i.e. "id": "local:0x1234").

[chiniforooshan, 2016/09/02 19:37:37]

Nat, Ben, can one of you please tell us your preference about the JSON format
here. The options discussed so far are:

1- ("id":"0x1000", ...), ("id":"0x1000", "id_is_global":false, ...), and
("id":"0x1000", "id_is_global":true, ...)

vs

2- ("id":"0x1000", ...), ("local_id":"0x1000", ...), and ("global_id":"0x1000",
...)

vs

3- ("id":"0x1000", ...), ("id":"local:0x1000", ...), and ("id":"global:0x1000",
...)

Thanks a lot!

[chiniforooshan, 2016/09/09 14:40:37]

+petrcermak maybe?

+    if (id_flags == TRACE_EVENT_FLAG_HAS_GLOBAL_ID)
+      id_field_name = "global_id";
+    StringAppendF(out, ",\"%s\":\"0x%" PRIx64 "\"",
+                  id_field_name.c_str(), static_cast<uint64_t>(id_));
   }
 
   if (flags_ & TRACE_EVENT_FLAG_BIND_TO_ENCLOSING)