[ChromeDriver] Implementing PerfLoggingPrefs for perf log

chrome/test/chromedriver/capabilities.h

Index: chrome/test/chromedriver/capabilities.h
diff --git a/chrome/test/chromedriver/capabilities.h b/chrome/test/chromedriver/capabilities.h
index edef9baeac599ce7285771b811219153ffc632c6..6ba7d40715b7c45229cf6ca3131bd1c32a0b217e 100644
--- a/chrome/test/chromedriver/capabilities.h
+++ b/chrome/test/chromedriver/capabilities.h
@@ -60,6 +60,30 @@ class Switches {
 
 typedef std::map<std::string, Log::Level> LoggingPrefs;
 
+struct PerfLoggingPrefs {
+  PerfLoggingPrefs();
+  ~PerfLoggingPrefs();

[samuong, 2014/08/04 06:08:22]

This probably doesn't matter since no one will ever subclass this, but should we
just make the destructor virtual anyway?

[johnmoore, 2014/08/04 17:37:57]

For consistency, I think we shouldn't, since neither the Capabilities nor
Switches classes have virtual destructors, since like you said they have no
reason to be subclassed.

+
+  // We must distinguish between a log domain being set by default and being
+  // explicitly set. Otherwise, |PerformanceLogger| could only handle 3 of 4
+  // possible combinations (tracing enabled/disabled + Timeline on/off).
+  enum InspectorDomainStatus {
+    kDefaultEnabled,  // enabled by default
+    kDefaultDisabled,  // disabled by default
+    kExplicitlyEnabled,  // explicitly enabled
+    kExplicitlyDisabled  // explicitly disabled

[samuong, 2014/08/04 06:08:22]

I think people will be able to figure out what the enum names mean even without
these four comments :)

[johnmoore, 2014/08/04 17:37:57]

Done.

+  };
+
+  // True if |domain_status| is kDefaultEnabled or kExplicitlyEnabled.
+  static bool IsEnabled(const InspectorDomainStatus& domain_status);

[samuong, 2014/08/04 06:08:22]

structs aren't usually supposed to have any logic in them, and since this is
static anyway you could easily just pull this out into a non-member function

[johnmoore, 2014/08/04 17:37:58]

Done.

+
+  InspectorDomainStatus network;
+  InspectorDomainStatus page;
+  InspectorDomainStatus timeline;
+
+  std::string trace_categories;  // Non-empty string enables tracing.
+};
+
 struct Capabilities {
   Capabilities();
   ~Capabilities();
@@ -114,6 +138,8 @@ struct Capabilities {
   // If set, enable minidump for chrome crashes and save to this directory.
   std::string minidump_path;
 
+  PerfLoggingPrefs perf_logging_prefs;
+
   scoped_ptr<base::DictionaryValue> prefs;
 
   Switches switches;