Enable trace with multiple categories with any one category ON

As per documentation   https://code.google.com/p/chromium/codesearch#chromium/src/base/trace_event/trace_event.h&l=27
for any trace event which specifies category group, the trace event needs to be
recorded when either of category present in the group is enabled.

Current implementation short circuits the check for enabled category group
and returns false as soon as any category is found which is not enabled.
Rather, short circuit for returning true when any enabled category is found.
Otherwise, should check for each category present in the group until any
one enabled category is found. Return false only if no enabled category
is found.

BUG=463385

TESTS=Updated TraceEventTestFixture tests in base_unittests

Review URL: https://codereview.chromium.org/971673004

Cr-Commit-Position: refs/heads/master@{#319417}

2 files changed, 28 insertions, 11 deletions

diff --git a/base/trace_event/trace_event_impl.cc b/base/trace_event/trace_event_impl.cc
index 445cb6d..34bbb28 100644
--- a/base/trace_event/trace_event_impl.cc
+++ b/base/trace_event/trace_event_impl.cc
@@ -2490,18 +2490,35 @@ 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 category_group_disabled = false;
   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.
+        // 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.
+        category_group_disabled = true;
+        break;
+      }
+      // 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.
+      category_group_disabled = false;
     }
+    // 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 (!category_group_disabled)
+      break;
   }
   // 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.
-  return included_.empty() && had_enabled_by_default;
+  return !category_group_disabled &&
+         included_.empty() && had_enabled_by_default;
 }
 
 bool CategoryFilter::IsCategoryEnabled(const char* category_name) const {
diff --git a/base/trace_event/trace_event_unittest.cc b/base/trace_event/trace_event_unittest.cc
index a6bc0d4..0d3b091 100644
--- a/base/trace_event/trace_event_unittest.cc
+++ b/base/trace_event/trace_event_unittest.cc
@@ -1205,8 +1205,8 @@ TEST_F(TraceEventTestFixture, Categories) {
   EndTraceAndFlush();
   EXPECT_TRUE(FindMatchingValue("cat", "inc2"));
   EXPECT_FALSE(FindMatchingValue("cat", "inc"));
-  EXPECT_FALSE(FindMatchingValue("cat", "inc2,inc"));
-  EXPECT_FALSE(FindMatchingValue("cat", "inc,inc2"));
+  EXPECT_TRUE(FindMatchingValue("cat", "inc2,inc"));
+  EXPECT_TRUE(FindMatchingValue("cat", "inc,inc2"));
 
   // Exclude existent wildcard -> all categories not matching wildcard
   Clear();
@@ -2624,18 +2624,18 @@ TEST_F(TraceEventTestFixture, CategoryFilter) {
   EXPECT_TRUE(default_cf.IsCategoryGroupEnabled("not-excluded-category"));
   EXPECT_FALSE(
       default_cf.IsCategoryGroupEnabled("disabled-by-default-category"));
-  EXPECT_FALSE(default_cf.IsCategoryGroupEnabled("Category1,CategoryDebug"));
-  EXPECT_FALSE(default_cf.IsCategoryGroupEnabled("CategoryDebug,Category1"));
-  EXPECT_FALSE(default_cf.IsCategoryGroupEnabled("CategoryTest,Category2"));
+  EXPECT_TRUE(default_cf.IsCategoryGroupEnabled("Category1,CategoryDebug"));
+  EXPECT_TRUE(default_cf.IsCategoryGroupEnabled("CategoryDebug,Category1"));
+  EXPECT_TRUE(default_cf.IsCategoryGroupEnabled("CategoryTest,Category2"));
 
   // Make sure that upon an empty string, we fall back to the default filter.
   default_cf = CategoryFilter();
   category_filter_str = default_cf.ToString();
   EXPECT_STREQ("-*Debug,-*Test", category_filter_str.c_str());
   EXPECT_TRUE(default_cf.IsCategoryGroupEnabled("not-excluded-category"));
-  EXPECT_FALSE(default_cf.IsCategoryGroupEnabled("Category1,CategoryDebug"));
-  EXPECT_FALSE(default_cf.IsCategoryGroupEnabled("CategoryDebug,Category1"));
-  EXPECT_FALSE(default_cf.IsCategoryGroupEnabled("CategoryTest,Category2"));
+  EXPECT_TRUE(default_cf.IsCategoryGroupEnabled("Category1,CategoryDebug"));
+  EXPECT_TRUE(default_cf.IsCategoryGroupEnabled("CategoryDebug,Category1"));
+  EXPECT_TRUE(default_cf.IsCategoryGroupEnabled("CategoryTest,Category2"));
 
   // Using an arbitrary non-empty filter.
   CategoryFilter cf("included,-excluded,inc_pattern*,-exc_pattern*");