[gin] Add ConvertableToTraceFormatWrapper

gin/v8_platform.cc

Index: gin/v8_platform.cc
diff --git a/gin/v8_platform.cc b/gin/v8_platform.cc
index f9d2d6acc3115673ac131143bf82709b6e4a92b9..9001b633ada9813b3048c37acc59aa0454ab940d 100644
--- a/gin/v8_platform.cc
+++ b/gin/v8_platform.cc
@@ -86,28 +86,47 @@ const char* V8Platform::GetCategoryGroupName(
       category_enabled_flag);
 }
 
-uint64_t V8Platform::AddTraceEvent(char phase,
-                                   const uint8_t* category_enabled_flag,
-                                   const char* name,
-                                   const char* scope,
-                                   uint64_t id,
-                                   uint64_t bind_id,
-                                   int32_t num_args,
-                                   const char** arg_names,
-                                   const uint8_t* arg_types,
-                                   const uint64_t* arg_values,
-                                   unsigned int flags) {
+namespace {
+
+class ConvertableToTraceFormatWrapper
+    : public base::trace_event::ConvertableToTraceFormat {
+ public:
+  explicit ConvertableToTraceFormatWrapper(

[jochen (gone - plz use gerrit), 2016/09/26 15:04:37]

please add a virtual dtor

[alph, 2016/09/26 22:06:56]

I wonder why? I'm fine with the default destructor here. I didn't find anything
about it in the style guide either.

[jochen (gone - plz use gerrit), 2016/09/27 19:09:04]

Assuming you'd move this class to a header, it would be a non-trivial dtor, and
then it shouldn't be inline. I think it's good practice to apply the same rules
to all classes.

[alph, 2016/09/27 19:24:23]

Done. Though I don't see how it helps. :-)
If I move the class into a header, I'd still need to move the implementation out
of the class and keep it in .cc to avoid inlining.
Btw the same applies to the constructor.

+      std::unique_ptr<v8::ConvertableToTraceFormat>& inner)
+      : inner_(std::move(inner)) {}
+  void AppendAsTraceFormat(std::string* out) const final {
+    inner_->AppendAsTraceFormat(out);
+  }
+
+ private:
+  std::unique_ptr<v8::ConvertableToTraceFormat> inner_;
+};

[jochen (gone - plz use gerrit), 2016/09/26 15:04:37]

disallow copy / assign

[alph, 2016/09/26 22:06:56]

Done.

+
+}  // namespace
+
+uint64_t V8Platform::AddTraceEvent(
+    char phase,
+    const uint8_t* category_enabled_flag,
+    const char* name,
+    const char* scope,
+    uint64_t id,
+    uint64_t bind_id,
+    int32_t num_args,
+    const char** arg_names,
+    const uint8_t* arg_types,
+    const uint64_t* arg_values,
+    std::unique_ptr<v8::ConvertableToTraceFormat>* arg_convertables,
+    unsigned int flags) {
   std::unique_ptr<base::trace_event::ConvertableToTraceFormat> convertables[2];
   if (num_args > 0 && arg_types[0] == TRACE_VALUE_TYPE_CONVERTABLE) {
     convertables[0].reset(
-        reinterpret_cast<base::trace_event::ConvertableToTraceFormat*>(
-            arg_values[0]));
+        new ConvertableToTraceFormatWrapper(arg_convertables[0]));
   }
   if (num_args > 1 && arg_types[1] == TRACE_VALUE_TYPE_CONVERTABLE) {
     convertables[1].reset(
-        reinterpret_cast<base::trace_event::ConvertableToTraceFormat*>(
-            arg_values[1]));
+        new ConvertableToTraceFormatWrapper(arg_convertables[1]));
   }
+  DCHECK(num_args <= 2);

[jochen (gone - plz use gerrit), 2016/09/26 15:04:37]

DCHECK_LT

[alph, 2016/09/26 22:06:56]

Done.

   base::trace_event::TraceEventHandle handle =
       TRACE_EVENT_API_ADD_TRACE_EVENT_WITH_BIND_ID(
           phase, category_enabled_flag, name, scope, id, bind_id, num_args,