Trace category groups and category filter.

base/debug/trace_event_impl.cc

Index: base/debug/trace_event_impl.cc
diff --git a/base/debug/trace_event_impl.cc b/base/debug/trace_event_impl.cc
index cbcf54b81c6aac08b08d5f71fb6796f6869b84de..9a89fef39512a5b8728c181ad5480a58e26b47bf 100644
--- a/base/debug/trace_event_impl.cc
+++ b/base/debug/trace_event_impl.cc
@@ -396,29 +396,18 @@ const char* TraceLog::GetCategoryName(const unsigned char* category_enabled) {
   return g_categories[category_index];
 }
 
-static void EnableMatchingCategory(int category_index,
-                                   const std::vector<std::string>& patterns,
-                                   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) {
-    is_match = MatchPattern(g_categories[category_index], ci->c_str());
-    if (is_match)
-      break;
-  }
-  g_category_enabled[category_index] = is_match ?
-      matched_value : unmatched_value;
+void TraceLog::EnableIncludedCategoryGroup(int category_index,
+                              const CategoryFilter& category_filter) {
+  bool is_enabled =
+      category_filter.IsCategoryGroupEnabled(g_categories[category_index]);
+  g_category_enabled[category_index] = is_enabled ?
+      TraceLog::CATEGORY_ENABLED : 0;
 }
 
-// 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 |matched_value|. Otherwise its enabled status is set to |unmatched_value|.
-static void EnableMatchingCategories(const std::vector<std::string>& patterns,
-                                     unsigned char matched_value,
-                                     unsigned char unmatched_value) {
+void TraceLog::EnableIncludedCategoryGroups(
+                                        const CategoryFilter& category_filter) {
   for (int i = 0; i < g_category_index; i++)
-    EnableMatchingCategory(i, patterns, matched_value, unmatched_value);
+    EnableIncludedCategoryGroup(i, category_filter);
 }
 
 const unsigned char* TraceLog::GetCategoryEnabledInternal(const char* name) {
@@ -448,15 +437,10 @@ const unsigned char* TraceLog::GetCategoryEnabledInternal(const char* name) {
       g_categories[new_index] = new_name;
       DCHECK(!g_category_enabled[new_index]);
       if (enable_count_) {
-        // 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);
-        }
+        // Note that if both included and excluded patterns in the
+        // CategoryFilter are empty, we exclude nothing,
+        // thereby enabling this category group.
+        EnableIncludedCategoryGroup(new_index, category_filter_);
       } else {
         g_category_enabled[new_index] = 0;
       }
@@ -477,24 +461,135 @@ void TraceLog::GetKnownCategories(std::vector<std::string>* categories) {
     categories->push_back(g_categories[i]);
 }
 
-void TraceLog::SetEnabled(const std::vector<std::string>& included_categories,
-                          const std::vector<std::string>& excluded_categories) {
+static bool DoesCategoryGroupContainCategory(const char* category_group,
+                                             const char* category) {

[rterrazas, 2013/02/21 04:08:05]

So...I was thinking, I'd like to change this to something like
IsCategoryPatternContained(container, category_pattern) and then consume this
method from IsCategoryEnabled(...). 

wdyt?

[nduca, 2013/02/22 19:55:19]

This sounds like a metho don the CategoryFilter object, no?
DoesCategoryGroupContainCategory(const char* group) or something or other.

+  DCHECK(category);
+  CStringTokenizer category_group_tokens(category_group,
+                          category_group + strlen(category_group), ",");
+  while (category_group_tokens.GetNext()) {
+    std::string category_group_token = category_group_tokens.token();
+    // Don't allow empty tokens, nor tokens with leading or trailing space.
+    DCHECK(!category_group_token.empty() &&
+            category_group_token.at(0) != ' '&&
+            category_group_token.at(category_group_token.length() - 1) != ' ')
+            << "Disallowed category string";
+    if (MatchPattern(category_group_token.c_str(), category))
+      return true;
+  }
+  return false;
+}
+
+const char* CategoryFilter::kDefaultCategoryFilterString = "-*Debug,-*Test";
+
+CategoryFilter::CategoryFilter() {
+  Initialize(CategoryFilter::kDefaultCategoryFilterString);
+}
+
+CategoryFilter::CategoryFilter(const std::string& filter_string) {
+  Initialize(filter_string);
+}
+
+void CategoryFilter::Initialize(const std::string& filter_string) {
+  // Pass 1: Tokenize list of categories, delimited by ','.
+  StringTokenizer tokens(filter_string, ",");
+  while (tokens.GetNext()) {
+    bool is_included = true;
+    std::string category = tokens.token();
+    // Ignore empty categories.
+    if(category.empty())
+      continue;
+    // Excluded categories start with '-'.
+    if (category.at(0) == '-') {
+      // Remove '-' from category string.
+      category = category.substr(1);
+      is_included = false;
+    }
+    if (is_included)
+      included_.push_back(category);
+    else
+      excluded_.push_back(category);
+  }
+}
+
+void CategoryFilter::WriteString(std::string* out,
+                                 bool included) const {
+  std::vector<std::string>::const_iterator ci;
+  std::vector<std::string>::const_iterator end;
+  if (included) {
+    ci = included_.begin();
+    end = included_.end();
+  } else {
+    ci = excluded_.begin();
+    end = excluded_.end();
+  }
+
+  for (; ci != end; ++ci) {
+    if (included)
+      StringAppendF(out, "%s,", ci->c_str());
+    else
+      StringAppendF(out, "-%s,", ci->c_str());
+  }
+}
+
+void CategoryFilter::ToString(std::string* filter_string) const {
+  WriteString(filter_string, true);
+  WriteString(filter_string, false);
+}
+
+bool CategoryFilter::IsCategoryGroupEnabled(const char* category_group_name)

[rterrazas, 2013/02/20 02:40:06]

This does a lot of the work that was done through
EnableMatching...(..,CATEGORY_ENABLED, 0)/EnableMatching...(..,0,
CATEGORY_ENABLED).

I almost did things that way, but we gained no more readability IMHO, but this
is simpler to read IMHO, anyhow...Lmkwyt.

+                                                                         const {
+  // TraceLog should call this method only as  part of enabling/disabling
+  // categories.
+  std::vector<std::string>::const_iterator ci = included_.begin();
+  for (; ci != included_.end(); ++ci) {
+    if (DoesCategoryGroupContainCategory(category_group_name, ci->c_str()))
+      return true;
+  }
+  ci = excluded_.begin();
+  for (; ci != excluded_.end(); ++ci) {
+    if (DoesCategoryGroupContainCategory(category_group_name, ci->c_str()))
+      return false;
+  }
+  // If the category group is not excluded, and there are no included patterns
+  // we consider this pattern enabled.
+  return included_.empty();
+}
+
+void CategoryFilter::Merge(const CategoryFilter& nested_filter) {
+ included_.insert(included_.end(),
+                  nested_filter.included_.begin(),
+                  nested_filter.included_.end());
+  excluded_.insert(excluded_.end(),
+                   nested_filter.excluded_.begin(),
+                   nested_filter.excluded_.end());
+}
+
+bool CategoryFilter::HasIncludedCategories() const {
+  return !included_.empty();
+}
+
+void CategoryFilter::Clear() {
+  included_.clear();
+  excluded_.clear();
+}
+
+void TraceLog::SetEnabled(const CategoryFilter& category_filter) {
+
+  // Actually enable/disable the categories.
   AutoLock lock(lock_);
 
   if (enable_count_++ > 0) {
     // Tracing is already enabled, so just merge in enabled categories.
     // We only expand the set of enabled categories upon nested SetEnable().
-    if (!included_categories_.empty() && !included_categories.empty()) {
-      included_categories_.insert(included_categories_.end(),
-                                  included_categories.begin(),
-                                  included_categories.end());
-      EnableMatchingCategories(included_categories_, CATEGORY_ENABLED, 0);
+    if (category_filter_.HasIncludedCategories() &&
+        category_filter.HasIncludedCategories()) {
+      category_filter_.Merge(category_filter);
     } else {
-      // If either old or new included categories are empty, allow all events.
-      included_categories_.clear();
-      excluded_categories_.clear();
-      EnableMatchingCategories(excluded_categories_, 0, CATEGORY_ENABLED);
+      // If either old or new included categories are empty, allow all not
+      // excluded events.
+      category_filter_.Clear();
     }
+    EnableIncludedCategoryGroups(category_filter_);
     return;
   }
 
@@ -510,45 +605,14 @@ void TraceLog::SetEnabled(const std::vector<std::string>& included_categories,
   dispatching_to_observer_list_ = false;
 
   logged_events_.reserve(1024);
-  included_categories_ = included_categories;
-  excluded_categories_ = excluded_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_, CATEGORY_ENABLED, 0);
-  else
-    EnableMatchingCategories(excluded_categories_, 0, CATEGORY_ENABLED);
-}
-
-void TraceLog::SetEnabled(const std::string& categories) {
-  std::vector<std::string> included, excluded;
-  // Tokenize list of categories, delimited by ','.
-  StringTokenizer tokens(categories, ",");
-  while (tokens.GetNext()) {
-    bool is_included = true;
-    std::string category = tokens.token();
-    // Excluded categories start with '-'.
-    if (category.at(0) == '-') {
-      // Remove '-' from category string.
-      category = category.substr(1);
-      is_included = false;
-    }
-    if (is_included)
-      included.push_back(category);
-    else
-      excluded.push_back(category);
-  }
-  SetEnabled(included, excluded);
+  category_filter_ = CategoryFilter(category_filter);
+  EnableIncludedCategoryGroups(category_filter_);
 }
 
-void TraceLog::GetEnabledTraceCategories(
-    std::vector<std::string>* included_out,
-    std::vector<std::string>* excluded_out) {
+CategoryFilter& TraceLog::GetCurrentCategoryFilter() {
   AutoLock lock(lock_);
-  if (enable_count_) {
-    *included_out = included_categories_;
-    *excluded_out = excluded_categories_;
-  }
+  DCHECK(enable_count_ > 0);
+  return category_filter_;
 }
 
 void TraceLog::SetDisabled() {
@@ -568,8 +632,7 @@ void TraceLog::SetDisabled() {
                     OnTraceLogWillDisable());
   dispatching_to_observer_list_ = false;
 
-  included_categories_.clear();
-  excluded_categories_.clear();
+  category_filter_.Clear();
   watch_category_ = NULL;
   watch_event_name_ = "";
   for (int i = 0; i < g_category_index; i++)
@@ -577,13 +640,6 @@ void TraceLog::SetDisabled() {
   AddThreadNameMetadataEvents();
 }
 
-void TraceLog::SetEnabled(bool enabled) {
-  if (enabled)
-    SetEnabled(std::vector<std::string>(), std::vector<std::string>());
-  else
-    SetDisabled();
-}
-
 void TraceLog::AddEnabledStateObserver(EnabledStateChangedObserver* listener) {
   enabled_state_observer_list_.AddObserver(listener);
 }