Fixed rpc broker interface.

Fixed style issues of http://codereview.chromium.org/5349004/

BUG=none
TEST=none

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

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

7 files changed, 51 insertions, 39 deletions

diff --git a/ceee/ie/broker/broker_rpc_client.cc b/ceee/ie/broker/broker_rpc_client.cc
index 3bf360e..c607fa9 100644
--- a/ceee/ie/broker/broker_rpc_client.cc
+++ b/ceee/ie/broker/broker_rpc_client.cc
@@ -132,28 +132,28 @@ HRESULT BrokerRpcClient::FireEvent(const char* event_name,
     return S_OK;
   } RpcExcept(HandleRpcException(RpcExceptionCode())) {
     LogRpcException("RPC error in FireEvent", RpcExceptionCode());
-  } RpcEndExcept
     return RPC_E_FAULT;
+  } RpcEndExcept
 }
 
-bool BrokerRpcClient::SendUmaHistogramTimes(BSTR event_name, int sample) {
+HRESULT BrokerRpcClient::SendUmaHistogramTimes(const char* name, int sample) {
   RpcTryExcept {
-    BrokerRpcClient_SendUmaHistogramTimes(binding_handle_, event_name, sample);
-    return true;
+    BrokerRpcClient_SendUmaHistogramTimes(binding_handle_, name, sample);
+    return S_OK;
   } RpcExcept(HandleRpcException(RpcExceptionCode())) {
     LogRpcException("RPC error in SendUmaHistogramTimes", RpcExceptionCode());
+    return RPC_E_FAULT;
   } RpcEndExcept
-  return false;
 }
 
-bool BrokerRpcClient::SendUmaHistogramData(BSTR event_name, int sample,
-                                           int min, int max, int bucket_count) {
+HRESULT BrokerRpcClient::SendUmaHistogramData(
+    const char* name, int sample, int min, int max, int bucket_count) {
   RpcTryExcept {
-    BrokerRpcClient_SendUmaHistogramData(binding_handle_, event_name, sample,
-                                         min, max, bucket_count);
-    return true;
+    BrokerRpcClient_SendUmaHistogramData(
+        binding_handle_, name, sample, min, max, bucket_count);
+    return S_OK;
   } RpcExcept(HandleRpcException(RpcExceptionCode())) {
     LogRpcException("RPC error in SendUmaHistogramData", RpcExceptionCode());
+    return RPC_E_FAULT;
   } RpcEndExcept
-  return false;
 }
diff --git a/ceee/ie/broker/broker_rpc_client.h b/ceee/ie/broker/broker_rpc_client.h
index 0f96e0e..d80b164 100644
--- a/ceee/ie/broker/broker_rpc_client.h
+++ b/ceee/ie/broker/broker_rpc_client.h
@@ -31,14 +31,11 @@ class BrokerRpcClient {
   // @{
   // Calls FireEvent on server side.
   virtual HRESULT FireEvent(const char* event_name, const char* event_args);
-  // Adds uma to histograms on the server side. Either performance timings or
+  // 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);
+  virtual HRESULT SendUmaHistogramTimes(const char* name, int sample);
+  virtual HRESULT SendUmaHistogramData(
+      const char* name, int sample, int min, int max, int bucket_count);
   // @}
 
  private:
diff --git a/ceee/ie/broker/broker_rpc_lib.idl b/ceee/ie/broker/broker_rpc_lib.idl
index abb583a..0574ac0 100644
--- a/ceee/ie/broker/broker_rpc_lib.idl
+++ b/ceee/ie/broker/broker_rpc_lib.idl
@@ -31,13 +31,15 @@ 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, string] const char* name,
     [in] int sample);
+
 void SendUmaHistogramData(
     [in] handle_t binding_handle,
-    [in] BSTR event_name,
+    [in, string] const char* name,
     [in] int sample,
     [in] int min,
     [in] int max,
diff --git a/ceee/ie/broker/broker_rpc_server.cc b/ceee/ie/broker/broker_rpc_server.cc
index 6600efd..0c07c38 100644
--- a/ceee/ie/broker/broker_rpc_server.cc
+++ b/ceee/ie/broker/broker_rpc_server.cc
@@ -135,12 +135,11 @@ void BrokerRpcServer_FireEvent(
 }
 
 void BrokerRpcServer_SendUmaHistogramTimes(handle_t binding_handle,
-                                           BSTR event_name,
+                                           const char* name,
                                            int sample) {
   // We can't unfortunately use the HISTOGRAM_*_TIMES here because they use
   // static variables to save time.
   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),
@@ -152,14 +151,13 @@ void BrokerRpcServer_SendUmaHistogramTimes(handle_t binding_handle,
 }
 
 void BrokerRpcServer_SendUmaHistogramData(handle_t binding_handle,
-                                          BSTR event_name,
+                                          const char* 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.
   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);
diff --git a/ceee/ie/broker/broker_rpc_unittest.cc b/ceee/ie/broker/broker_rpc_unittest.cc
index ba24a56..74d0d7a 100644
--- a/ceee/ie/broker/broker_rpc_unittest.cc
+++ b/ceee/ie/broker/broker_rpc_unittest.cc
@@ -25,11 +25,17 @@ using testing::_;
 MOCK_STATIC_CLASS_BEGIN(BrokerRpcMock)
   MOCK_STATIC_INIT_BEGIN(BrokerRpcMock)
     MOCK_STATIC_INIT(GetRpcEndPointAddress);
-    MOCK_STATIC_INIT(BrokerRpcServer_FireEvent)
+    MOCK_STATIC_INIT(BrokerRpcServer_FireEvent);
+    MOCK_STATIC_INIT(BrokerRpcServer_SendUmaHistogramTimes);
+    MOCK_STATIC_INIT(BrokerRpcServer_SendUmaHistogramData);
   MOCK_STATIC_INIT_END()
   MOCK_STATIC0(std::wstring, , GetRpcEndPointAddress);
   MOCK_STATIC3(void, , BrokerRpcServer_FireEvent, handle_t, const char*,
                const char*);
+  MOCK_STATIC3(void, , BrokerRpcServer_SendUmaHistogramTimes, handle_t,
+               const char*, int);
+  MOCK_STATIC6(void, , BrokerRpcServer_SendUmaHistogramData, handle_t,
+               const char*, int, int, int, int);
 MOCK_STATIC_CLASS_END(BrokerRpcMock)
 
 class BrokerRpcTest : public testing::Test {
@@ -48,7 +54,7 @@ class BrokerRpcTest : public testing::Test {
 TEST_F(BrokerRpcTest, ConnectNoServer) {
   BrokerRpcClient client;
   ASSERT_FALSE(client.is_connected());
-  ASSERT_FALSE(SUCCEEDED(client.Connect(false)));
+  ASSERT_HRESULT_FAILED(client.Connect(false));
   ASSERT_FALSE(client.is_connected());
 }
 
@@ -58,25 +64,35 @@ TEST_F(BrokerRpcTest, Connect) {
   ASSERT_TRUE(server.Start());
   ASSERT_TRUE(server.is_started());
   BrokerRpcClient client;
-  ASSERT_TRUE(SUCCEEDED(client.Connect(false)));
+  ASSERT_HRESULT_SUCCEEDED(client.Connect(false));
   ASSERT_TRUE(client.is_connected());
 }
 
-TEST_F(BrokerRpcTest, FireEvent) {
+TEST_F(BrokerRpcTest, RpcCalls) {
   BrokerRpcServer server;
   ASSERT_TRUE(server.Start());
 
   BrokerRpcClient client;
-  ASSERT_TRUE(SUCCEEDED(client.Connect(false)));
+  ASSERT_HRESULT_SUCCEEDED(client.Connect(false));
 
-  const char* event_name = "event_name";
-  const char* event_args = "event_args";
+  const char* name = "name";
+  const char* args = "args";
 
-  EXPECT_CALL(broker_rpc_mock_, BrokerRpcServer_FireEvent(_, StrEq(event_name),
-                                                          StrEq(event_args)))
+  EXPECT_CALL(broker_rpc_mock_,
+      BrokerRpcServer_FireEvent(_, StrEq(name), StrEq(args)))
           .Times(1);
 
-  ASSERT_TRUE(SUCCEEDED(client.FireEvent(event_name, event_args)));
+  ASSERT_HRESULT_SUCCEEDED(client.FireEvent(name, args));
+
+  EXPECT_CALL(broker_rpc_mock_,
+      BrokerRpcServer_SendUmaHistogramTimes(_, StrEq(name), 321))
+          .Times(1);
+  ASSERT_HRESULT_SUCCEEDED(client.SendUmaHistogramTimes(name, 321));
+
+  EXPECT_CALL(broker_rpc_mock_,
+      BrokerRpcServer_SendUmaHistogramData(_, StrEq(name), 1, 2, 3, 4))
+          .Times(1);
+  ASSERT_HRESULT_SUCCEEDED(client.SendUmaHistogramData(name, 1, 2, 3, 4));
 }
 
 }  // namespace
diff --git a/ceee/ie/common/metrics_util.h b/ceee/ie/common/metrics_util.h
index 861a1b4..8af9034 100644
--- a/ceee/ie/common/metrics_util.h
+++ b/ceee/ie/common/metrics_util.h
@@ -35,10 +35,9 @@ class ScopedTimer {
   }
   ~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()))) {
+      if (FAILED(broker_rpc_->SendUmaHistogramTimes(
+              name_.c_str(), static_cast<int>(delta.InMilliseconds())))) {
         NOTREACHED() << "An error happened during RPC.";
       }
     }
diff --git a/ceee/ie/testing/mock_broker_and_friends.h b/ceee/ie/testing/mock_broker_and_friends.h
index 3b52b74..0fffb28 100644
--- a/ceee/ie/testing/mock_broker_and_friends.h
+++ b/ceee/ie/testing/mock_broker_and_friends.h
@@ -395,8 +395,8 @@ class MockBrokerRpcClient : public BrokerRpcClient {
   MOCK_METHOD0(Disconnect, void());
   MOCK_CONST_METHOD0(is_connected, bool());
   MOCK_METHOD2(FireEvent, HRESULT(const char*, const char*));
-  MOCK_METHOD2(SendUmaHistogramTimes, bool(BSTR, int));
-  MOCK_METHOD5(SendUmaHistogramData, bool(BSTR, int, int, int, int));
+  MOCK_METHOD2(SendUmaHistogramTimes, HRESULT(const char*, int));
+  MOCK_METHOD5(SendUmaHistogramData, HRESULT(const char*, int, int, int, int));
 };