Enable trace with multiple categories with any one category ON

base/trace_event/trace_event_impl.cc

Index: base/trace_event/trace_event_impl.cc
diff --git a/base/trace_event/trace_event_impl.cc b/base/trace_event/trace_event_impl.cc
index 445cb6db97aad8cedd410c60ef3447ab731c86ce..6374c785099b3442019bb46149d81e7a08a146ca 100644
--- a/base/trace_event/trace_event_impl.cc
+++ b/base/trace_event/trace_event_impl.cc
@@ -2490,14 +2490,36 @@ bool CategoryFilter::IsCategoryGroupEnabled(
   // Do a second pass to check for explicitly disabled categories
   // (those explicitly enabled have priority due to first pass).
   category_group_tokens.Reset();
+  bool had_category_group_enabled = true;

[dsinclair, 2015/03/03 14:54:05]

This flag is backwards to what we're checking and makes the logic hard to
follow. Can you rename it to had_category_group_disabled and update the logic
below? This check is running through the explicitly disabled categories.

Actually, can we call it 'all_category_groups_disabled' I think that would be
the clearest. Then as soon as something doesn't match in the loops below, you
can set it to true.

You then don't need the final if (!had_category_group_enabled) you can put it
right into the return as:

return !all_category_groups_disabled || (included_.empty() &&
had_enabled_by_default)

   while (category_group_tokens.GetNext()) {
     std::string category_group_token = category_group_tokens.token();
     for (StringList::const_iterator ci = excluded_.begin();
          ci != excluded_.end(); ++ci) {
-      if (MatchPattern(category_group_token.c_str(), ci->c_str()))
-        return false;
+      if (MatchPattern(category_group_token.c_str(), ci->c_str())) {
+        // Current token of category_group_name is present in excluded_ list.

[dsinclair, 2015/03/03 14:54:05]

Nit: remove space between excluded_ and list.

+        // Flag the exclusion and proceed further to check if any of the
+        // remaining categories of category_group_name is not present in the
+        // excluded_ list.
+        had_category_group_enabled = false;
+        break;
+      } else {

[dsinclair, 2015/03/03 14:54:05]

Since the if () has a break, you don't need else here. We can only get here if
the if didn't execute.

+        // One of the category of category_group_name is not present in
+        // excluded_ list. So, it has to be included_ list. Enable the
+        // category_group_name for recording.
+        had_category_group_enabled = true;
+      }
     }
+    // One of the categories present in category_group_name is not present in
+    // excluded_ list. Implies this category_group_name group can be enabled
+    // for recording, since one of its groups is enabled for recording.
+    if (had_category_group_enabled)
+      break;
   }
+  // None of the categories part of category_group_name are enabled.
+  // Don't allow recording of this category_group_name.
+  if (!had_category_group_enabled)
+    return false;
+
   // If the category group is not excluded, and there are no included patterns
   // we consider this category group enabled, as long as it had categories
   // other than disabled-by-default.