Added a whitelist for trace events that are known to have args without PII.

Companion CL: https://codereview.chromium.org/1067233002/

R=nduca,davidben,jam
BUG=466769

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

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

base/trace_event/trace_event_impl.cc

@@ -68,6 +68,7 @@ const char kRecordAsMuchAsPossible[] = "record-as-much-as-possible";
 const char kTraceToConsole[] = "trace-to-console";
 const char kEnableSampling[] = "enable-sampling";
 const char kEnableSystrace[] = "enable-systrace";
+const char kEnableArgumentFilter[] = "enable-argument-filter";
 
 // Controls the number of trace events we will buffer in-memory
 // before throwing them away.
@@ -695,22 +696,28 @@ void TraceEvent::AppendValueAsJSON(unsigned char type,
   }
 }
 
-void TraceEvent::AppendAsJSON(std::string* out) const {
+void TraceEvent::AppendAsJSON(
+    std::string* out,
+    const ArgumentFilterPredicate& argument_filter_predicate) const {
   int64 time_int64 = timestamp_.ToInternalValue();
   int process_id = TraceLog::GetInstance()->process_id();
+  const char* category_group_name =
+      TraceLog::GetCategoryGroupName(category_group_enabled_);
+
   // Category group checked at category creation time.
   DCHECK(!strchr(name_, '"'));
-  StringAppendF(out,
-      "{\"pid\":%i,\"tid\":%i,\"ts\":%" PRId64 ","
+  StringAppendF(out, "{\"pid\":%i,\"tid\":%i,\"ts\":%" PRId64
+                     ","
                      "\"ph\":\"%c\",\"cat\":\"%s\",\"name\":\"%s\",\"args\":{",
-      process_id,
-      thread_id_,
-      time_int64,
-      phase_,
-      TraceLog::GetCategoryGroupName(category_group_enabled_),
+                process_id, thread_id_, time_int64, phase_, category_group_name,
                 name_);
 
   // Output argument names and values, stop at first NULL argument name.
+  if (arg_names_[0]) {
+    bool allow_args = argument_filter_predicate.is_null() ||
+                      argument_filter_predicate.Run(category_group_name, name_);
+
+    if (allow_args) {
       for (int i = 0; i < kTraceMaxNumArgs && arg_names_[i]; ++i) {
         if (i > 0)
           *out += ",";
@@ -723,6 +730,11 @@ void TraceEvent::AppendAsJSON(std::string* out) const {
         else
           AppendValueAsJSON(arg_types_[i], arg_values_[i], out);
       }
+    } else {
+      *out += "\"stripped\":1";
+    }
+  }
+
   *out += "}";
 
   if (phase_ == TRACE_EVENT_PHASE_COMPLETE) {
@@ -1003,6 +1015,8 @@ bool TraceOptions::SetFromString(const std::string& options_string) {
       enable_sampling = true;
     } else if (*iter == kEnableSystrace) {
       enable_systrace = true;
+    } else if (*iter == kEnableArgumentFilter) {
+      enable_argument_filter = true;
     } else {
       return false;
     }
@@ -1032,6 +1046,8 @@ std::string TraceOptions::ToString() const {
     ret = ret + "," + kEnableSampling;
   if (enable_systrace)
     ret = ret + "," + kEnableSystrace;
+  if (enable_argument_filter)
+    ret = ret + "," + kEnableArgumentFilter;
   return ret;
 }
 
@@ -1486,10 +1502,20 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
   }
 }
 
+void TraceLog::SetArgumentFilterPredicate(
+    const TraceEvent::ArgumentFilterPredicate& argument_filter_predicate) {
+  AutoLock lock(lock_);
+  DCHECK(!argument_filter_predicate.is_null());
+  DCHECK(argument_filter_predicate_.is_null());
+  argument_filter_predicate_ = argument_filter_predicate;
+}
+
 TraceLog::InternalTraceOptions TraceLog::GetInternalOptionsFromTraceOptions(
     const TraceOptions& options) {
   InternalTraceOptions ret =
       options.enable_sampling ? kInternalEnableSampling : kInternalNone;
+  if (options.enable_argument_filter)
+    ret |= kInternalEnableArgumentFilter;
   switch (options.record_mode) {
     case RECORD_UNTIL_FULL:
       return ret | kInternalRecordUntilFull;
@@ -1513,6 +1539,7 @@ TraceOptions TraceLog::GetCurrentTraceOptions() const {
   TraceOptions ret;
   InternalTraceOptions option = trace_options();
   ret.enable_sampling = (option & kInternalEnableSampling) != 0;
+  ret.enable_argument_filter = (option & kInternalEnableArgumentFilter) != 0;
   if (option & kInternalRecordUntilFull)
     ret.record_mode = RECORD_UNTIL_FULL;
   else if (option & kInternalRecordContinuously)
@@ -1756,8 +1783,8 @@ void TraceLog::Flush(const TraceLog::OutputCallback& cb,
 // Usually it runs on a different thread.
 void TraceLog::ConvertTraceEventsToTraceFormat(
     scoped_ptr<TraceBuffer> logged_events,
-    const TraceLog::OutputCallback& flush_output_callback) {
-
+    const OutputCallback& flush_output_callback,
+    const TraceEvent::ArgumentFilterPredicate& argument_filter_predicate) {
   if (flush_output_callback.is_null())
     return;
 
@@ -1776,7 +1803,8 @@ void TraceLog::ConvertTraceEventsToTraceFormat(
       for (size_t j = 0; j < chunk->size(); ++j) {
         if (json_events_str_ptr->size())
           json_events_str_ptr->data().append(",\n");
-        chunk->GetEventAt(j)->AppendAsJSON(&(json_events_str_ptr->data()));
+        chunk->GetEventAt(j)->AppendAsJSON(&(json_events_str_ptr->data()),
+                                           argument_filter_predicate);
       }
     }
     flush_output_callback.Run(json_events_str_ptr, has_more_events);
@@ -1786,6 +1814,7 @@ void TraceLog::ConvertTraceEventsToTraceFormat(
 void TraceLog::FinishFlush(int generation) {
   scoped_ptr<TraceBuffer> previous_logged_events;
   OutputCallback flush_output_callback;
+  TraceEvent::ArgumentFilterPredicate argument_filter_predicate;
 
   if (!CheckGeneration(generation))
     return;
@@ -1800,20 +1829,25 @@ void TraceLog::FinishFlush(int generation) {
     flush_task_runner_ = NULL;
     flush_output_callback = flush_output_callback_;
     flush_output_callback_.Reset();
+
+    if (trace_options() & kInternalEnableArgumentFilter) {
+      CHECK(!argument_filter_predicate_.is_null());
+      argument_filter_predicate = argument_filter_predicate_;
+    }
   }
 
   if (use_worker_thread_ &&
       WorkerPool::PostTask(
-          FROM_HERE,
-          Bind(&TraceLog::ConvertTraceEventsToTraceFormat,
+          FROM_HERE, Bind(&TraceLog::ConvertTraceEventsToTraceFormat,
                           Passed(&previous_logged_events),
-               flush_output_callback),
+                          flush_output_callback, argument_filter_predicate),
           true)) {
     return;
   }
 
   ConvertTraceEventsToTraceFormat(previous_logged_events.Pass(),
-                                  flush_output_callback);
+                                  flush_output_callback,
+                                  argument_filter_predicate);
 }
 
 // Run in each thread holding a local event buffer.
@@ -1863,6 +1897,7 @@ void TraceLog::OnFlushTimeout(int generation) {
 void TraceLog::FlushButLeaveBufferIntact(
     const TraceLog::OutputCallback& flush_output_callback) {
   scoped_ptr<TraceBuffer> previous_logged_events;
+  TraceEvent::ArgumentFilterPredicate argument_filter_predicate;
   {
     AutoLock lock(lock_);
     AddMetadataEventsWhileLocked();
@@ -1872,10 +1907,16 @@ void TraceLog::FlushButLeaveBufferIntact(
                                   thread_shared_chunk_.Pass());
     }
     previous_logged_events = logged_events_->CloneForIteration().Pass();
+
+    if (trace_options() & kInternalEnableArgumentFilter) {
+      CHECK(!argument_filter_predicate_.is_null());
+      argument_filter_predicate = argument_filter_predicate_;
+    }
   }  // release lock
 
   ConvertTraceEventsToTraceFormat(previous_logged_events.Pass(),
-                                  flush_output_callback);
+                                  flush_output_callback,
+                                  argument_filter_predicate);
 }
 
 void TraceLog::UseNextTraceBuffer() {

base/trace_event/trace_event_impl.h

@@ -123,7 +123,11 @@ class BASE_EXPORT TraceEvent {
   void UpdateDuration(const TimeTicks& now, const TimeTicks& thread_now);
 
   // Serialize event data to JSON
-  void AppendAsJSON(std::string* out) const;
+  typedef base::Callback<bool(const char* category_group_name,
+                              const char* event_name)> ArgumentFilterPredicate;
+  void AppendAsJSON(
+      std::string* out,
+      const ArgumentFilterPredicate& argument_filter_predicate) const;
   void AppendPrettyPrinted(std::ostringstream* out) const;
 
   static void AppendValueAsJSON(unsigned char type,
@@ -384,12 +388,14 @@ struct BASE_EXPORT TraceOptions {
   TraceOptions()
       : record_mode(RECORD_UNTIL_FULL),
         enable_sampling(false),
-        enable_systrace(false) {}
+        enable_systrace(false),
+        enable_argument_filter(false) {}
 
   explicit TraceOptions(TraceRecordMode record_mode)
       : record_mode(record_mode),
         enable_sampling(false),
-        enable_systrace(false) {}
+        enable_systrace(false),
+        enable_argument_filter(false) {}
 
   // |options_string| is a comma-delimited list of trace options.
   // Possible options are: "record-until-full", "record-continuously",
@@ -420,6 +426,7 @@ struct BASE_EXPORT TraceOptions {
   TraceRecordMode record_mode;
   bool enable_sampling;
   bool enable_systrace;
+  bool enable_argument_filter;
 };
 
 struct BASE_EXPORT TraceLogStatus {
@@ -533,6 +540,8 @@ class BASE_EXPORT TraceLog {
   void SetEventCallbackEnabled(const CategoryFilter& category_filter,
                                EventCallback cb);
   void SetEventCallbackDisabled();
+  void SetArgumentFilterPredicate(
+      const TraceEvent::ArgumentFilterPredicate& argument_filter_predicate);
 
   // Flush all collected events to the given output callback. The callback will
   // be called one or more times either synchronously or asynchronously from
@@ -720,7 +729,8 @@ class BASE_EXPORT TraceLog {
   // Usually it runs on a different thread.
   static void ConvertTraceEventsToTraceFormat(
       scoped_ptr<TraceBuffer> logged_events,
-      const TraceLog::OutputCallback& flush_output_callback);
+      const TraceLog::OutputCallback& flush_output_callback,
+      const TraceEvent::ArgumentFilterPredicate& argument_filter_predicate);
   void FinishFlush(int generation);
   void OnFlushTimeout(int generation);
 
@@ -747,6 +757,7 @@ class BASE_EXPORT TraceLog {
   static const InternalTraceOptions kInternalEchoToConsole;
   static const InternalTraceOptions kInternalEnableSampling;
   static const InternalTraceOptions kInternalRecordAsMuchAsPossible;
+  static const InternalTraceOptions kInternalEnableArgumentFilter;
 
   // This lock protects TraceLog member accesses (except for members protected
   // by thread_info_lock_) from arbitrary threads.
@@ -811,6 +822,7 @@ class BASE_EXPORT TraceLog {
   // Set when asynchronous Flush is in progress.
   OutputCallback flush_output_callback_;
   scoped_refptr<SingleThreadTaskRunner> flush_task_runner_;
+  TraceEvent::ArgumentFilterPredicate argument_filter_predicate_;
   subtle::AtomicWord generation_;
   bool use_worker_thread_;
 

base/trace_event/trace_event_impl_constants.cc

@@ -23,6 +23,8 @@ const TraceLog::InternalTraceOptions
     TraceLog::kInternalEchoToConsole = 1 << 3;
 const TraceLog::InternalTraceOptions
     TraceLog::kInternalRecordAsMuchAsPossible = 1 << 4;
+const TraceLog::InternalTraceOptions
+    TraceLog::kInternalEnableArgumentFilter = 1 << 5;
 
 }  // namespace trace_event
 }  // namespace base

base/trace_event/trace_event_unittest.cc

@@ -2205,6 +2205,52 @@ TEST_F(TraceEventTestFixture, PrimitiveArgs) {
   EXPECT_EQ(1, int_value);
 }
 
+namespace {
+
+bool IsTraceEventArgsWhitelisted(const char* category_group_name,
+                                 const char* event_name) {
+  if (MatchPattern(category_group_name, "toplevel") &&
+      MatchPattern(event_name, "*")) {
+    return true;
+  }
+
+  return false;
+}
+
+}  // namespace
+
+TEST_F(TraceEventTestFixture, ArgsWhitelisting) {
+  TraceLog::GetInstance()->SetArgumentFilterPredicate(
+      base::Bind(&IsTraceEventArgsWhitelisted));
+
+  TraceOptions trace_options;
+  trace_options.enable_argument_filter = true;
+  TraceLog::GetInstance()->SetEnabled(CategoryFilter("*"),
+                                      TraceLog::RECORDING_MODE, trace_options);
+
+  TRACE_EVENT1("toplevel", "event1", "int_one", 1);
+  TRACE_EVENT1("whitewashed", "event2", "int_two", 1);
+  EndTraceAndFlush();
+
+  const DictionaryValue* args_dict = NULL;
+  DictionaryValue* dict = NULL;
+  int int_value;
+
+  dict = FindNamePhase("event1", "X");
+  ASSERT_TRUE(dict);
+  dict->GetDictionary("args", &args_dict);
+  ASSERT_TRUE(args_dict);
+  EXPECT_TRUE(args_dict->GetInteger("int_one", &int_value));
+  EXPECT_EQ(1, int_value);
+
+  dict = FindNamePhase("event2", "X");
+  ASSERT_TRUE(dict);
+  dict->GetDictionary("args", &args_dict);
+  ASSERT_TRUE(args_dict);
+  EXPECT_FALSE(args_dict->GetInteger("int_two", &int_value));
+  EXPECT_TRUE(args_dict->GetInteger("stripped", &int_value));
+}
+
 class TraceEventCallbackTest : public TraceEventTestFixture {
  public:
   void SetUp() override {

chrome/app/chrome_main_delegate.cc

@@ -16,6 +16,7 @@
 #include "base/process/memory.h"
 #include "base/process/process_handle.h"
 #include "base/strings/string_util.h"
+#include "base/trace_event/trace_event_impl.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/browser/defaults.h"
@@ -30,6 +31,7 @@
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/profiling.h"
 #include "chrome/common/switch_utils.h"
+#include "chrome/common/trace_event_args_whitelist.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/plugin/chrome_content_plugin_client.h"
 #include "chrome/renderer/chrome_content_renderer_client.h"
@@ -438,6 +440,9 @@ bool ChromeMainDelegate::BasicStartupComplete(int* exit_code) {
 
   Profiling::ProcessStarted();
 
+  base::trace_event::TraceLog::GetInstance()->SetArgumentFilterPredicate(
+      base::Bind(&IsTraceEventArgsWhitelisted));
+
 #if defined(OS_WIN)
   v8_breakpad_support::SetUp();
 #endif

chrome/chrome_common.gypi

@@ -110,6 +110,8 @@
       'common/spellcheck_result.h',
       'common/switch_utils.cc',
       'common/switch_utils.h',
+      'common/trace_event_args_whitelist.cc',
+      'common/trace_event_args_whitelist.h',
       'common/tts_messages.h',
       'common/tts_utterance_request.cc',
       'common/tts_utterance_request.h',

chrome/common/OWNERS

@@ -58,3 +58,8 @@ per-file autocomplete_match_type.*=mpearson@chromium.org
 per-file autocomplete_match_type.*=pkasting@chromium.org
 per-file autocomplete_match_type.*=sky@chromium.org
 per-file crash_keys*=rsesek@chromium.org
+
+# Tracing
+per-file trace_event_args_whitelist*=nduca@chromium.org
+per-file trace_event_args_whitelist*=dsinclair@chromium.org
+per-file trace_event_args_whitelist*=oysteine@chromium.org

chrome/common/trace_event_args_whitelist.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/trace_event_args_whitelist.h"
+
+#include "base/strings/string_tokenizer.h"
+#include "base/strings/string_util.h"
+
+namespace {
+
+const char* const kEventArgsWhitelist[][2] = {{"toplevel", "*"},
+                                              {"__metadata", "thread_name"},
+                                              {NULL, NULL}};
+
+}  // namespace
+
+bool IsTraceEventArgsWhitelisted(const char* category_group_name,
+                                 const char* event_name) {
+  base::CStringTokenizer category_group_tokens(
+      category_group_name, category_group_name + strlen(category_group_name),
+      ",");
+  while (category_group_tokens.GetNext()) {
+    const std::string& category_group_token = category_group_tokens.token();
+    for (int i = 0; kEventArgsWhitelist[i][0] != NULL; ++i) {
+      DCHECK(kEventArgsWhitelist[i][1]);
+
+      if (MatchPattern(category_group_token.c_str(),
+                       kEventArgsWhitelist[i][0]) &&
+          MatchPattern(event_name, kEventArgsWhitelist[i][1])) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}

chrome/common/trace_event_args_whitelist.h

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_COMMON_TRACE_EVENT_ARGS_WHITELIST_H_
+#define CHROME_COMMON_TRACE_EVENT_ARGS_WHITELIST_H_
+
+// Used to filter trace event arguments against a whitelist of events that
+// have been manually vetted to not include any PII.
+bool IsTraceEventArgsWhitelisted(const char* category_group_name,
+                                 const char* event_name);
+
+#endif  // CHROME_COMMON_TRACE_EVENT_ARGS_WHITELIST_H_