[gin] Add ConvertableToTraceFormatWrapper

gin/v8_platform.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gin/public/v8_platform.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/sys_info.h"
 #include "base/threading/worker_pool.h"
@@ skipping 68 matching lines @@
 const uint8_t* V8Platform::GetCategoryGroupEnabled(const char* name) {
   return TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(name);
 }
 
 const char* V8Platform::GetCategoryGroupName(
     const uint8_t* category_enabled_flag) {
   return base::trace_event::TraceLog::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,
           arg_names, arg_types, (const long long unsigned int*)arg_values,
           convertables, flags);
   uint64_t result;
   memcpy(&result, &handle, sizeof(result));
   return result;
 }
 
@@ skipping 62 matching lines @@
     v8::Platform::TraceStateObserver* observer) {
   g_trace_state_dispatcher.Get().AddObserver(observer);
 }
 
 void V8Platform::RemoveTraceStateObserver(
     v8::Platform::TraceStateObserver* observer) {
   g_trace_state_dispatcher.Get().RemoveObserver(observer);
 }
 
 }  // namespace gin