Put Android systrace calls in trace_event_impl only.

http://codereview.chromium.org/11345019/ added code in both trace_event.h/cc
and trace_event_impl.h/cc to support Android systrace. However, to make
TRACE_EVENT in Webkit work, we would also need to change the TraceEvent.h in
WebKit.

This change removes Android systrace calls from trace_event.h/cc and put all
ATrace specific code into trace_event_impl.h/cc so that trace_event for Android
can work for WebKit without changing WebKit and/or webkit glue code.

TBR=brettw
BUG=


Review URL: https://chromiumcodereview.appspot.com/11360208

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@167983 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 72 insertions, 58 deletions

diff --git a/base/debug/trace_event.cc b/base/debug/trace_event.cc
index 4298bba..be77d5a 100644
--- a/base/debug/trace_event.cc
+++ b/base/debug/trace_event.cc
@@ -15,7 +15,7 @@ void TraceEndOnScopeClose::Initialize(const unsigned char* category_enabled,
 
 void TraceEndOnScopeClose::AddEventIfEnabled() {
   // Only called when p_data_ is non-null.
-  if (TRACE_EVENT_API_IS_ATRACE_ENABLED() || *p_data_->category_enabled) {
+  if (*p_data_->category_enabled) {
     TRACE_EVENT_API_ADD_TRACE_EVENT(
         TRACE_EVENT_PHASE_END,
         p_data_->category_enabled,
@@ -39,7 +39,7 @@ void TraceEndOnScopeCloseThreshold::Initialize(
 
 void TraceEndOnScopeCloseThreshold::AddEventIfEnabled() {
   // Only called when p_data_ is non-null.
-  if (TRACE_EVENT_API_IS_ATRACE_ENABLED() || *p_data_->category_enabled) {
+  if (*p_data_->category_enabled) {
     TRACE_EVENT_API_ADD_TRACE_EVENT(
         TRACE_EVENT_PHASE_END,
         p_data_->category_enabled,
diff --git a/base/debug/trace_event.h b/base/debug/trace_event.h
index f78bb3c..07863ad 100644
--- a/base/debug/trace_event.h
+++ b/base/debug/trace_event.h
@@ -597,14 +597,6 @@
 #define TRACE_EVENT_API_ADD_TRACE_EVENT \
     base::debug::TraceLog::GetInstance()->AddTraceEvent
 
-// Checks if Android ATrace is enabled.
-#if defined(OS_ANDROID)
-#define TRACE_EVENT_API_IS_ATRACE_ENABLED() \
-    base::debug::TraceLog::IsATraceEnabled()
-#else
-#define TRACE_EVENT_API_IS_ATRACE_ENABLED() false
-#endif
-
 ////////////////////////////////////////////////////////////////////////////////
 
 // Implementation detail: trace event macros create temporary variables
@@ -634,17 +626,12 @@
               INTERNAL_TRACE_EVENT_UID(catstatic))); \
     }
 
-// Implementation detail: internal macro to check if Android ATrace or the
-// current category (using the current static category variable) is enabled.
-#define INTERNAL_TRACE_EVENT_IS_ATRACE_OR_CATEGORY_ENABLED() \
-    TRACE_EVENT_API_IS_ATRACE_ENABLED() || *INTERNAL_TRACE_EVENT_UID(catstatic)
-
 // Implementation detail: internal macro to create static category and add
 // event if the category is enabled.
 #define INTERNAL_TRACE_EVENT_ADD(phase, category, name, flags, ...) \
     do { \
       INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
-      if (INTERNAL_TRACE_EVENT_IS_ATRACE_OR_CATEGORY_ENABLED()) { \
+      if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
         trace_event_internal::AddTraceEvent( \
             phase, INTERNAL_TRACE_EVENT_UID(catstatic), name, \
             trace_event_internal::kNoEventId, flags, ##__VA_ARGS__); \
@@ -658,7 +645,7 @@
     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
     trace_event_internal::TraceEndOnScopeClose \
         INTERNAL_TRACE_EVENT_UID(profileScope); \
-    if (INTERNAL_TRACE_EVENT_IS_ATRACE_OR_CATEGORY_ENABLED()) { \
+    if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
       trace_event_internal::AddTraceEvent( \
           TRACE_EVENT_PHASE_BEGIN, \
           INTERNAL_TRACE_EVENT_UID(catstatic), \
@@ -676,7 +663,7 @@
     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
     trace_event_internal::TraceEndOnScopeCloseThreshold \
         INTERNAL_TRACE_EVENT_UID(profileScope); \
-    if (INTERNAL_TRACE_EVENT_IS_ATRACE_OR_CATEGORY_ENABLED()) { \
+    if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
       int INTERNAL_TRACE_EVENT_UID(begin_event_id) = \
         trace_event_internal::AddTraceEvent( \
             TRACE_EVENT_PHASE_BEGIN, \
@@ -694,7 +681,7 @@
                                          ...) \
     do { \
       INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
-      if (INTERNAL_TRACE_EVENT_IS_ATRACE_OR_CATEGORY_ENABLED()) { \
+      if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
         unsigned char trace_event_flags = flags | TRACE_EVENT_FLAG_HAS_ID; \
         trace_event_internal::TraceID trace_event_trace_id( \
             id, &trace_event_flags); \
diff --git a/base/debug/trace_event_android.cc b/base/debug/trace_event_android.cc
index 2fbbc44..c56c3c1 100644
--- a/base/debug/trace_event_android.cc
+++ b/base/debug/trace_event_android.cc
@@ -30,11 +30,6 @@ void TraceLog::InitATrace() {
     LOG(WARNING) << "Couldn't open " << kATraceMarkerFile;
 }
 
-// static
-bool TraceLog::IsATraceEnabled() {
-  return g_atrace_fd != -1;
-}
-
 void TraceLog::SendToATrace(char phase,
                             const char* category,
                             const char* name,
@@ -42,7 +37,7 @@ void TraceLog::SendToATrace(char phase,
                             const char** arg_names,
                             const unsigned char* arg_types,
                             const unsigned long long* arg_values) {
-  if (!IsATraceEnabled())
+  if (g_atrace_fd == -1)
     return;
 
   switch (phase) {
@@ -139,5 +134,11 @@ void TraceLog::AddClockSyncMetadataEvents() {
     close(atrace_fd);
 }
 
+// Must be called with lock_ locked.
+void TraceLog::ApplyATraceEnabledFlag(unsigned char* category_enabled) {
+  if (g_atrace_fd != -1)
+    *category_enabled |= ATRACE_ENABLED;
+}
+
 }  // namespace debug
 }  // namespace base
diff --git a/base/debug/trace_event_impl.cc b/base/debug/trace_event_impl.cc
index fa23def..823acbe 100644
--- a/base/debug/trace_event_impl.cc
+++ b/base/debug/trace_event_impl.cc
@@ -395,7 +395,8 @@ const char* TraceLog::GetCategoryName(const unsigned char* category_enabled) {
 
 static void EnableMatchingCategory(int category_index,
                                    const std::vector<std::string>& patterns,
-                                   unsigned char is_included) {
+                                   unsigned char matched_value,
+                                   unsigned char unmatched_value) {
   std::vector<std::string>::const_iterator ci = patterns.begin();
   bool is_match = false;
   for (; ci != patterns.end(); ++ci) {
@@ -404,53 +405,67 @@ static void EnableMatchingCategory(int category_index,
       break;
   }
   g_category_enabled[category_index] = is_match ?
-      is_included : (is_included ^ 1);
+      matched_value : unmatched_value;
 }
 
 // Enable/disable each category based on the category filters in |patterns|.
 // If the category name matches one of the patterns, its enabled status is set
-// to |is_included|. Otherwise its enabled status is set to !|is_included|.
+// to |matched_value|. Otherwise its enabled status is set to |unmatched_value|.
 static void EnableMatchingCategories(const std::vector<std::string>& patterns,
-                                     unsigned char is_included) {
+                                     unsigned char matched_value,
+                                     unsigned char unmatched_value) {
   for (int i = 0; i < g_category_index; i++)
-    EnableMatchingCategory(i, patterns, is_included);
+    EnableMatchingCategory(i, patterns, matched_value, unmatched_value);
 }
 
 const unsigned char* TraceLog::GetCategoryEnabledInternal(const char* name) {
   AutoLock lock(lock_);
   DCHECK(!strchr(name, '"')) << "Category names may not contain double quote";
 
+  unsigned char* category_enabled = NULL;
   // Search for pre-existing category matching this name
   for (int i = 0; i < g_category_index; i++) {
-    if (strcmp(g_categories[i], name) == 0)
-      return &g_category_enabled[i];
+    if (strcmp(g_categories[i], name) == 0) {
+      category_enabled = &g_category_enabled[i];
+      break;
+    }
   }
 
-  // Create a new category
-  DCHECK(g_category_index < TRACE_EVENT_MAX_CATEGORIES) <<
-      "must increase TRACE_EVENT_MAX_CATEGORIES";
-  if (g_category_index < TRACE_EVENT_MAX_CATEGORIES) {
-    int new_index = g_category_index++;
-    // Don't hold on to the name pointer, so that we can create categories with
-    // strings not known at compile time (this is required by SetWatchEvent).
-    const char* new_name = base::strdup(name);
-    ANNOTATE_LEAKING_OBJECT_PTR(new_name);
-    g_categories[new_index] = new_name;
-    DCHECK(!g_category_enabled[new_index]);
-    if (enabled_) {
-      // Note that if both included and excluded_categories are empty, the else
-      // clause below excludes nothing, thereby enabling this category.
-      if (!included_categories_.empty())
-        EnableMatchingCategory(new_index, included_categories_, 1);
-      else
-        EnableMatchingCategory(new_index, excluded_categories_, 0);
+  if (!category_enabled) {
+    // Create a new category
+    DCHECK(g_category_index < TRACE_EVENT_MAX_CATEGORIES) <<
+        "must increase TRACE_EVENT_MAX_CATEGORIES";
+    if (g_category_index < TRACE_EVENT_MAX_CATEGORIES) {
+      int new_index = g_category_index++;
+      // Don't hold on to the name pointer, so that we can create categories
+      // with strings not known at compile time (this is required by
+      // SetWatchEvent).
+      const char* new_name = base::strdup(name);
+      ANNOTATE_LEAKING_OBJECT_PTR(new_name);
+      g_categories[new_index] = new_name;
+      DCHECK(!g_category_enabled[new_index]);
+      if (enabled_) {
+        // Note that if both included and excluded_categories are empty, the
+        // else clause below excludes nothing, thereby enabling this category.
+        if (!included_categories_.empty()) {
+          EnableMatchingCategory(new_index, included_categories_,
+                                 CATEGORY_ENABLED, 0);
+        } else {
+          EnableMatchingCategory(new_index, excluded_categories_,
+                                 0, CATEGORY_ENABLED);
+        }
+      } else {
+        g_category_enabled[new_index] = 0;
+      }
+      category_enabled = &g_category_enabled[new_index];
     } else {
-      g_category_enabled[new_index] = 0;
+      category_enabled = &g_category_enabled[g_category_categories_exhausted];
     }
-    return &g_category_enabled[new_index];
-  } else {
-    return &g_category_enabled[g_category_categories_exhausted];
   }
+#if defined(OS_ANDROID)
+  ApplyATraceEnabledFlag(category_enabled);
+#endif
+  return category_enabled;
 }
 
 void TraceLog::GetKnownCategories(std::vector<std::string>* categories) {
@@ -483,9 +498,9 @@ void TraceLog::SetEnabled(const std::vector<std::string>& included_categories,
   // Note that if both included and excluded_categories are empty, the else
   // clause below excludes nothing, thereby enabling all categories.
   if (!included_categories_.empty())
-    EnableMatchingCategories(included_categories_, 1);
+    EnableMatchingCategories(included_categories_, CATEGORY_ENABLED, 0);
   else
-    EnableMatchingCategories(excluded_categories_, 0);
+    EnableMatchingCategories(excluded_categories_, 0, CATEGORY_ENABLED);
 }
 
 void TraceLog::SetEnabled(const std::string& categories) {
@@ -617,7 +632,7 @@ int TraceLog::AddTraceEvent(char phase,
   int ret_begin_id = -1;
   {
     AutoLock lock(lock_);
-    if (!*category_enabled)
+    if (*category_enabled != CATEGORY_ENABLED)
       return -1;
     if (logged_events_.size() >= kTraceEventBufferSize)
       return -1;
diff --git a/base/debug/trace_event_impl.h b/base/debug/trace_event_impl.h
index c186373..33e782c 100644
--- a/base/debug/trace_event_impl.h
+++ b/base/debug/trace_event_impl.h
@@ -210,7 +210,6 @@ class BASE_EXPORT TraceLog {
 
 #if defined(OS_ANDROID)
   static void InitATrace();
-  static bool IsATraceEnabled();
 #endif
 
   // Enabled state listeners give a callback when tracing is enabled or
@@ -316,6 +315,17 @@ class BASE_EXPORT TraceLog {
   // by the Singleton class.
   friend struct StaticMemorySingletonTraits<TraceLog>;
 
+  // The pointer returned from GetCategoryEnabledInternal() points to a value
+  // with zero or more of the following bits. Used in this class only.
+  // The TRACE_EVENT macros should only use the value as a bool.
+  enum CategoryEnabledFlags {
+    // Normal enabled flag for categories enabled with Enable().
+    CATEGORY_ENABLED = 1 << 0,
+    // On Android if ATrace is enabled, all categories will have this bit.
+    // Not used on other platforms.
+    ATRACE_ENABLED = 1 << 1
+  };
+
   // Helper class for managing notification_thread_count_ and running
   // notification callbacks. This is very similar to a reader-writer lock, but
   // shares the lock with TraceLog and manages the notification flags.
@@ -353,6 +363,7 @@ class BASE_EXPORT TraceLog {
                     const unsigned char* arg_types,
                     const unsigned long long* arg_values);
   void AddClockSyncMetadataEvents();
+  static void ApplyATraceEnabledFlag(unsigned char* category_enabled);
 #endif
 
   // TODO(nduca): switch to per-thread trace buffers to reduce thread