Add a mode flag to the tracing framework.

base/debug/trace_event_impl.cc

 // Copyright (c) 2012 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 "base/debug/trace_event_impl.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/debug/leak_annotations.h"
 #include "base/debug/trace_event.h"
 #include "base/format_macros.h"
 #include "base/lazy_instance.h"
 #include "base/memory/singleton.h"
 #include "base/process_util.h"
 #include "base/stl_util.h"
+#include "base/string_split.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/strings/string_tokenizer.h"
 #include "base/sys_info.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_id_name_manager.h"
 #include "base/threading/thread_local.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
@@ skipping 38 matching lines @@
 const int g_category_categories_exhausted = 1;
 const int g_category_metadata = 2;
 int g_category_index = 3; // skip initial 3 categories
 
 // The name of the current thread. This is used to decide if the current
 // thread name has changed. We combine all the seen thread names into the
 // output name for the thread.
 LazyInstance<ThreadLocalPointer<const char> >::Leaky
     g_current_thread_name = LAZY_INSTANCE_INITIALIZER;
 
+const char kRecordUntilFull[] = "record-until-full";
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // TraceEvent
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 namespace {
 
@@ skipping 256 matching lines @@
 void TraceLog::NotificationHelper::SendNotificationIfAny() {
   if (notification_)
     callback_copy_.Run(notification_);
 }
 
 // static
 TraceLog* TraceLog::GetInstance() {
   return Singleton<TraceLog, StaticMemorySingletonTraits<TraceLog> >::get();
 }
 
+// static
+// Note, if you add more options here you also need to update:
+// content/browser/devtools/devtools_tracing_handler:TraceOptionsFromString
+int TraceLog::TraceOptionsFromString(const std::string& options) {
+  std::vector<std::string> split;
+  std::vector<std::string>::iterator iter;

[jar (doing other things), 2013/02/22 19:31:28]

nit: move into line 363, so it is next to first use.

[dsinclair, 2013/02/22 20:01:42]

Done.

+  int ret = 0;
+
+  base::SplitString(options, ',', &split);

[jar (doing other things), 2013/02/22 19:31:28]

nit: declare closer to first use.  Probably push this line up to between line
358 and 359.

[dsinclair, 2013/02/22 20:01:42]

Done.

+  for (iter = split.begin(); iter != split.end(); ++iter) {
+    if (*iter == kRecordUntilFull) {
+      ret |= RECORD_UNTIL_FULL;
+    }

[jar (doing other things), 2013/02/22 19:31:28]

nit: Should we have a DCHECK for other spellings here?

[dsinclair, 2013/02/22 20:01:42]

Added a else NOTREACHED();

+  }
+  if (ret == 0)
+    ret = RECORD_UNTIL_FULL;

[jar (doing other things), 2013/02/22 19:31:28]

nit: Add comment: // Default when no options are specified.

The code is pretty simple, but yet confusing, since with only one option, this
assignment always happens one way or the other (line 365):-/.  It may help
clarify when you add a DCHECK for other spellings, but really this is forward
looking code, anticipating that there will be other options scanned for.  

How about adding a comment between line 367 and 368:
// Anticipating a collection of option, we provide a baseline default.

The weird thing is that you can presumably have a bunch of OR'ed options.  The
fact that they are ORed suggests you can have one, and not another.  As a
result, it is surprising that you have a default to one, when there are no
options specified.  I would (for example) have expected that if some subset of
options were not selected, *then* you would OR in this default.

Can you help clarify what the intent is, so that folks will understand how to
modify this when other options are added?

[dsinclair, 2013/02/22 20:01:42]

Skipping the comment as there is already a patch waiting to build on this when
it gets committed.

I've updated the ret == 0 check to be a !(ret & RECORD_UNTIL_FULL) which will
get expended when we have other recording options that could get set.

+
+  return ret;
+}
+
 TraceLog::TraceLog()
     : enable_count_(0),
       dispatching_to_observer_list_(false),
-      watch_category_(NULL) {
+      watch_category_(NULL),
+      trace_options_(RECORD_UNTIL_FULL) {
   // Trace is enabled or disabled on one thread while other threads are
   // accessing the enabled flag. We don't care whether edge-case events are
   // traced or not, so we allow races on the enabled flag to keep the trace
   // macros fast.
   // TODO(jbates): ANNOTATE_BENIGN_RACE_SIZED crashes windows TSAN bots:
   // ANNOTATE_BENIGN_RACE_SIZED(g_category_enabled, sizeof(g_category_enabled),
   //                            "trace_event category enabled");
   for (int i = 0; i < TRACE_EVENT_MAX_CATEGORIES; ++i) {
     ANNOTATE_BENIGN_RACE(&g_category_enabled[i],
                          "trace_event category enabled");
@@ skipping 106 matching lines @@
   return category_enabled;
 }
 
 void TraceLog::GetKnownCategories(std::vector<std::string>* categories) {
   AutoLock lock(lock_);
   for (int i = 0; i < g_category_index; i++)
     categories->push_back(g_categories[i]);
 }
 
 void TraceLog::SetEnabled(const std::vector<std::string>& included_categories,
-                          const std::vector<std::string>& excluded_categories) {
+                          const std::vector<std::string>& excluded_categories,
+                          int options) {
   AutoLock lock(lock_);
 
   if (enable_count_++ > 0) {
+    if (options != trace_options_) {
+      DLOG(ERROR) << "Attemting to re-enable tracing with a different "
+                  << "set of options.";
+    }
+
     // 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);
     } 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);
     }
     return;
   }
+  trace_options_ = options;
 
   if (dispatching_to_observer_list_) {
     DLOG(ERROR) <<
         "Cannot manipulate TraceLog::Enabled state from an observer.";
     return;
   }
 
   dispatching_to_observer_list_ = true;
   FOR_EACH_OBSERVER(EnabledStateChangedObserver, enabled_state_observer_list_,
                     OnTraceLogWillEnable());
   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) {
+void TraceLog::SetEnabled(const std::string& categories, int options) {
   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);
+  SetEnabled(included, excluded, options);
 }
 
 void TraceLog::GetEnabledTraceCategories(
     std::vector<std::string>* included_out,
     std::vector<std::string>* excluded_out) {
   AutoLock lock(lock_);
   if (enable_count_) {
     *included_out = included_categories_;
     *excluded_out = excluded_categories_;
   }
@@ skipping 18 matching lines @@
 
   included_categories_.clear();
   excluded_categories_.clear();
   watch_category_ = NULL;
   watch_event_name_ = "";
   for (int i = 0; i < g_category_index; i++)
     g_category_enabled[i] = 0;
   AddThreadNameMetadataEvents();
 }
 
-void TraceLog::SetEnabled(bool enabled) {
+void TraceLog::SetEnabled(bool enabled, int options) {
   if (enabled)
-    SetEnabled(std::vector<std::string>(), std::vector<std::string>());
+    SetEnabled(std::vector<std::string>(), std::vector<std::string>(), options);
   else
     SetDisabled();
 }
 
 void TraceLog::AddEnabledStateObserver(EnabledStateChangedObserver* listener) {
   enabled_state_observer_list_.AddObserver(listener);
 }
 
 void TraceLog::RemoveEnabledStateObserver(
     EnabledStateChangedObserver* listener) {
@@ skipping 269 matching lines @@
         0,                       // num_args
         NULL,                    // arg_names
         NULL,                    // arg_types
         NULL,                    // arg_values
         TRACE_EVENT_FLAG_NONE);  // flags
   }
 }
 
 }  // namespace trace_event_internal