Category group support/Renamings.

base/debug/trace_event_impl.h

 // 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.
 
 
 #ifndef BASE_DEBUG_TRACE_EVENT_IMPL_H_
 #define BASE_DEBUG_TRACE_EVENT_IMPL_H_
 
 #include "build/build_config.h"
 
@@ skipping 47 matching lines @@
     long long as_int;
     double as_double;
     const void* as_pointer;
     const char* as_string;
   };
 
   TraceEvent();
   TraceEvent(int thread_id,
              TimeTicks timestamp,
              char phase,
-             const unsigned char* category_enabled,
+             const unsigned char* category_group_enabled,
              const char* name,
              unsigned long long id,
              int num_args,
              const char** arg_names,
              const unsigned char* arg_types,
              const unsigned long long* arg_values,
              unsigned char flags);
   ~TraceEvent();
 
   // Serialize event data to JSON
   static void AppendEventsAsJSON(const std::vector<TraceEvent>& events,
                                  size_t start,
                                  size_t count,
                                  std::string* out);
   void AppendAsJSON(std::string* out) const;
 
   static void AppendValueAsJSON(unsigned char type,
                                 TraceValue value,
                                 std::string* out);
 
   TimeTicks timestamp() const { return timestamp_; }
 
   // Exposed for unittesting:
 
   const base::RefCountedString* parameter_copy_storage() const {
     return parameter_copy_storage_.get();
   }
 
-  const unsigned char* category_enabled() const { return category_enabled_; }
+  const unsigned char* category_group_enabled() const {
+    return category_group_enabled_;
+  }
+
   const char* name() const { return name_; }
 
  private:
   // Note: these are ordered by size (largest first) for optimal packing.
   TimeTicks timestamp_;
   // id_ can be used to store phase-specific data.
   unsigned long long id_;
   TraceValue arg_values_[kTraceMaxNumArgs];
   const char* arg_names_[kTraceMaxNumArgs];
-  const unsigned char* category_enabled_;
+  const unsigned char* category_group_enabled_;
   const char* name_;
   scoped_refptr<base::RefCountedString> parameter_copy_storage_;
   int thread_id_;
   char phase_;
   unsigned char flags_;
   unsigned char arg_types_[kTraceMaxNumArgs];
 };
 
 
 // TraceResultBuffer collects and converts trace fragments returned by TraceLog
@@ skipping 32 matching lines @@
 
   // When all fragments have been added, call Finish to complete the JSON
   // formatted output.
   void Finish();
 
  private:
   OutputCallback output_callback_;
   bool append_comma_;
 };
 
+class BASE_EXPORT CategoryFilter {
+  // The default category filter, used when none is provided.
+  // Allows all categories through, except if they end in the suffix 'Debug' or
+  // 'Test'.
+  static const char* kDefaultCategoryFilterString;
+
+  public:

[dsinclair, 2013/02/22 21:13:44]

nit: public should be indented 1 instead of 2.

[rterrazas, 2013/02/25 05:55:02]

Done.

+  // Constructs the default category filter. This is equivalent to
+  // CategoryFilter(CategoryFilter::kDefaultCategoryFilterString);
+  CategoryFilter();
+
+  // |filter_string| is a comma-delimited list of category wildcards.
+  // A category can have an optional '-' prefix to make it an excluded category.
+  // All the same rules apply above, so for example, having both included and
+  // excluded categories in the same list would not be supported.
+  //
+  // Example: SetEnabled("test_MyTest*");
+  // Example: SetEnabled("test_MyTest*,test_OtherStuff");
+  // Example: SetEnabled("-excluded_category1,-excluded_category2");

[nduca, 2013/02/21 06:02:36]

can you add an example of how to disable everything but 1 thing? E.g. -*,foo
 would catch ONLY the foo category, right?

[dsinclair, 2013/02/22 21:13:44]

Should these be s/SetEnabled/CategoryFilter/?

[rterrazas, 2013/02/25 05:55:02]

Done.

[rterrazas, 2013/02/25 05:55:02]

Done.

+  CategoryFilter(const std::string& filter_string);
+
+  // Writes the string representation of the CategoryFilter. This is a comma
+  // separated string, similar in nature to the one used to determine
+  // enabled/disabled category patterns, except here there is an arbitrary
+  // order, included categories go first, then excluded categories. Excluded
+  // categories are distinguished from included categories by the prefix '-'.
+  void ToString(std::string* filter_string) const;

[dsinclair, 2013/02/22 21:13:44]

Why write to an input parameter instead of just returning?

[rterrazas, 2013/02/25 05:55:02]

Did that to be consistent with the approach used when building the JSON string
that's read by TEV.

I believe we use that approach for that piece of the code to avoid copying that
string, which can get really large, however in this case, I believe what you're
suggesting looks prettier and the string that we'll be writing is smallish.

Done.

+
+  // Determines whether category group would be enabled or
+  // disabled by this filter.
+  bool IsCategoryGroupEnabled(const char* category_group) const;
+
+  // Merges nested_filter with the current CategoryFilter
+  void Merge(const CategoryFilter& nested_filter);
+
+  // Determines whether or not we have included category patterns.
+  bool HasIncludedCategories() const;

[nduca, 2013/02/21 06:02:36]

Not sure I get what this function does by its name....  I think the word
"included" is a bit too implementation specific.

[rterrazas, 2013/02/25 05:55:02]

Done.

+
+  // Clears both included/excluded pattern lists.

[nduca, 2013/02/21 06:02:36]

So then what does it default to? kDefaultCategoryFilterString, or ""? We should
be very very explicit.

[rterrazas, 2013/02/25 05:55:02]

Done.

+  void Clear();
+
+private:

[dsinclair, 2013/02/22 21:13:44]

private should be indented 1.

[rterrazas, 2013/02/25 05:55:02]

Done.

+  void Initialize(const std::string& filter_string);
+  void WriteString(std::string* out, bool included) const;
+
+  std::vector<std::string> included_;
+  std::vector<std::string> excluded_;
+
+};

[dsinclair, 2013/02/22 21:13:44]

Does this need a DISALLOW_COPY_AND_ASSIGN(CategoryFilter)?

[rterrazas, 2013/02/25 05:55:02]

Nope, we can't do that as we do assign to copy the object through stackframes.

[rterrazas, 2013/02/25 06:05:44]

However, I did overload = and provided an explicit copy constructor, as per the
guidelines.

 
 class BASE_EXPORT TraceLog {
  public:
   // Notification is a mask of one or more of the following events.
   enum Notification {
     // The trace buffer does not flush dynamically, so when it fills up,
     // subsequent trace events will be dropped. This callback is generated when
     // the trace buffer is full. The callback must be thread safe.
     TRACE_BUFFER_FULL = 1 << 0,
     // A subscribed trace-event occurred.
     EVENT_WATCH_NOTIFICATION = 1 << 1
   };
 
   static TraceLog* GetInstance();
 
-  // Get set of known categories. This can change as new code paths are reached.
-  // The known categories are inserted into |categories|.
-  void GetKnownCategories(std::vector<std::string>* categories);
+  // Get set of known category groups. This can change as new code paths are
+  // reached. The known category groups are inserted into |category_groups|.
+  void GetKnownCategoryGroups(std::vector<std::string>* category_groups);
 
-  // Enable tracing for provided list of categories. If tracing is already
-  // enabled, this method does nothing -- changing categories during trace is
-  // not supported.
-  // If both included_categories and excluded_categories are empty,
-  //   all categories are traced.
-  // Else if included_categories is non-empty, only those are traced.
-  // Else if excluded_categories is non-empty, everything but those are traced.
-  // Wildcards * and ? are supported (see MatchPattern in string_util.h).
-  void SetEnabled(const std::vector<std::string>& included_categories,
-                  const std::vector<std::string>& excluded_categories);
-
-  // |categories| is a comma-delimited list of category wildcards.
-  // A category can have an optional '-' prefix to make it an excluded category.
-  // All the same rules apply above, so for example, having both included and
-  // excluded categories in the same list would not be supported.
-  //
-  // Example: SetEnabled("test_MyTest*");
-  // Example: SetEnabled("test_MyTest*,test_OtherStuff");
-  // Example: SetEnabled("-excluded_category1,-excluded_category2");
-  void SetEnabled(const std::string& categories);
-
-  // Retieves the categories set via a prior call to SetEnabled(). Only
-  // meaningful if |IsEnabled()| is true.
-  void GetEnabledTraceCategories(std::vector<std::string>* included_out,
-                                 std::vector<std::string>* excluded_out);
+  // Enables tracing. See CategoryFilter comments for details
+  // on how to control what categories will be traced.
+  void SetEnabled(const CategoryFilter& category_filter);
 
   // Disable tracing for all categories.
   void SetDisabled();
+
+  // Retieves the current filter used to enable categories. Only
+  // meaningful if |IsEnabled()| is true.

[nduca, 2013/02/21 06:02:36]

Dont like that this returns a non-const reference. It seems very dangerous to
allow this to be mutated on the fly?

[rterrazas, 2013/02/25 05:55:02]

Right, that wasn't by design, that was a mistake I made. Thanks for pointing it
out!.
Done.

+  CategoryFilter& GetCurrentCategoryFilter();
+
   // Helper method to enable/disable tracing for all categories.
-  void SetEnabled(bool enabled);
   bool IsEnabled() { return !!enable_count_; }
 
 #if defined(OS_ANDROID)
   void StartATrace();
   void StopATrace();
 #endif
 
   // Enabled state listeners give a callback when tracing is enabled or
   // disabled. This can be used to tie into other library's tracing systems
   // on-demand.
@@ skipping 25 matching lines @@
 
   // Flush all collected events to the given output callback. The callback will
   // be called one or more times with IPC-bite-size chunks. The string format is
   // undefined. Use TraceResultBuffer to convert one or more trace strings to
   // JSON.
   typedef base::Callback<void(const scoped_refptr<base::RefCountedString>&)>
       OutputCallback;
   void Flush(const OutputCallback& cb);
 
   // Called by TRACE_EVENT* macros, don't call this directly.
-  static const unsigned char* GetCategoryEnabled(const char* name);
-  static const char* GetCategoryName(const unsigned char* category_enabled);
+  // The name parameter is a category group for example:
+  // TRACE_EVENT0("renderer,webkit", "WebViewImpl::HandleInputEvent")
+  static const unsigned char* GetCategoryGroupEnabled(const char* name);
+  static const char* GetCategoryGroupName(
+                                   const unsigned char* category_group_enabled);

[dsinclair, 2013/02/22 21:13:44]

nit: This should be indented 4 from static on the line above.

[rterrazas, 2013/02/25 05:55:02]

Done.

 
   // Called by TRACE_EVENT* macros, don't call this directly.
   // If |copy| is set, |name|, |arg_name1| and |arg_name2| will be deep copied
   // into the event; see "Memory scoping note" and TRACE_EVENT_COPY_XXX above.
   void AddTraceEvent(char phase,
-                     const unsigned char* category_enabled,
-                     const char* name,
+                    const unsigned char* category_group_enabled,
+                    const char* category_group,

[dsinclair, 2013/02/22 21:13:44]

nit: indenting.

[rterrazas, 2013/02/25 05:55:02]

Done.

                      unsigned long long id,
                      int num_args,
                      const char** arg_names,
                      const unsigned char* arg_types,
                      const unsigned long long* arg_values,
                      unsigned char flags);
   void AddTraceEventWithThreadIdAndTimestamp(
       char phase,
       const unsigned char* category_enabled,
       const char* name,
       unsigned long long id,
       int thread_id,
       const TimeTicks& timestamp,
       int num_args,
       const char** arg_names,
       const unsigned char* arg_types,
       const unsigned long long* arg_values,
       unsigned char flags);
   static void AddTraceEventEtw(char phase,
-                               const char* name,
+                               const char* category_group,
                                const void* id,
                                const char* extra);
   static void AddTraceEventEtw(char phase,
-                               const char* name,
+                               const char* category_group,
                                const void* id,
                                const std::string& extra);
 
   // For every matching event, a notification will be fired. NOTE: the
   // notification will fire for each matching event that has already occurred
   // since tracing was started (including before tracing if the process was
   // started with tracing turned on).
   void SetWatchEvent(const std::string& category_name,
                      const std::string& event_name);
   // Cancel the watch event. If tracing is enabled, this may race with the
@@ skipping 21 matching lines @@
 
   // Allow setting an offset between the current TimeTicks time and the time
   // that should be reported.
   void SetTimeOffset(TimeDelta offset);
 
  private:
   // This allows constructor and destructor to be private and usable only
   // by the Singleton class.
   friend struct StaticMemorySingletonTraits<TraceLog>;
 
-  // The pointer returned from GetCategoryEnabledInternal() points to a value
-  // with zero or more of the following bits. Used in this class only.
+  // Enable/disable each category group based on the given |category_filter|.
+  // If the category group contains a category that matches an included category
+  // pattern, that category group will be enabled.
+  static void EnableIncludedCategoryGroups(
+                                         const CategoryFilter& category_filter);

[nduca, 2013/02/21 06:02:36]

indent is funky here. Indent to 4 lines when you wrap, if indenting to the
column after the open ( doesn't fit.

[rterrazas, 2013/02/25 05:55:02]

Done.

+  static void EnableIncludedCategoryGroup(int category_index,
+                                         const CategoryFilter& category_filter);
+
+  // The pointer returned from GetCategoryGroupEnabledInternal() points to a
+  // value with zero or more of the following bits. Used in this class only.
   // The TRACE_EVENT macros should only use the value as a bool.
   enum CategoryEnabledFlags {
     // Normal enabled flag for categories enabled with Enable().
     CATEGORY_ENABLED = 1 << 0,
     // On Android if ATrace is enabled, all categories will have this bit.
     // Not used on other platforms.
     ATRACE_ENABLED = 1 << 1
   };
 
   // Helper class for managing notification_thread_count_ and running
@@ skipping 14 matching lines @@
     inline void SendNotificationIfAny();
 
    private:
     TraceLog* trace_log_;
     NotificationCallback callback_copy_;
     int notification_;
   };
 
   TraceLog();
   ~TraceLog();
-  const unsigned char* GetCategoryEnabledInternal(const char* name);
+  const unsigned char* GetCategoryGroupEnabledInternal(const char* name);
   void AddThreadNameMetadataEvents();
 
 #if defined(OS_ANDROID)
   void SendToATrace(char phase,
-                    const char* category,
+                    const char* category_group,
                     const char* name,
                     int num_args,
                     const char** arg_names,
                     const unsigned char* arg_types,
                     const unsigned long long* arg_values);
-  static void ApplyATraceEnabledFlag(unsigned char* category_enabled);
+  static void ApplyATraceEnabledFlag(unsigned char* category_group_enabled);
 #endif
 
   // TODO(nduca): switch to per-thread trace buffers to reduce thread
   // synchronization.
   // This lock protects TraceLog member accesses from arbitrary threads.
   Lock lock_;
   int enable_count_;
   NotificationCallback notification_callback_;
   std::vector<TraceEvent> logged_events_;
-  std::vector<std::string> included_categories_;
-  std::vector<std::string> excluded_categories_;
+  CategoryFilter category_filter_;
   bool dispatching_to_observer_list_;
   ObserverList<EnabledStateChangedObserver> enabled_state_observer_list_;
 
   base::hash_map<int, std::string> thread_names_;
 
   // XORed with TraceID to make it unlikely to collide with other processes.
   unsigned long long process_id_hash_;
 
   int process_id_;
 
   TimeDelta time_offset_;
 
   // Allow tests to wake up when certain events occur.
   const unsigned char* watch_category_;
   std::string watch_event_name_;
 
   DISALLOW_COPY_AND_ASSIGN(TraceLog);
 };
 
 }  // namespace debug
 }  // namespace base
 
 #endif  // BASE_DEBUG_TRACE_EVENT_IMPL_H_