Added support to trace_event for passing static string arguments without copy

base/debug/trace_event.h

Index: base/debug/trace_event.h
diff --git a/base/debug/trace_event.h b/base/debug/trace_event.h
index 0fcdaff165927c405a33d1dd012cc8d2680bf86f..c2138a8e1a890089ba81870bd8cc6d2222895a73 100644
--- a/base/debug/trace_event.h
+++ b/base/debug/trace_event.h
@@ -24,7 +24,7 @@
 // Additional parameters can be associated with an event:
 //   void doSomethingCostly2(int howMuch) {
 //     TRACE_EVENT1("MY_SUBSYSTEM", "doSomethingCostly",
-//         "howMuch", StringPrintf("%i", howMuch).c_str());
+//         "howMuch", howMuch);
 //     ...
 //   }
 //
@@ -48,8 +48,15 @@
 // To avoid this issue with the |name| and |arg_name| parameters, use the
 // TRACE_EVENT_COPY_XXX overloads of the macros at additional runtime overhead.
 // Notes: The category must always be in a long-lived char* (i.e. static const).
-//        The |arg_values|, when used, are always deep copied and so never have
-//        this restriction.
+//        The |arg_values|, when used, are always deep copied unless type is
+//        const char*, so never have this restriction.
+//
+// const char* arg_values are referenced:
+//     TRACE_EVENT1("category", "event1",

[John Grabowski, 2011/08/30 18:13:38]

Comment should have COPY in the name (2 spots)

+//                  "arg1", "literal string is only referenced");
+// std::string arg_values are copied:
+//     TRACE_EVENT1("category", "event2",

[John Grabowski, 2011/08/30 18:13:38]

This actually does not work for us.  We convert some string types to const char*
but still need them copied.  Will send details in email.

+//                  "arg1", std::string("string will be copied"));
 //
 //
 // Thread Safety:
@@ -99,19 +106,28 @@
 // Older style trace macros with explicit id and extra data
 // Only these macros result in publishing data to ETW as currently implemented.
 #define TRACE_EVENT_BEGIN_ETW(name, id, extra) \
-  base::debug::TraceLog::AddTraceEventEtw( \
-      base::debug::TRACE_EVENT_PHASE_BEGIN, \
-      name, reinterpret_cast<const void*>(id), extra);
+  do { \
+    INTERNAL_TRACE_EVENT_ETW(name, \
+        base::debug::TRACE_EVENT_PHASE_BEGIN, id, extra); \
+    TRACE_EVENT_BEGIN2("ETW Trace Event", name, \
+        "id", reinterpret_cast<const void*>(id), "extra", extra) \
+  } while(0)

[scheib, 2011/08/30 16:16:02]

Sorry, I'm confused why the changes were needed here? Also, it appears that the
legacy trace points are being discarded on non windows platforms? Aren't they
still handy to have? E.g. the events that come from V8 via webkit
(webkitclient_impl.cc)?

[jbates, 2011/08/30 17:01:51]

The behavior should be the same as before (unless I've made a mistake). This was
just changed to reuse our existing macros, since our macros now support
std::string arguments. TraceLog::AddTraceEventEtw converted std::string to const
char*. Instead, these macros just pass through whether it's std::string or const
char*, since we now support both.

 
 #define TRACE_EVENT_END_ETW(name, id, extra) \
-  base::debug::TraceLog::AddTraceEventEtw( \
-      base::debug::TRACE_EVENT_PHASE_END, \
-      name, reinterpret_cast<const void*>(id), extra);
+  do { \
+    INTERNAL_TRACE_EVENT_ETW(name, \
+        base::debug::TRACE_EVENT_PHASE_END, id, extra); \
+    TRACE_EVENT_END2("ETW Trace Event", name, \
+        "id", reinterpret_cast<const void*>(id), "extra", extra) \
+  } while(0)
 
 #define TRACE_EVENT_INSTANT_ETW(name, id, extra) \
-  base::debug::TraceLog::AddTraceEventEtw( \
-      base::debug::TRACE_EVENT_PHASE_INSTANT, \
-      name, reinterpret_cast<const void*>(id), extra);
+  do { \
+    INTERNAL_TRACE_EVENT_ETW(name, \
+        base::debug::TRACE_EVENT_PHASE_INSTANT, id, extra); \
+    TRACE_EVENT_INSTANT2("ETW Trace Event", name, \
+        "id", reinterpret_cast<const void*>(id), "extra", extra) \
+  } while(0)
 
 // Records a pair of begin and end events called "name" for the current
 // scope, with 0, 1 or 2 associated arguments. If the category is not
@@ -279,6 +295,15 @@
         INTERNAL_TRACE_EVENT_UID(begin_event_id), threshold); \
   }
 
+// Implementation detail: support macro for windows ETW style tracing.
+#if defined(OS_WIN)
+#define INTERNAL_TRACE_EVENT_ETW(name, phase, id, extra) \
+  TraceEventETWProvider::Trace( \
+      name, phase, reinterpret_cast<const void*>(id), extra)
+#else
+#define INTERNAL_TRACE_EVENT_ETW(name, phase, id, extra)
+#endif
+
 namespace base {
 
 class RefCountedString;
@@ -313,7 +338,8 @@ class BASE_EXPORT TraceValue {
     TRACE_TYPE_INT,
     TRACE_TYPE_DOUBLE,
     TRACE_TYPE_POINTER,
-    TRACE_TYPE_STRING
+    TRACE_TYPE_STRING,
+    TRACE_TYPE_STATIC_STRING
   };
 
   TraceValue() : type_(TRACE_TYPE_UNDEFINED) {
@@ -352,7 +378,10 @@ class BASE_EXPORT TraceValue {
   TraceValue(const void* rhs) : type_(TRACE_TYPE_POINTER) {
     value_.as_pointer = rhs;
   }
-  TraceValue(const char* rhs) : type_(TRACE_TYPE_STRING) {
+  TraceValue(const std::string& rhs) : type_(TRACE_TYPE_STRING) {
+    value_.as_string = rhs.c_str();
+  }
+  TraceValue(const char* rhs) : type_(TRACE_TYPE_STATIC_STRING) {
     value_.as_string = rhs;
   }
 
@@ -382,7 +411,7 @@ class BASE_EXPORT TraceValue {
     return value_.as_pointer;
   }
   const char* as_string() const {
-    DCHECK_EQ(TRACE_TYPE_STRING, type_);
+    DCHECK(type_ == TRACE_TYPE_STRING || type_ == TRACE_TYPE_STATIC_STRING);
     return value_.as_string;
   }
   const char** as_assignable_string() {
@@ -431,6 +460,14 @@ class TraceEvent {
 
   TimeTicks timestamp() const { return timestamp_; }
 
+  // Exposed for unittesting:
+
+  const base::RefCountedString* parameter_copy_storage() const {
+    return parameter_copy_storage_.get();
+  }
+
+  const char* name() const { return name_; }
+
  private:
   unsigned long process_id_;
   unsigned long thread_id_;
@@ -491,21 +528,19 @@ class BASE_EXPORT TraceLog {
                     int threshold_begin_id,
                     int64 threshold,
                     bool copy);
-  static void AddTraceEventEtw(TraceEventPhase phase,
-                               const char* name,
-                               const void* id,
-                               const char* extra);
-  static void AddTraceEventEtw(TraceEventPhase phase,
-                               const char* name,
-                               const void* id,
-                               const std::string& extra) {
-    AddTraceEventEtw(phase, name, id, extra.c_str());
-  }
 
-  // Exposed for unittesting only, allows resurrecting our
-  // singleton instance post-AtExit processing.
+  // Exposed for unittesting:
+
+  // Allows resurrecting our singleton instance post-AtExit processing.
   static void Resurrect();
 
+  // Allow tests to inspect TraceEvents.
+  size_t GetEventsSize() const { return logged_events_.size(); }
+  const TraceEvent& GetEventAt(size_t index) const {
+    DCHECK(index < logged_events_.size());
+    return logged_events_[index];
+  }
+
  private:
   // This allows constructor and destructor to be private and usable only
   // by the Singleton class.
@@ -534,6 +569,7 @@ namespace internal {
 // Used by TRACE_EVENTx macro. Do not use directly.
 class BASE_EXPORT TraceEndOnScopeClose {
  public:
+  // Note: members of data_ intentionally left uninitialized. See Initialize.
   TraceEndOnScopeClose() : p_data_(NULL) {}
   ~TraceEndOnScopeClose() {
     if (p_data_)
@@ -563,6 +599,7 @@ class BASE_EXPORT TraceEndOnScopeClose {
 // Used by TRACE_EVENTx macro. Do not use directly.
 class BASE_EXPORT TraceEndOnScopeCloseThreshold {
  public:
+  // Note: members of data_ intentionally left uninitialized. See Initialize.
   TraceEndOnScopeCloseThreshold() : p_data_(NULL) {}
   ~TraceEndOnScopeCloseThreshold() {
     if (p_data_)