Add UMA histograms services to CEEE. The histograms are aggregated in the broker, using RPC to send them, and then sent once every 15 minutes to the server.

Uses metrics_service.cc from chrome_frame. Adds dependency to RPC runtime for the BHO and to glue_renderer for linking.

This depends on http://codereview.chromium.org/5247001 for uploading data.

BUG=none
TEST=none

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

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

13 files changed, 261 insertions, 1 deletions

diff --git a/ceee/ie/broker/broker.gyp b/ceee/ie/broker/broker.gyp
index b7f40ef..e669013 100644
--- a/ceee/ie/broker/broker.gyp
+++ b/ceee/ie/broker/broker.gyp
@@ -62,6 +62,8 @@
         '../../../ceee/common/common.gyp:initializing_coclass',
         '../../../ceee/common/common.gyp:ceee_common',
         '<(DEPTH)/chrome_frame/chrome_frame.gyp:chrome_tab_idl',
+        # Metrics service requires this.
+        '<(DEPTH)/webkit/support/webkit_support.gyp:webkit_user_agent',
       ],
       'sources': [
         'api_dispatcher.cc',
@@ -102,6 +104,10 @@
         'window_api_module.h',
         'window_events_funnel.cc',
         'window_events_funnel.h',
+        # For chrome metrics service
+        '<(DEPTH)/chrome_frame/metrics_service.cc',
+        '<(DEPTH)/chrome_frame/metrics_service.h',
+        '<(DEPTH)/chrome_frame/renderer_glue.cc',
       ],
       'include_dirs': [
         # For chrome_tab.h
@@ -133,6 +139,7 @@
         '<(DEPTH)/chrome/chrome.gyp:chrome_version_header',
         '<(DEPTH)/chrome_frame/chrome_frame.gyp:chrome_frame_ie',  # for GUIDs.
         '<(DEPTH)/chrome_frame/chrome_frame.gyp:chrome_tab_idl',
+        '<(DEPTH)/chrome/chrome.gyp:common',
       ],
       'msvs_settings': {
         'VCLinkerTool': {
diff --git a/ceee/ie/broker/broker_module.cc b/ceee/ie/broker/broker_module.cc
index fca2164..bdfec36 100644
--- a/ceee/ie/broker/broker_module.cc
+++ b/ceee/ie/broker/broker_module.cc
@@ -21,6 +21,7 @@
 #include "ceee/ie/common/crash_reporter.h"
 #include "ceee/common/com_utils.h"
 #include "chrome/common/url_constants.h"
+#include "chrome_frame/metrics_service.h"
 
 namespace {
 
@@ -164,6 +165,9 @@ HRESULT CeeeBrokerModule::PreMessageLoop(int show) {
   if (!rpc_server_.Start())
     return RPC_E_FAULT;
 
+  // Initialize metrics. We need the rpc_server_ above to be available.
+  MetricsService::Start();
+
   return CAtlExeModuleT<CeeeBrokerModule>::PreMessageLoop(show);
 }
 
@@ -172,6 +176,10 @@ HRESULT CeeeBrokerModule::PostMessageLoop() {
   Singleton<ExecutorsManager,
             ExecutorsManager::SingletonTraits>()->Terminate();
   WindowEventsFunnel::Terminate();
+
+  // Upload data if necessary.
+  MetricsService::Stop();
+
   chrome_postman_.Term();
   return hr;
 }
diff --git a/ceee/ie/broker/broker_rpc_client.cc b/ceee/ie/broker/broker_rpc_client.cc
index d24025c..54a308b 100644
--- a/ceee/ie/broker/broker_rpc_client.cc
+++ b/ceee/ie/broker/broker_rpc_client.cc
@@ -132,3 +132,25 @@ HRESULT BrokerRpcClient::FireEvent(const char* event_name,
   } RpcEndExcept
   return RPC_E_FAULT;
 }
+
+bool BrokerRpcClient::SendUmaHistogramTimes(BSTR event_name, int sample) {
+  RpcTryExcept {
+    BrokerRpcClient_SendUmaHistogramTimes(binding_handle_, event_name, sample);
+    return true;
+  } RpcExcept(HandleRpcException(RpcExceptionCode())) {
+    LogRpcException("RPC error in SendUmaHistogramTimes", RpcExceptionCode());
+  } RpcEndExcept
+  return false;
+}
+
+bool BrokerRpcClient::SendUmaHistogramData(BSTR event_name, int sample,
+                                           int min, int max, int bucket_count) {
+  RpcTryExcept {
+    BrokerRpcClient_SendUmaHistogramData(binding_handle_, event_name, sample,
+        min, max, bucket_count);
+    return true;
+  } RpcExcept(HandleRpcException(RpcExceptionCode())) {
+    LogRpcException("RPC error in SendUmaHistogramData", RpcExceptionCode());
+  } RpcEndExcept
+  return false;
+}
diff --git a/ceee/ie/broker/broker_rpc_client.h b/ceee/ie/broker/broker_rpc_client.h
index 6a9c79d..d204e62 100644
--- a/ceee/ie/broker/broker_rpc_client.h
+++ b/ceee/ie/broker/broker_rpc_client.h
@@ -31,6 +31,14 @@ class BrokerRpcClient {
   // @{
   // Calls FireEvent on server side.
   HRESULT FireEvent(const char* event_name, const char* event_args);
+  // Adds uma to histograms on the server side. Either performance timings or
+  // generic histogram.
+  virtual bool SendUmaHistogramTimes(BSTR event_name,
+                                     int sample);
+  virtual bool SendUmaHistogramData(BSTR event_name,
+                                    int sample,
+                                    int min, int max,
+                                    int bucket_count);
   // @}
 
   // Starts new CEEE broker if nessesary.
diff --git a/ceee/ie/broker/broker_rpc_lib.idl b/ceee/ie/broker/broker_rpc_lib.idl
index 7077e48..abb583a 100644
--- a/ceee/ie/broker/broker_rpc_lib.idl
+++ b/ceee/ie/broker/broker_rpc_lib.idl
@@ -31,6 +31,17 @@ void FireEvent(
     [in] handle_t binding_handle,
     [in, string] const char* event_name,
     [in, string] const char* event_args);
+void SendUmaHistogramTimes(
+    [in] handle_t binding_handle,
+    [in] BSTR event_name,
+    [in] int sample);
+void SendUmaHistogramData(
+    [in] handle_t binding_handle,
+    [in] BSTR event_name,
+    [in] int sample,
+    [in] int min,
+    [in] int max,
+    [in] int bucket_count);
 // @}
 }
 
diff --git a/ceee/ie/broker/broker_rpc_server.cc b/ceee/ie/broker/broker_rpc_server.cc
index 24147dd..0e899be 100644
--- a/ceee/ie/broker/broker_rpc_server.cc
+++ b/ceee/ie/broker/broker_rpc_server.cc
@@ -7,6 +7,7 @@
 #include "ceee/ie/broker/broker_rpc_server.h"
 
 #include "base/atomic_sequence_num.h"
+#include "base/metrics/histogram.h"
 #include "base/logging.h"
 #include "base/win_util.h"
 #include "broker_rpc_lib.h"  // NOLINT
@@ -15,6 +16,10 @@
 #include "ceee/ie/broker/broker_rpc_utils.h"
 #include "ceee/ie/broker/chrome_postman.h"
 
+// This lock ensures that histograms created by the broker are thread safe.
+// The histograms created here can be initialized on multiple threads.
+Lock g_metrics_lock;
+
 BrokerRpcServer::BrokerRpcServer()
     : is_started_(false),
       current_thread_(::GetCurrentThreadId()) {
@@ -128,3 +133,39 @@ void BrokerRpcServer_FireEvent(
   if (ChromePostman::GetInstance())
     ChromePostman::GetInstance()->FireEvent(event_name, event_args);
 }
+
+void BrokerRpcServer_SendUmaHistogramTimes(handle_t binding_handle,
+                                           BSTR event_name,
+                                           int sample) {
+  // We can't unfortunately use the HISTOGRAM_*_TIMES here because they use
+  // static variables to save time. FactoryTimeGet is not so expensive though
+  // and this call is non-blocking.
+  AutoLock lock(g_metrics_lock);
+  std::string name(CW2A(event_name).m_psz);
+  scoped_refptr<base::Histogram> counter =
+      base::Histogram::FactoryTimeGet(name,
+          base::TimeDelta::FromMilliseconds(1),
+          base::TimeDelta::FromSeconds(10),
+          50, base::Histogram::kUmaTargetedHistogramFlag);
+  DCHECK_EQ(name, counter->histogram_name());
+  if (counter.get())
+    counter->AddTime(base::TimeDelta::FromMilliseconds(sample));
+}
+
+void BrokerRpcServer_SendUmaHistogramData(handle_t binding_handle,
+                                          BSTR event_name,
+                                          int sample,
+                                          int min, int max,
+                                          int bucket_count) {
+  // We can't unfortunately use the HISTOGRAM_*_COUNT here because they use
+  // static variables to save time. FactoryTimeGet is not so expensive though
+  // and this call is non-blocking.
+  AutoLock lock(g_metrics_lock);
+  std::string name(CW2A(event_name).m_psz);
+  scoped_refptr<base::Histogram> counter =
+      base::Histogram::FactoryGet(name, min, max, bucket_count,
+          base::Histogram::kUmaTargetedHistogramFlag);
+  DCHECK_EQ(name, counter->histogram_name());
+  if (counter.get())
+    counter->AddTime(base::TimeDelta::FromMilliseconds(sample));
+}
diff --git a/ceee/ie/common/common.gyp b/ceee/ie/common/common.gyp
index 94e79f4..0790fb5 100644
--- a/ceee/ie/common/common.gyp
+++ b/ceee/ie/common/common.gyp
@@ -77,6 +77,7 @@
         'mock_ie_tab_interfaces.h',
         'ceee_module_util.cc',
         'ceee_module_util.h',
+        'metrics_util.h',
 
         # TODO(joi@chromium.org) Refactor to use chrome/common library.
         '../../../chrome/browser/automation/extension_automation_constants.cc',
diff --git a/ceee/ie/common/metrics_util.h b/ceee/ie/common/metrics_util.h
new file mode 100644
index 0000000..fed8033
--- /dev/null
+++ b/ceee/ie/common/metrics_util.h
@@ -0,0 +1,61 @@
+// Copyright (c) 2010 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.
+//
+// CEEE module-wide metrics utilities.
+
+#ifndef CEEE_IE_COMMON_METRICS_UTIL_H__
+#define CEEE_IE_COMMON_METRICS_UTIL_H__
+
+#include <atlsafe.h>
+#include <string>
+
+#include "base/logging.h"
+#include "base/time.h"
+#include "base/utf_string_conversions.h"
+#include "base/win/scoped_bstr.h"
+#include "base/win/windows_version.h"
+#include "ceee/ie/broker/broker_rpc_client.h"
+
+namespace metrics_util {
+
+// This is a class that measures the time taken from its construction to its
+// destruction. A good way to use it is to instantiate it on the stack.
+// TODO(hansl): Make sure the BrokerRpcClient calls from this class are non-
+//              blocking.
+class ScopedTimer {
+ public:
+  // Callers must ensure broker_rpc exceeds the life of this object.
+  ScopedTimer(const std::string name, BrokerRpcClient* broker_rpc)
+      : name_(name), broker_rpc_(broker_rpc), start_(base::TimeTicks::Now()) {
+    CHECK(broker_rpc) << "Invalid parameter.";
+
+    if (name.length() == 0) {
+      NOTREACHED() << "Histogram name shouldn't be empty.";
+      broker_rpc_ = NULL;  // Ensure we don't call the broker_rpc.
+    }
+  }
+  ~ScopedTimer() {
+    if (broker_rpc_) {
+      base::win::ScopedBstr name(CA2W(name_.c_str()).m_psz);
+      base::TimeDelta delta = base::TimeTicks::Now() - start_;
+      if (!broker_rpc_->SendUmaHistogramTimes(
+              name, static_cast<int>(delta.InMilliseconds()))) {
+        NOTREACHED() << "An error happened during RPC.";
+      }
+    }
+  }
+
+ protected:
+  base::TimeTicks start_;
+
+  // The RPC Client to use when sending events.
+  BrokerRpcClient* broker_rpc_;
+
+  // The name of the event when sending it.
+  std::string name_;
+};
+
+}
+
+#endif  // CEEE_IE_COMMON_METRICS_UTIL_H__
diff --git a/ceee/ie/common/metrics_util_unittest.cc b/ceee/ie/common/metrics_util_unittest.cc
new file mode 100644
index 0000000..2acc41c
--- /dev/null
+++ b/ceee/ie/common/metrics_util_unittest.cc
@@ -0,0 +1,72 @@
+// Copyright (c) 2010 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.
+//
+// Unit tests for the CEEE metrics utilities.
+
+#include <atlconv.h>
+
+#include "ceee/ie/common/metrics_util.h"
+
+#include <wtypes.h>
+#include <string>
+
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "base/win/registry.h"
+#include "base/string_util.h"
+#include "ceee/common/process_utils_win.h"
+#include "ceee/ie/testing/mock_broker_and_friends.h"
+#include "ceee/testing/utils/mock_com.h"
+#include "ceee/testing/utils/mock_window_utils.h"
+#include "ceee/testing/utils/mock_win32.h"
+#include "ceee/testing/utils/test_utils.h"
+#include "base/time.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace {
+
+using testing::_;
+using testing::DoAll;
+using testing::NotNull;
+using testing::SetArgumentPointee;
+using testing::StrictMock;
+using testing::Return;
+
+class BrokerRpcClientMock : public BrokerRpcClient {
+public:
+  MOCK_METHOD2(SendUmaHistogramTimes, bool(BSTR, int));
+  MOCK_METHOD5(SendUmaHistogramData, bool(BSTR, int, int, int, int));
+};
+
+class CeeeMetricsUtilTest : public testing::Test {
+ protected:
+  virtual void CreateAndDestroyTimer(BrokerRpcClient* rpc_client) {
+    metrics_util::ScopedTimer timer("NonEmptyName", rpc_client);
+  }
+};
+
+// Invalid broker or empty name initialization should CHECK(false).
+TEST_F(CeeeMetricsUtilTest, ScopedTimerError) {
+  testing::LogDisabler no_dcheck;
+  // Since we can't test yet for NOTREACHED() or CHECK(false), do nothing.
+  // TODO(hansl): when we can test for NOTREACHED, validate that passing invalid
+  // values to the ScopedTimer will indeed fail.
+}
+
+
+// Test for successful uses of the ScopedTimer.
+TEST_F(CeeeMetricsUtilTest, ScopedTimerSuccess) {
+  testing::LogDisabler no_dcheck;
+
+  // Test that timing is right. This should ultimately succeed.
+  // We expect less than a couple milliseconds on this call.
+  BrokerRpcClientMock broker_rpc;
+  EXPECT_CALL(broker_rpc, SendUmaHistogramTimes(_, testing::Lt(10)));
+  CreateAndDestroyTimer(&broker_rpc);
+}
+
+}  // namespace
diff --git a/ceee/ie/ie.gyp b/ceee/ie/ie.gyp
index a731e8e..91e7ebf 100644
--- a/ceee/ie/ie.gyp
+++ b/ceee/ie/ie.gyp
@@ -34,6 +34,7 @@
         'common/crash_reporter_unittest.cc',
         'common/extension_manifest_unittest.cc',
         'common/ceee_module_util_unittest.cc',
+        'common/metrics_util_unittest.cc',
         'plugin/bho/browser_helper_object_unittest.cc',
         'plugin/bho/cookie_accountant_unittest.cc',
         'plugin/bho/cookie_events_funnel_unittest.cc',
diff --git a/ceee/ie/plugin/bho/bho.gyp b/ceee/ie/plugin/bho/bho.gyp
index d6e4d9e..c38d820 100644
--- a/ceee/ie/plugin/bho/bho.gyp
+++ b/ceee/ie/plugin/bho/bho.gyp
@@ -16,6 +16,7 @@
       'dependencies': [
         '../toolband/toolband.gyp:toolband_idl',
         '../../broker/broker.gyp:broker',
+        '../../broker/broker.gyp:broker_rpc_lib',
         '../../common/common.gyp:ie_common',
         '../../common/common.gyp:ie_common_settings',
         '../../../common/common.gyp:ceee_common',
@@ -23,7 +24,7 @@
         '../../../../base/base.gyp:base',
         '../../../../chrome_frame/chrome_frame.gyp:chrome_tab_idl',
         # For the vtable patching stuff...
-        '../../../../chrome_frame/chrome_frame.gyp:chrome_frame_ie',
+        '<(DEPTH)/chrome_frame/chrome_frame.gyp:chrome_frame_ie',
       ],
       'sources': [
         'browser_helper_object.cc',
@@ -79,6 +80,9 @@
         # For chrome_tab.h
         '<(SHARED_INTERMEDIATE_DIR)',
       ],
+      'libraries': [
+        'rpcrt4.lib',
+      ],
     },
   ]
 }
diff --git a/ceee/ie/plugin/bho/browser_helper_object.cc b/ceee/ie/plugin/bho/browser_helper_object.cc
index 545e65f..e237442 100644
--- a/ceee/ie/plugin/bho/browser_helper_object.cc
+++ b/ceee/ie/plugin/bho/browser_helper_object.cc
@@ -22,6 +22,7 @@
 #include "ceee/ie/broker/tab_api_module.h"
 #include "ceee/ie/common/extension_manifest.h"
 #include "ceee/ie/common/ie_util.h"
+#include "ceee/ie/common/metrics_util.h"
 #include "ceee/ie/common/ceee_module_util.h"
 #include "ceee/ie/plugin/bho/cookie_accountant.h"
 #include "ceee/ie/plugin/bho/http_negotiate.h"
@@ -105,6 +106,8 @@ BrowserHelperObject::BrowserHelperObject()
 }
 
 BrowserHelperObject::~BrowserHelperObject() {
+  broker_rpc_.Disconnect();
+
   TRACE_EVENT_END("ceee.bho", this, "");
 }
 
@@ -129,6 +132,8 @@ STDMETHODIMP BrowserHelperObject::SetSite(IUnknown* site) {
   }
 
   if (NULL == site) {
+    metrics_util::ScopedTimer metrics_timer("ceee/BHO.TearDown", &broker_rpc_);
+
     // We're being torn down.
     TearDown();
 
@@ -137,6 +142,7 @@ STDMETHODIMP BrowserHelperObject::SetSite(IUnknown* site) {
     FireOnUnmappedEvent();
   }
 
+  metrics_util::ScopedTimer metrics_timer("ceee/BHO.Initialize", &broker_rpc_);
   HRESULT hr = SuperSite::SetSite(site);
   if (FAILED(hr))
     return hr;
@@ -252,6 +258,9 @@ HRESULT BrowserHelperObject::Initialize(IUnknown* site) {
       com::LogHr(hr);
   DCHECK(SUCCEEDED(hr)) << "CoCreating Broker. " << com::LogHr(hr);
   if (SUCCEEDED(hr)) {
+    broker_rpc_.Connect();
+    DCHECK(broker_rpc_.is_connected());
+
     DCHECK(executor_ == NULL);
     hr = CreateExecutor(&executor_);
     LOG_IF(ERROR, FAILED(hr)) << "Failed to create Executor, hr=" <<
@@ -742,6 +751,9 @@ STDMETHODIMP_(void) BrowserHelperObject::OnBeforeNavigate2(
 
 void BrowserHelperObject::OnBeforeNavigate2Impl(
     const ScopedDispatchPtr& webbrowser_disp, const CComBSTR& url) {
+  metrics_util::ScopedTimer metrics_timer("ceee/BHO.BeforeNavigate",
+                                          &broker_rpc_);
+
   base::win::ScopedComPtr<IWebBrowser2> webbrowser;
   HRESULT hr = webbrowser.QueryFrom(webbrowser_disp);
   if (FAILED(hr)) {
@@ -805,6 +817,8 @@ STDMETHODIMP_(void) BrowserHelperObject::OnDocumentComplete(
 
 void BrowserHelperObject::OnDocumentCompleteImpl(
     const ScopedWebBrowser2Ptr& webbrowser, const CComBSTR& url) {
+  metrics_util::ScopedTimer metrics_timer("ceee/BHO.DocumentComplete",
+                                          &broker_rpc_);
   for (std::vector<Sink*>::iterator iter = sinks_.begin();
        iter != sinks_.end(); ++iter) {
     (*iter)->OnDocumentComplete(webbrowser, url);
@@ -843,6 +857,9 @@ STDMETHODIMP_(void) BrowserHelperObject::OnNavigateComplete2(
 
 void BrowserHelperObject::OnNavigateComplete2Impl(
     const ScopedWebBrowser2Ptr& webbrowser, const CComBSTR& url) {
+  metrics_util::ScopedTimer metrics_timer("ceee/BHO.NavigateComplete",
+                                          &broker_rpc_);
+
   HandleNavigateComplete(webbrowser, url);
 
   for (std::vector<Sink*>::iterator iter = sinks_.begin();
@@ -892,6 +909,9 @@ STDMETHODIMP_(void) BrowserHelperObject::OnNavigateError(
 void BrowserHelperObject::OnNavigateErrorImpl(
     const ScopedWebBrowser2Ptr& webbrowser, const CComBSTR& url,
     LONG status_code) {
+  metrics_util::ScopedTimer metrics_timer("ceee/BHO.NavigateError",
+                                          &broker_rpc_);
+
   for (std::vector<Sink*>::iterator iter = sinks_.begin();
        iter != sinks_.end(); ++iter) {
     (*iter)->OnNavigateError(webbrowser, url, status_code);
diff --git a/ceee/ie/plugin/bho/browser_helper_object.h b/ceee/ie/plugin/bho/browser_helper_object.h
index 3758fec..039b305 100644
--- a/ceee/ie/plugin/bho/browser_helper_object.h
+++ b/ceee/ie/plugin/bho/browser_helper_object.h
@@ -21,6 +21,7 @@
 #include "base/win/scoped_bstr.h"
 #include "base/win/scoped_comptr.h"
 #include "base/task.h"
+#include "ceee/ie/broker/broker_rpc_client.h"
 #include "ceee/ie/plugin/bho/tab_events_funnel.h"
 #include "ceee/ie/common/chrome_frame_host.h"
 #include "ceee/ie/plugin/bho/frame_event_handler.h"
@@ -366,6 +367,9 @@ class ATL_NO_VTABLE BrowserHelperObject
   // Indicates if the current shown page is a full-tab chrome frame.
   bool full_tab_chrome_frame_;
 
+  // The RPC client used to communicate with the broker.
+  BrokerRpcClient broker_rpc_;
+
  private:
   // Used during initialization to get the tab information from Chrome and
   // register ourselves with the broker.