The TRACE_LINK_IDS macro

base/trace_event/trace_event.h

Index: base/trace_event/trace_event.h
diff --git a/base/trace_event/trace_event.h b/base/trace_event/trace_event.h
index 4de64837161fded5fd176d1dc5136006d8521a9e..47d1924dccbdeee0010d4a4e8394d889d787cf5e 100644
--- a/base/trace_event/trace_event.h
+++ b/base/trace_event/trace_event.h
@@ -15,10 +15,13 @@
 #include <string>
 
 #include "base/atomicops.h"
+#include "base/format_macros.h"

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

remember to remove the unnecessary .h once you move the method out-of-line

[chiniforooshan, 2016/10/03 18:12:19]

Done.

 #include "base/macros.h"
+#include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "base/trace_event/common/trace_event_common.h"
 #include "base/trace_event/heap_profiler.h"
+#include "base/trace_event/trace_event_argument.h"

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

Yeah please don't pull extra includes in this file or will very likely create a
mess in blink.

[chiniforooshan, 2016/10/03 18:12:19]

Done.

 #include "base/trace_event/trace_event_system_stats_monitor.h"
 #include "base/trace_event/trace_log.h"
 #include "build/build_config.h"
@@ -376,29 +379,20 @@ TRACE_EVENT_API_CLASS_EXPORT extern \
     }                                                                       \
   } while (0)
 
-// This macro ignores whether the bind_id is local, global, or mangled.
-#define INTERNAL_TRACE_EVENT_ADD_BIND_IDS(category_group, name, id, bind_id,  \
-                                          ...)                                \
+// The linked ID will not be mangled.
+#define INTERNAL_TRACE_EVENT_ADD_LINK_IDS(category_group, name, id1, id2)     \
     do {                                                                      \
       INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group);                 \
       if (INTERNAL_TRACE_EVENT_CATEGORY_GROUP_ENABLED_FOR_RECORDING_MODE()) { \
-        trace_event_internal::TraceID source_id((id));                        \
+        trace_event_internal::TraceID source_id((id1));                       \
         unsigned int source_flags = source_id.id_flags();                     \
-        trace_event_internal::TraceID target_id((bind_id));                   \
-        if (target_id.scope() == trace_event_internal::kGlobalScope) {        \
-          trace_event_internal::AddTraceEvent(                                \
-              TRACE_EVENT_PHASE_BIND_IDS,                                     \
-              INTERNAL_TRACE_EVENT_UID(category_group_enabled),               \
-              name, source_id.scope(), source_id.raw_id(),                    \
-              source_flags, target_id.raw_id(), ##__VA_ARGS__);               \
-        } else {                                                              \
-          trace_event_internal::AddTraceEvent(                                \
-              TRACE_EVENT_PHASE_BIND_IDS,                                     \
-              INTERNAL_TRACE_EVENT_UID(category_group_enabled),               \
-              name, source_id.scope(), source_id.raw_id(),                    \
-              source_flags, target_id.raw_id(),                               \
-              "bind_scope", target_id.scope(), ##__VA_ARGS__);                \
-        }                                                                     \
+        trace_event_internal::TraceID target_id((id2));                       \
+        trace_event_internal::AddTraceEvent(                                  \
+            TRACE_EVENT_PHASE_LINK_IDS,                                       \
+            INTERNAL_TRACE_EVENT_UID(category_group_enabled),                 \
+            name, source_id.scope(), source_id.raw_id(), source_flags,        \
+            trace_event_internal::kNoId,                                      \
+            "linked_id", target_id.dict_value());                             \

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

don't you need a std::move here?

[chiniforooshan, 2016/10/03 18:12:19]

It's a return value. If I actually do std::move here I get a warning: "moving a
temporary object prevents copy elision".

This is similar to many other cases already in the code. e.g. we don't std::move
here:

https://cs.chromium.org/chromium/src/cc/debug/benchmark_instrumentation.cc?l=18.

       }                                                                       \
     } while (0)
 
@@ -582,6 +576,39 @@ class TraceID {
   const char* scope() const { return scope_; }
   unsigned int id_flags() const { return id_flags_; }
 
+  std::unique_ptr<base::trace_event::TracedValue> dict_value() const {

[caseq, 2016/10/01 01:33:53]

Let's move it out-of-line, it's too big.

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

+1 this is definitely beyon the limits for an inline function (see
https://google.github.io/styleguide/cppguide.html#Inline_Functions)

Also call it AsTracedValue() for uniformity with the other code.

also maybe the signature here schould be unique_ptr<ConvertableToTraceFormat>

TracedValue is not exposed by this header. I wonder if doing that would cause
problems in v8 or blink

[chiniforooshan, 2016/10/03 18:12:19]

Done.

+    std::unique_ptr<base::trace_event::TracedValue> value(
+        new base::trace_event::TracedValue());

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

makeunique

[chiniforooshan, 2016/10/03 18:12:19]

Done.

+
+    if (scope_ != kGlobalScope)
+      value->SetString("scope", scope_);
+    switch (id_flags_) {
+      case TRACE_EVENT_FLAG_HAS_ID:
+        value->SetString(
+            "id",
+            base::StringPrintf("0x%" PRIx64, static_cast<uint64_t>(raw_id_)));
+        break;
+      case TRACE_EVENT_FLAG_HAS_GLOBAL_ID:
+        value->BeginDictionary("id2");
+        value->SetString(
+            "global",
+            base::StringPrintf("0x%" PRIx64, static_cast<uint64_t>(raw_id_)));
+        value->EndDictionary();
+        break;
+      case TRACE_EVENT_FLAG_HAS_LOCAL_ID:
+        value->BeginDictionary("id2");
+        value->SetString(
+            "local",
+            base::StringPrintf("0x%" PRIx64, static_cast<uint64_t>(raw_id_)));
+        value->EndDictionary();
+        break;
+      default:
+        NOTREACHED() << "Unrecognized ID flag";
+    }
+
+    return value;
+  }
+
  private:
   const char* scope_ = nullptr;
   unsigned long long raw_id_;