Refactor WebSocket Live experiment code.

Move saving result in WebSocketExperimentTask.
Add net::WebSocket::ProtocolVersion in WebSocketExperimentTask::Config.
Change Histgram managements.

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/1539007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43314 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 202 insertions, 150 deletions

diff --git a/chrome/browser/net/websocket_experiment/websocket_experiment_runner.cc b/chrome/browser/net/websocket_experiment/websocket_experiment_runner.cc
index 845d448..c632e77 100644
--- a/chrome/browser/net/websocket_experiment/websocket_experiment_runner.cc
+++ b/chrome/browser/net/websocket_experiment/websocket_experiment_runner.cc
@@ -6,86 +6,17 @@
 
 #include "base/compiler_specific.h"
 #include "base/field_trial.h"
-#include "base/histogram.h"
 #include "base/message_loop.h"
 #include "base/task.h"
 #include "chrome/browser/chrome_thread.h"
 #include "net/base/net_errors.h"
+#include "net/websockets/websocket.h"
 
 namespace chrome_browser_net_websocket_experiment {
 
 static const char *kExperimentHost = "websocket-experiment.chromium.org";
 static const int kAlternativePort = 61985;
 
-static const int kUrlFetchDeadlineSec = 10;
-static const int kWebSocketConnectDeadlineSec = 10;
-static const int kWebSocketEchoDeadlineSec = 5;
-static const int kWebSocketIdleSec = 1;
-static const int kWebSocketPushDeadlineSec = 1;
-static const int kWebSocketByeDeadlineSec = 10;
-static const int kWebSocketCloseDeadlineSec = 5;
-static const int kWebSocketTimeSec = 10;
-static const int kTimeBucketCount = 50;
-
-// TODO(ukai): Use new thread-safe-reference-counted Histograms.
-#define UPDATE_HISTOGRAM_ENUMS(name, sample, boundary_value) do {       \
-    switch (task_state_) {                                              \
-      case STATE_RUN_WS:                                                \
-        {                                                               \
-          UMA_HISTOGRAM_ENUMERATION(                                    \
-              "WebSocketExperiment.Basic." name,                        \
-               sample, boundary_value);                                 \
-        }                                                               \
-        break;                                                          \
-      case STATE_RUN_WSS:                                               \
-        {                                                               \
-          UMA_HISTOGRAM_ENUMERATION(                                    \
-              "WebSocketExperiment.Secure." name,                       \
-               sample, boundary_value);                                 \
-        }                                                               \
-        break;                                                          \
-      case STATE_RUN_WS_NODEFAULT_PORT:                                 \
-        {                                                               \
-          UMA_HISTOGRAM_ENUMERATION(                                    \
-               "WebSocketExperiment.NoDefaultPort." name,               \
-               sample, boundary_value);                                 \
-        }                                                               \
-        break;                                                          \
-      default:                                                          \
-        NOTREACHED();                                                   \
-        break;                                                          \
-    }                                                                   \
-  } while (0)
-
-#define UPDATE_HISTOGRAM_TIMES(name, sample, min, max, bucket_count) do { \
-    switch (task_state_) {                                              \
-      case STATE_RUN_WS:                                                \
-        {                                                               \
-          UMA_HISTOGRAM_CUSTOM_TIMES(                                   \
-              "WebSocketExperiment.Basic." name,                        \
-              sample, min, max, bucket_count);                          \
-        }                                                               \
-        break;                                                          \
-      case STATE_RUN_WSS:                                               \
-        {                                                               \
-          UMA_HISTOGRAM_CUSTOM_TIMES(                                   \
-              "WebSocketExperiment.Secure." name,                       \
-              sample, min, max, bucket_count);                          \
-        }                                                               \
-        break;                                                          \
-      case STATE_RUN_WS_NODEFAULT_PORT:                                 \
-        {                                                               \
-          UMA_HISTOGRAM_CUSTOM_TIMES(                                   \
-              "WebSocketExperiment.NoDefaultPort." name,                \
-              sample, min, max, bucket_count);                          \
-        }                                                               \
-        break;                                                          \
-      default:                                                          \
-        NOTREACHED();                                                   \
-        break;                                                          \
-    }                                                                   \
-  } while (0);
-
 // Hold reference while experiment is running.
 static scoped_refptr<WebSocketExperimentRunner> runner;
 
@@ -115,11 +46,13 @@ WebSocketExperimentRunner::WebSocketExperimentRunner()
       task_state_(STATE_NONE),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           task_callback_(this, &WebSocketExperimentRunner::OnTaskCompleted)) {
+  WebSocketExperimentTask::InitHistogram();
   InitConfig();
 }
 
 WebSocketExperimentRunner::~WebSocketExperimentRunner() {
   DCHECK(!task_.get());
+  WebSocketExperimentTask::ReleaseHistogram();
 }
 
 void WebSocketExperimentRunner::Run() {
@@ -145,23 +78,7 @@ void WebSocketExperimentRunner::InitConfig() {
   config_.next_delay_ms = 12 * 60 * 60 * 1000;  // 12 hours
 
   WebSocketExperimentTask::Config task_config;
-  task_config.ws_protocol = "google-websocket-liveexperiment";
-  task_config.ws_origin = "http://dev.chromium.org/";
-  task_config.url_fetch_deadline_ms = kUrlFetchDeadlineSec * 1000;
-  task_config.websocket_onopen_deadline_ms =
-      kWebSocketConnectDeadlineSec * 1000;
-  task_config.websocket_hello_message = "Hello";
-  task_config.websocket_hello_echoback_deadline_ms =
-      kWebSocketEchoDeadlineSec * 1000;
-  // Note: wait 1.5 sec in websocket_experiment_def.txt
-  task_config.websocket_idle_ms = kWebSocketIdleSec * 1000;
-  task_config.websocket_receive_push_message_deadline_ms =
-      kWebSocketPushDeadlineSec * 1000;
-  task_config.websocket_bye_message = "Bye";
-  task_config.websocket_bye_deadline_ms =
-      kWebSocketByeDeadlineSec * 1000;
-  task_config.websocket_close_deadline_ms =
-      kWebSocketCloseDeadlineSec * 1000;
+  task_config.protocol_version = net::WebSocket::DRAFT75;
 
   config_.ws_config = task_config;
   config_.ws_config.url =
@@ -248,63 +165,10 @@ void WebSocketExperimentRunner::OnTaskCompleted(int result) {
     Release();
     return;
   }
-  UpdateTaskResultHistogram(task_.get());
+  task_->SaveResult();
   task_.reset();
 
   DoLoop();
 }
 
-void WebSocketExperimentRunner::UpdateTaskResultHistogram(
-    const WebSocketExperimentTask* task) {
-  DCHECK(task);
-  const WebSocketExperimentTask::Result& task_result = task->result();
-
-  UPDATE_HISTOGRAM_ENUMS("LastState", task_result.last_state,
-                         WebSocketExperimentTask::NUM_STATES);
-
-  UPDATE_HISTOGRAM_TIMES("UrlFetch", task_result.url_fetch,
-                         base::TimeDelta::FromMilliseconds(1),
-                         base::TimeDelta::FromSeconds(kUrlFetchDeadlineSec),
-                         kTimeBucketCount);
-
-  if (task_result.last_state <
-      WebSocketExperimentTask::STATE_WEBSOCKET_CONNECT_COMPLETE)
-    return;
-
-  UPDATE_HISTOGRAM_TIMES("WebSocketConnect", task_result.websocket_connect,
-                         base::TimeDelta::FromMilliseconds(1),
-                         base::TimeDelta::FromSeconds(
-                             kWebSocketConnectDeadlineSec),
-                         kTimeBucketCount);
-
-  if (task_result.last_state <
-      WebSocketExperimentTask::STATE_WEBSOCKET_RECV_HELLO)
-    return;
-
-  UPDATE_HISTOGRAM_TIMES("WebSocketEcho", task_result.websocket_echo,
-                         base::TimeDelta::FromMilliseconds(1),
-                         base::TimeDelta::FromSeconds(
-                             kWebSocketEchoDeadlineSec),
-                         kTimeBucketCount);
-
-  if (task_result.last_state <
-      WebSocketExperimentTask::STATE_WEBSOCKET_KEEP_IDLE)
-    return;
-
-  UPDATE_HISTOGRAM_TIMES("WebSocketIdle", task_result.websocket_idle,
-                         base::TimeDelta::FromMilliseconds(1),
-                         base::TimeDelta::FromSeconds(
-                             kWebSocketIdleSec + kWebSocketPushDeadlineSec),
-                         kTimeBucketCount);
-
-  if (task_result.last_state <
-      WebSocketExperimentTask::STATE_WEBSOCKET_CLOSE_COMPLETE)
-    return;
-
-  UPDATE_HISTOGRAM_TIMES("WebSocketTotal", task_result.websocket_total,
-                         base::TimeDelta::FromMilliseconds(1),
-                         base::TimeDelta::FromSeconds(kWebSocketTimeSec),
-                         kTimeBucketCount);
-}
-
 }  // namespace chrome_browser_net_websocket_experiment
diff --git a/chrome/browser/net/websocket_experiment/websocket_experiment_task.cc b/chrome/browser/net/websocket_experiment/websocket_experiment_task.cc
index 1754b00..1dc468a 100644
--- a/chrome/browser/net/websocket_experiment/websocket_experiment_task.cc
+++ b/chrome/browser/net/websocket_experiment/websocket_experiment_task.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/net/websocket_experiment/websocket_experiment_task.h"
 
+#include "base/hash_tables.h"
+#include "base/histogram.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/net/url_request_context_getter.h"
 #include "chrome/browser/profile.h"
@@ -46,11 +48,55 @@ net::WebSocket* WebSocketExperimentTask::Context::CreateWebSocket(
                                   config.ws_protocol,
                                   config.ws_origin,
                                   config.ws_location,
-                                  net::WebSocket::DRAFT75,
+                                  config.protocol_version,
                                   getter->GetURLRequestContext()));
   return new net::WebSocket(request, delegate);
 }
 
+// Expects URL Fetch and WebSocket connection handshake will finish in
+// a few seconds.
+static const int kUrlFetchDeadlineSec = 10;
+static const int kWebSocketConnectDeadlineSec = 10;
+// Expects WebSocket live experiment server echoes message back within a few
+// seconds.
+static const int kWebSocketEchoDeadlineSec = 5;
+// WebSocket live experiment server keeps idle for 1.5 seconds and sends
+// a message.  So, expects idle for at least 1 second and expects message
+// arrives within 1 second after that.
+static const int kWebSocketIdleSec = 1;
+static const int kWebSocketPushDeadlineSec = 1;
+// WebSocket live experiment server sends "bye" message soon.
+static const int kWebSocketByeDeadlineSec = 10;
+// WebSocket live experiment server closes after it receives "bye" message.
+static const int kWebSocketCloseDeadlineSec = 5;
+
+// All of above are expected within a few seconds.  We'd like to see time
+// distribution between 0 to 10 seconds.
+static const int kWebSocketTimeSec = 10;
+static const int kTimeBucketCount = 50;
+
+// Holds Histogram objects during experiments run.
+static base::hash_map<std::string, Histogram*>* g_histogram_table;
+
+WebSocketExperimentTask::Config::Config()
+    : ws_protocol("google-websocket-liveexperiment"),
+      ws_origin("http://dev.chromium.org/"),
+      protocol_version(net::WebSocket::DEFAULT_VERSION),
+      url_fetch_deadline_ms(kUrlFetchDeadlineSec * 1000),
+      websocket_onopen_deadline_ms(kWebSocketConnectDeadlineSec * 1000),
+      websocket_hello_message("Hello"),
+      websocket_hello_echoback_deadline_ms(kWebSocketEchoDeadlineSec * 1000),
+      // Note: websocket live experiment server is configured to wait 1.5 sec
+      // in websocket_experiment_def.txt on server.  So, client expects idle
+      // at least 1 sec and expects a message arrival within next 1 sec.
+      websocket_idle_ms(kWebSocketIdleSec * 1000),
+      websocket_receive_push_message_deadline_ms(
+          kWebSocketPushDeadlineSec * 1000),
+      websocket_bye_message("Bye"),
+      websocket_bye_deadline_ms(kWebSocketByeDeadlineSec * 1000),
+      websocket_close_deadline_ms(kWebSocketCloseDeadlineSec * 1000) {
+}
+
 WebSocketExperimentTask::WebSocketExperimentTask(
     const Config& config,
     net::CompletionCallback* callback)
@@ -66,6 +112,101 @@ WebSocketExperimentTask::~WebSocketExperimentTask() {
   DCHECK(!websocket_);
 }
 
+/* static */
+void WebSocketExperimentTask::InitHistogram() {
+  DCHECK(!g_histogram_table);
+  g_histogram_table = new base::hash_map<std::string, Histogram*>;
+}
+
+static std::string GetCounterNameForConfig(
+    const WebSocketExperimentTask::Config& config, const std::string& name) {
+  // TODO(ukai): use config.protocol_version.
+  if (config.url.SchemeIs("wss")) {
+    return "WebSocketExperiment.Secure." + name;
+  } else if (config.url.has_port() && config.url.IntPort() != 80) {
+    return "WebSocketExperiment.NoDefaultPort." + name;
+  } else {
+    return "WebSocketExperiment.Basic." + name;
+  }
+}
+
+static Histogram* GetEnumsHistogramForConfig(
+    const WebSocketExperimentTask::Config& config,
+    const std::string& name,
+    Histogram::Sample boundary_value) {
+  DCHECK(g_histogram_table);
+  std::string counter_name = GetCounterNameForConfig(config, name);
+  base::hash_map<std::string, Histogram*>::iterator found =
+      g_histogram_table->find(counter_name);
+  if (found != g_histogram_table->end()) {
+    return found->second;
+  }
+  Histogram* counter = LinearHistogram::FactoryGet(
+      counter_name, 1, boundary_value, boundary_value + 1,
+      Histogram::kUmaTargetedHistogramFlag);
+  counter->AddRef();  // Released in ReleaseHistogram().
+  g_histogram_table->insert(std::make_pair(counter_name, counter));
+  return counter;
+}
+
+static Histogram* GetTimesHistogramForConfig(
+    const WebSocketExperimentTask::Config& config,
+    const std::string& name,
+    base::TimeDelta min,
+    base::TimeDelta max,
+    size_t bucket_count) {
+  DCHECK(g_histogram_table);
+  std::string counter_name = GetCounterNameForConfig(config, name);
+  base::hash_map<std::string, Histogram*>::iterator found =
+      g_histogram_table->find(counter_name);
+  if (found != g_histogram_table->end()) {
+    return found->second;
+  }
+  Histogram* counter = Histogram::FactoryGet(
+      counter_name, min, max, bucket_count,
+      Histogram::kUmaTargetedHistogramFlag);
+  counter->AddRef();  // Released in ReleaseHistogram().
+  g_histogram_table->insert(std::make_pair(counter_name, counter));
+  return counter;
+}
+
+static void UpdateHistogramEnums(
+    const WebSocketExperimentTask::Config& config,
+    const std::string& name,
+    Histogram::Sample sample,
+    Histogram::Sample boundary_value) {
+  Histogram* counter = GetEnumsHistogramForConfig(config, name, boundary_value);
+  counter->Add(sample);
+}
+
+static void UpdateHistogramTimes(
+    const WebSocketExperimentTask::Config& config,
+    const std::string& name,
+    base::TimeDelta sample,
+    base::TimeDelta min,
+    base::TimeDelta max,
+    size_t bucket_count) {
+  Histogram* counter = GetTimesHistogramForConfig(
+      config, name, min, max, bucket_count);
+  counter->AddTime(sample);
+}
+
+/* static */
+void WebSocketExperimentTask::ReleaseHistogram() {
+  DCHECK(g_histogram_table);
+  for (base::hash_map<std::string, Histogram*>::iterator iter =
+           g_histogram_table->begin();
+       iter != g_histogram_table->end();
+       ++iter) {
+    Histogram* counter = iter->second;
+    if (counter != NULL)
+      counter->Release();
+    iter->second = NULL;
+  }
+  delete g_histogram_table;
+  g_histogram_table = NULL;
+}
+
 void WebSocketExperimentTask::Run() {
   next_state_ = STATE_URL_FETCH;
   DoLoop(net::OK);
@@ -76,6 +217,49 @@ void WebSocketExperimentTask::Cancel() {
   DoLoop(net::OK);
 }
 
+void WebSocketExperimentTask::SaveResult() const {
+  UpdateHistogramEnums(config_, "LastState", result_.last_state, NUM_STATES);
+  UpdateHistogramTimes(config_, "UrlFetch", result_.url_fetch,
+                       base::TimeDelta::FromMilliseconds(1),
+                       base::TimeDelta::FromSeconds(kUrlFetchDeadlineSec),
+                       kTimeBucketCount);
+
+  if (result_.last_state < STATE_WEBSOCKET_CONNECT_COMPLETE)
+    return;
+
+  UpdateHistogramTimes(config_, "WebSocketConnect", result_.websocket_connect,
+                       base::TimeDelta::FromMilliseconds(1),
+                       base::TimeDelta::FromSeconds(
+                           kWebSocketConnectDeadlineSec),
+                       kTimeBucketCount);
+
+  if (result_.last_state < STATE_WEBSOCKET_RECV_HELLO)
+    return;
+
+  UpdateHistogramTimes(config_, "WebSocketEcho", result_.websocket_echo,
+                       base::TimeDelta::FromMilliseconds(1),
+                       base::TimeDelta::FromSeconds(
+                           kWebSocketEchoDeadlineSec),
+                       kTimeBucketCount);
+
+  if (result_.last_state < STATE_WEBSOCKET_KEEP_IDLE)
+    return;
+
+  UpdateHistogramTimes(config_, "WebSocketIdle", result_.websocket_idle,
+                       base::TimeDelta::FromMilliseconds(1),
+                       base::TimeDelta::FromSeconds(
+                           kWebSocketIdleSec + kWebSocketPushDeadlineSec),
+                       kTimeBucketCount);
+
+  if (result_.last_state < STATE_WEBSOCKET_CLOSE_COMPLETE)
+    return;
+
+  UpdateHistogramTimes(config_, "WebSocketTotal", result_.websocket_total,
+                       base::TimeDelta::FromMilliseconds(1),
+                       base::TimeDelta::FromSeconds(kWebSocketTimeSec),
+                       kTimeBucketCount);
+}
+
 // URLFetcher::Delegate method.
 void WebSocketExperimentTask::OnURLFetchComplete(
     const URLFetcher* source,
diff --git a/chrome/browser/net/websocket_experiment/websocket_experiment_task.h b/chrome/browser/net/websocket_experiment/websocket_experiment_task.h
index eeab4a7..4ae9078 100644
--- a/chrome/browser/net/websocket_experiment/websocket_experiment_task.h
+++ b/chrome/browser/net/websocket_experiment/websocket_experiment_task.h
@@ -74,18 +74,13 @@ class WebSocketExperimentTask : public URLFetcher::Delegate,
   };
   class Config {
    public:
-    Config()
-        : url_fetch_deadline_ms(0),
-          websocket_onopen_deadline_ms(0),
-          websocket_hello_echoback_deadline_ms(0),
-          websocket_idle_ms(0),
-          websocket_receive_push_message_deadline_ms(0),
-          websocket_bye_deadline_ms(0),
-          websocket_close_deadline_ms(0) {}
+    Config();
+
     GURL url;
     std::string ws_protocol;
     std::string ws_origin;
     std::string ws_location;
+    net::WebSocket::ProtocolVersion protocol_version;
 
     GURL http_url;
 
@@ -133,8 +128,17 @@ class WebSocketExperimentTask : public URLFetcher::Delegate,
                           net::CompletionCallback* callback);
   virtual ~WebSocketExperimentTask();
 
+  // Initializes histograms that WebSocketExperimentTask will use to save
+  // results.  Must be called once before calling SaveResult().
+  static void InitHistogram();
+
+  // Releases histograms to store results.
+  // Must be called after all WebSocketExperimentTasks are finished.
+  static void ReleaseHistogram();
+
   void Run();
   void Cancel();
+  void SaveResult() const;
 
   const Config& config() const { return config_; }
   const Result& result() const { return result_; }