Restart of ceee_broker on crash.

Code re-factoring and cleanup.

BUG=none
TEST=none

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

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

15 files changed, 290 insertions, 146 deletions

diff --git a/ceee/ie/broker/broker_rpc_client.cc b/ceee/ie/broker/broker_rpc_client.cc
index 4142475..387f548 100644
--- a/ceee/ie/broker/broker_rpc_client.cc
+++ b/ceee/ie/broker/broker_rpc_client.cc
@@ -9,17 +9,60 @@
 #include <atlbase.h>
 #include "base/lock.h"
 #include "base/logging.h"
+#include "base/tuple.h"
 #include "base/win/scoped_comptr.h"
 #include "broker_lib.h"  // NOLINT
 #include "broker_rpc_lib.h"  // NOLINT
 #include "ceee/common/com_utils.h"
 #include "ceee/ie/broker/broker_rpc_utils.h"
 
-BrokerRpcClient::BrokerRpcClient() : context_(0), binding_handle_(NULL) {
+
+namespace {
+
+void LogRpcException(const char* str, unsigned int exception_code) {
+  LOG(ERROR) << str << com::LogWe(exception_code);
 }
 
-BrokerRpcClient::~BrokerRpcClient() {
-  Disconnect();
+HRESULT BindRpc(std::wstring endpoint, RPC_BINDING_HANDLE* binding_handle) {
+  DCHECK(binding_handle != NULL);
+  std::wstring protocol = kRpcProtocol;
+  DCHECK(!protocol.empty());
+  DCHECK(!endpoint.empty());
+  if (protocol.empty() || endpoint.empty() || binding_handle == NULL)
+    return E_INVALIDARG;
+
+  RPC_BINDING_HANDLE tmp_binding_handle = NULL;
+
+  // TODO(vitalybuka@google.com): There's no guarantee (aside from name
+  // uniqueness) that it will connect to an endpoint created by the same user.
+  // Hint: The missing invocation is RpcBindingSetAuthInfoEx.
+  VLOG(1) << "Connecting to RPC server. Endpoint: " << endpoint;
+  RPC_WSTR string_binding = NULL;
+  // Create binding string for given end point.
+  RPC_STATUS status = ::RpcStringBindingCompose(
+      NULL,
+      reinterpret_cast<RPC_WSTR>(&protocol[0]),
+      NULL,
+      reinterpret_cast<RPC_WSTR>(&endpoint[0]),
+      NULL,
+      &string_binding);
+
+  if (RPC_S_OK == status) {
+    // Create binding from just generated binding string. Binding handle should
+    // be used for PRC calls.
+    status = ::RpcBindingFromStringBinding(string_binding, &tmp_binding_handle);
+    ::RpcStringFree(&string_binding);
+    if (RPC_S_OK == status) {
+      VLOG(1) << "RPC client is connected. Endpoint: " << endpoint;
+      *binding_handle = tmp_binding_handle;
+      return S_OK;
+    } else {
+      LogRpcException("Failed to bind. RPC_STATUS:", status);
+    }
+  } else {
+    LogRpcException("Failed to compose binding string. RPC_STATUS:", status);
+  }
+  return RPC_E_FAULT;
 }
 
 int HandleRpcException(unsigned int rpc_exception_code) {
@@ -38,8 +81,16 @@ int HandleRpcException(unsigned int rpc_exception_code) {
   return EXCEPTION_EXECUTE_HANDLER;
 }
 
-static void LogRpcException(const char* str, unsigned int exception_code) {
-  LOG(ERROR) << str << com::LogWe(exception_code);
+}  // namespace
+
+BrokerRpcClient::BrokerRpcClient(bool allow_restarts)
+    : context_(0),
+      binding_handle_(NULL),
+      allow_restarts_(allow_restarts) {
+}
+
+BrokerRpcClient::~BrokerRpcClient() {
+  Disconnect();
 }
 
 void BrokerRpcClient::LockContext() {
@@ -58,56 +109,33 @@ void BrokerRpcClient::ReleaseContext() {
   } RpcEndExcept
 }
 
+HRESULT BrokerRpcClient::StartServer(IUnknown** server) {
+  base::win::ScopedComPtr<IUnknown> broker;
+  // TODO(vitalybuka@google.com): Start broker without COM after the last
+  // COM interface is removed.
+  HRESULT hr = broker.CreateInstance(CLSID_CeeeBroker);
+  LOG_IF(ERROR, FAILED(hr)) << "Failed to create broker. " << com::LogHr(hr);
+  if (FAILED(hr))
+    return hr;
+  *server = broker.Detach();
+  return S_OK;
+}
+
 HRESULT BrokerRpcClient::Connect(bool start_server) {
   if (is_connected())
     return S_OK;
 
   // Keep alive until RPC is connected.
-  base::win::ScopedComPtr<ICeeeBrokerRegistrar> broker;
+  base::win::ScopedComPtr<IUnknown> broker;
   if (start_server) {
-    // TODO(vitalybuka@google.com): Start broker without COM after the last
-    // COM interface is removed.
-    HRESULT hr = broker.CreateInstance(CLSID_CeeeBroker);
-    LOG_IF(ERROR, FAILED(hr)) << "Failed to create broker. " << com::LogHr(hr);
+    HRESULT hr = StartServer(broker.Receive());
     if (FAILED(hr))
       return hr;
   }
 
-  std::wstring end_point = GetRpcEndPointAddress();
-  std::wstring protocol = kRpcProtocol;
-  DCHECK(!protocol.empty());
-  DCHECK(!end_point.empty());
-  if (protocol.empty() || end_point.empty())
-    return RPC_E_FAULT;
-
-  // TODO(vitalybuka@google.com): There's no guarantee (aside from name
-  // uniqueness) that it will connect to an endpoint created by the same user.
-  // Hint: The missing invocation is RpcBindingSetAuthInfoEx.
-  LOG(INFO) << "Connecting to RPC server. Endpoint: " << end_point;
-  RPC_WSTR string_binding = NULL;
-  // Create binding string with given protocol and end point.
-  RPC_STATUS status = ::RpcStringBindingCompose(
-      NULL,
-      reinterpret_cast<RPC_WSTR>(&protocol[0]),
-      NULL,
-      reinterpret_cast<RPC_WSTR>(&end_point[0]),
-      NULL,
-      &string_binding);
-  LOG_IF(ERROR, RPC_S_OK != status) <<
-      "Failed to compose binding string. RPC_STATUS=0x" << com::LogWe(status);
+  if (SUCCEEDED(BindRpc(GetRpcEndpointAddress(), &binding_handle_)))
+    LockContext();
 
-  if (RPC_S_OK == status) {
-    // Create binding from just generated binding string. Binding handle should
-    // used for PRC calls.
-    status = ::RpcBindingFromStringBinding(string_binding, &binding_handle_);
-    LOG_IF(ERROR, RPC_S_OK != status) <<
-        "Failed to bind. RPC_STATUS=0x" << com::LogWe(status);
-    ::RpcStringFree(&string_binding);
-    if (RPC_S_OK == status) {
-      LOG(INFO) << "RPC client is connected. Endpoint: " << end_point;
-      LockContext();
-    }
-  }
   if (!is_connected()) {
     Disconnect();
     return RPC_E_FAULT;
@@ -125,26 +153,42 @@ void BrokerRpcClient::Disconnect() {
   }
 }
 
-HRESULT BrokerRpcClient::FireEvent(const char* event_name,
-                                   const char* event_args) {
+template<class Function, class Params>
+HRESULT BrokerRpcClient::RunRpc(bool allow_restart,
+                                Function rpc_function,
+                                const Params& params) {
+  DCHECK(rpc_function);
+  if (!is_connected())
+    return RPC_E_FAULT;
   RpcTryExcept {
-    BrokerRpcClient_FireEvent(binding_handle_, context_, event_name,
-                              event_args);
+    DispatchToFunction(rpc_function, params);
     return S_OK;
   } RpcExcept(HandleRpcException(RpcExceptionCode())) {
-    LogRpcException("RPC error in FireEvent", RpcExceptionCode());
+    LogRpcException("RPC error in RunRpc", RpcExceptionCode());
+
+    if (allow_restart &&
+        RPC_S_OK != ::RpcMgmtIsServerListening(binding_handle_)) {
+      Disconnect();
+      if (SUCCEEDED(Connect(true))) {
+        return RunRpc(false, rpc_function, params);
+      }
+    }
     return RPC_E_FAULT;
   } RpcEndExcept
 }
 
+HRESULT BrokerRpcClient::FireEvent(const char* event_name,
+                                   const char* event_args) {
+  return RunRpc(allow_restarts_,
+                &BrokerRpcClient_FireEvent,
+                MakeRefTuple(binding_handle_, context_, event_name,
+                             event_args));
+}
+
 HRESULT BrokerRpcClient::SendUmaHistogramTimes(const char* name, int sample) {
-  RpcTryExcept {
-    BrokerRpcClient_SendUmaHistogramTimes(binding_handle_, name, sample);
-    return S_OK;
-  } RpcExcept(HandleRpcException(RpcExceptionCode())) {
-    LogRpcException("RPC error in SendUmaHistogramTimes", RpcExceptionCode());
-    return RPC_E_FAULT;
-  } RpcEndExcept
+  return RunRpc(allow_restarts_,
+                &BrokerRpcClient_SendUmaHistogramTimes,
+                MakeRefTuple(binding_handle_, name, sample));
 }
 
 HRESULT BrokerRpcClient::SendUmaHistogramData(const char* name,
@@ -152,12 +196,8 @@ HRESULT BrokerRpcClient::SendUmaHistogramData(const char* name,
                                               int min,
                                               int max,
                                               int bucket_count) {
-  RpcTryExcept {
-    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 RunRpc(allow_restarts_,
+                &BrokerRpcClient_SendUmaHistogramData,
+                MakeRefTuple(binding_handle_, name, sample, min, max,
+                          bucket_count));
 }
diff --git a/ceee/ie/broker/broker_rpc_client.h b/ceee/ie/broker/broker_rpc_client.h
index 38b84cc..3ca59ff1 100644
--- a/ceee/ie/broker/broker_rpc_client.h
+++ b/ceee/ie/broker/broker_rpc_client.h
@@ -10,6 +10,7 @@
 #include <wtypes.h>
 #include "base/basictypes.h"
 
+struct IUnknown;
 // Interface for sending events.
 class IEventSender {
  public:
@@ -18,19 +19,21 @@ class IEventSender {
                             const char* event_args) = 0;
 };
 
-
 // Class provides communication with BrokerRpcServer.
 class BrokerRpcClient : public IEventSender {
  public:
-  BrokerRpcClient();
+  // @param allow_restarts if true client will restart server if it is somehow
+  //     gone after successful connecting. Client restarts server one time after
+  //     each successful connecting.
+  explicit BrokerRpcClient(bool allow_restarts);
   virtual ~BrokerRpcClient();
 
   // Initialize connection with server.
-  // @param start_server if true method will try to start server if it is
-  // not started yet. Usually only tests pass false here.
+  // @param start_server if true method will try to start server if it is not
+  //     started yet. Usually only tests pass false here.
   virtual HRESULT Connect(bool start_server);
 
-  // Relese connection with server
+  // Releases connection with server
   virtual void Disconnect();
 
   // Returns true if object ready for remote calls.
@@ -52,14 +55,24 @@ class BrokerRpcClient : public IEventSender {
                                        int bucket_count);
   // @}
 
+ protected:
+  // Starts ceee broker process. This is unittest seam.
+  virtual HRESULT StartServer(IUnknown** server);
+
  private:
   void LockContext();
   void ReleaseContext();
 
+  template<class Function, class Params>
+  HRESULT RunRpc(bool allow_restart,
+                 Function rpc_function,
+                 const Params& params);
+
   RPC_BINDING_HANDLE binding_handle_;
   // Context handle. It is required to make RPC server know number of active
   // clients.
   void* context_;
+  bool allow_restarts_;
   DISALLOW_COPY_AND_ASSIGN(BrokerRpcClient);
 };
 
diff --git a/ceee/ie/broker/broker_rpc_lib.idl b/ceee/ie/broker/broker_rpc_lib.idl
index 1992931..84263e5 100644
--- a/ceee/ie/broker/broker_rpc_lib.idl
+++ b/ceee/ie/broker/broker_rpc_lib.idl
@@ -15,37 +15,32 @@ typedef [context_handle] void* BrokerContextHandle;
 // @name Implementation specific calls.
 // @{
 // Returns context handle for new client.
-BrokerContextHandle Connect( 
-    [in] handle_t binding_handle);
+BrokerContextHandle Connect([in] handle_t binding_handle);
 
 // Release context handle.
-void Disconnect( 
-    [in] handle_t binding_handle,
-    [in, out] BrokerContextHandle* context);
+void Disconnect([in] handle_t binding_handle,
+                [in, out] BrokerContextHandle* context);
 // @}
 
 // @name Remote calls.
 // @{
 // Fires event to broker.
 // @param context is required to avoid execution by wrong broker instance.
-void FireEvent(
-    [in] handle_t binding_handle,
-    [in] BrokerContextHandle context,
-    [in, string] const char* event_name,
-    [in, string] const char* event_args);
-
-void SendUmaHistogramTimes(
-    [in] handle_t binding_handle,
-    [in, string] const char* name,
-    [in] int sample);
-
-void SendUmaHistogramData(
-    [in] handle_t binding_handle,
-    [in, string] const char* name,
-    [in] int sample,
-    [in] int min,
-    [in] int max,
-    [in] int bucket_count);
+void FireEvent([in] handle_t binding_handle,
+               [in] BrokerContextHandle context,
+               [in, string] const char* event_name,
+               [in, string] const char* event_args);
+
+void SendUmaHistogramTimes([in] handle_t binding_handle,
+                           [in, string] const char* name,
+                           [in] int sample);
+
+void SendUmaHistogramData([in] handle_t binding_handle,
+                          [in, string] const char* 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 bd4ebdd..3adfd42 100644
--- a/ceee/ie/broker/broker_rpc_server.cc
+++ b/ceee/ie/broker/broker_rpc_server.cc
@@ -7,8 +7,9 @@
 #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/metrics/histogram.h"
+#include "base/process_util.h"
 #include "base/win_util.h"
 #include "broker_rpc_lib.h"  // NOLINT
 #include "ceee/common/com_utils.h"
@@ -16,10 +17,40 @@
 #include "ceee/ie/broker/broker_rpc_utils.h"
 #include "ceee/ie/broker/chrome_postman.h"
 
+
+namespace {
 // 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;
 
+RPC_STATUS PrepareEndpoint(std::wstring endpoint) {
+  std::wstring protocol = kRpcProtocol;
+  DCHECK(!protocol.empty());
+  DCHECK(!endpoint.empty());
+  if (protocol.empty() || endpoint.empty())
+    return false;
+  VLOG(1) << "RPC server is starting. Endpoint: " << endpoint;
+  // Tell RPC runtime to use local interprocess communication for given
+  // end point.
+  RPC_STATUS status = ::RpcServerUseProtseqEp(
+      reinterpret_cast<RPC_WSTR>(&protocol[0]),
+      RPC_C_PROTSEQ_MAX_REQS_DEFAULT,
+      reinterpret_cast<RPC_WSTR>(&endpoint[0]),
+      NULL);
+  LOG_IF(ERROR, RPC_S_OK != status && RPC_S_DUPLICATE_ENDPOINT != status) <<
+      "Failed to set protocol for RPC end point. RPC_STATUS=0x" <<
+      com::LogWe(status);
+  // This is not an error because unittest may start several servers. For
+  // ceee_broker this is an error because we should not have several instances
+  // of broker for the same endpoint. However BrokerRpcServer will fail anyway
+  // while starting to listen.
+  if (RPC_S_DUPLICATE_ENDPOINT == status)
+    status = RPC_S_OK;
+  return status;
+}
+
+}  // namespace
+
 BrokerRpcServer::BrokerRpcServer()
     : is_started_(false),
       current_thread_(::GetCurrentThreadId()) {
@@ -36,25 +67,10 @@ bool BrokerRpcServer::Start() {
   if (is_started())
     return true;
 
-  std::wstring end_point = GetRpcEndPointAddress();
-  std::wstring protocol = kRpcProtocol;
-  DCHECK(!protocol.empty());
-  DCHECK(!end_point.empty());
-  if (protocol.empty() || end_point.empty())
-    return false;
+  std::wstring endpoint = GetRpcEndpointAddress();
+  RPC_STATUS status = PrepareEndpoint(endpoint);
 
-  LOG(INFO) << "RPC server is starting. Endpoint: " << end_point;
-  // Tell RPC runtime to use local interprocess communication for given
-  // end point.
-  RPC_STATUS status = ::RpcServerUseProtseqEp(
-      reinterpret_cast<RPC_WSTR>(&protocol[0]),
-      RPC_C_PROTSEQ_MAX_REQS_DEFAULT,
-      reinterpret_cast<RPC_WSTR>(&end_point[0]),
-      NULL);
-  LOG_IF(ERROR, RPC_S_OK != status && RPC_S_DUPLICATE_ENDPOINT != status) <<
-      "Failed to set protocol for RPC end point. RPC_STATUS=0x" <<
-      com::LogWe(status);
-  if (RPC_S_OK == status || RPC_S_DUPLICATE_ENDPOINT == status) {
+  if (RPC_S_OK == status) {
     // Register RPC interface with the RPC runtime.
     status = ::RpcServerRegisterIfEx(BrokerRpcServer_CeeeBroker_v1_1_s_ifspec,
         NULL, NULL, 0, RPC_C_LISTEN_MAX_CALLS_DEFAULT, NULL);
@@ -66,7 +82,7 @@ bool BrokerRpcServer::Start() {
       LOG_IF(ERROR, RPC_S_OK != status) <<
           "Failed to start listening. RPC_STATUS=0x" << com::LogWe(status);
       if (RPC_S_OK == status) {
-        LOG(INFO) << "RPC server is started. Endpoint: " << end_point;
+        VLOG(1) << "RPC server is started. Endpoint: " << endpoint;
         is_started_ = true;
       }
     }
@@ -82,17 +98,19 @@ bool BrokerRpcServer::Stop() {
   is_started_ = false;
   // Stop server listening for RPC.
   RPC_STATUS status = ::RpcMgmtStopServerListening(NULL);
-  LOG_IF(WARNING, RPC_S_OK != status) <<
-         "Failed to stop listening. RPC_STATUS=0x" << com::LogWe(status);
+  LOG_IF(WARNING, RPC_S_OK != status && RPC_S_NOT_LISTENING != status) <<
+      "Failed to stop listening. RPC_STATUS=0x" << com::LogWe(status);
   // Wait while server stops listening threads.
   status = ::RpcMgmtWaitServerListen();
-  LOG_IF(WARNING, RPC_S_OK != status) <<
-         "Failed to wait server listen. RPC_STATUS=0x" << com::LogWe(status);
+  LOG_IF(WARNING, RPC_S_OK != status && RPC_S_NOT_LISTENING != status) <<
+      "Failed to wait server listen. RPC_STATUS=0x" << com::LogWe(status);
   // Unregister RPC interface.
   status = ::RpcServerUnregisterIf(
       BrokerRpcServer_CeeeBroker_v1_1_s_ifspec, NULL, FALSE);
-  LOG_IF(WARNING, RPC_S_OK != status) <<
-         "Failed to unregister interface. RPC_STATUS=0x" << com::LogWe(status);
+  LOG_IF(WARNING, RPC_S_OK != status || RPC_S_UNKNOWN_MGR_TYPE != status ||
+                  RPC_S_UNKNOWN_IF != status) <<
+      "Failed to unregister interface. RPC_STATUS=0x" << com::LogWe(status);
+
   return RPC_S_OK == status;
 }
 
diff --git a/ceee/ie/broker/broker_rpc_unittest.cc b/ceee/ie/broker/broker_rpc_unittest.cc
index 9795338..fd8f875 100644
--- a/ceee/ie/broker/broker_rpc_unittest.cc
+++ b/ceee/ie/broker/broker_rpc_unittest.cc
@@ -13,23 +13,23 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using testing::_;
-using testing::StrEq;
-using testing::StrictMock;
-using testing::Return;
-
 namespace {
 
 using testing::_;
+using testing::DoAll;
+using testing::Invoke;
+using testing::InvokeWithoutArgs;
+using testing::StrEq;
+using testing::Return;
 
 MOCK_STATIC_CLASS_BEGIN(BrokerRpcMock)
   MOCK_STATIC_INIT_BEGIN(BrokerRpcMock)
-    MOCK_STATIC_INIT(GetRpcEndPointAddress);
+    MOCK_STATIC_INIT(GetRpcEndpointAddress);
     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_STATIC0(std::wstring, , GetRpcEndpointAddress);
   MOCK_STATIC4(void, , BrokerRpcServer_FireEvent, handle_t, BrokerContextHandle,
                const char*, const char*);
   MOCK_STATIC3(void, , BrokerRpcServer_SendUmaHistogramTimes, handle_t,
@@ -41,7 +41,7 @@ MOCK_STATIC_CLASS_END(BrokerRpcMock)
 class BrokerRpcTest : public testing::Test {
  protected:
   virtual void SetUp() {
-    EXPECT_CALL(broker_rpc_mock_, GetRpcEndPointAddress())
+    EXPECT_CALL(broker_rpc_mock_, GetRpcEndpointAddress())
         .WillRepeatedly(Return(L"BrokerRpcTestEP"));
   }
 
@@ -52,7 +52,7 @@ class BrokerRpcTest : public testing::Test {
 };
 
 TEST_F(BrokerRpcTest, ConnectNoServer) {
-  BrokerRpcClient client;
+  BrokerRpcClient client(false);
   ASSERT_FALSE(client.is_connected());
   ASSERT_HRESULT_FAILED(client.Connect(false));
   ASSERT_FALSE(client.is_connected());
@@ -63,7 +63,7 @@ TEST_F(BrokerRpcTest, Connect) {
   ASSERT_FALSE(server.is_started());
   ASSERT_TRUE(server.Start());
   ASSERT_TRUE(server.is_started());
-  BrokerRpcClient client;
+  BrokerRpcClient client(false);
   ASSERT_HRESULT_SUCCEEDED(client.Connect(false));
   ASSERT_TRUE(client.is_connected());
 }
@@ -72,7 +72,7 @@ TEST_F(BrokerRpcTest, RpcCalls) {
   BrokerRpcServer server;
   ASSERT_TRUE(server.Start());
 
-  BrokerRpcClient client;
+  BrokerRpcClient client(false);
   ASSERT_HRESULT_SUCCEEDED(client.Connect(false));
 
   const char* name = "name";
@@ -95,4 +95,74 @@ TEST_F(BrokerRpcTest, RpcCalls) {
   ASSERT_HRESULT_SUCCEEDED(client.SendUmaHistogramData(name, 1, 2, 3, 4));
 }
 
+class TestingBrokerRpcClient : public BrokerRpcClient {
+ public:
+  explicit TestingBrokerRpcClient(bool allow_restarts)
+      : BrokerRpcClient(allow_restarts) {}
+  MOCK_METHOD1(StartServer, HRESULT(IUnknown**));
+};
+
+TEST_F(BrokerRpcTest, StartServer) {
+  BrokerRpcServer server;
+  TestingBrokerRpcClient client(true);
+  EXPECT_CALL(client, StartServer(_))
+      .Times(0);
+  ASSERT_HRESULT_FAILED(client.Connect(false));
+
+  EXPECT_CALL(client, StartServer(_))
+      .Times(1)
+      .WillOnce(DoAll(IgnoreResult(InvokeWithoutArgs(&server,
+                                                     &BrokerRpcServer::Start)),
+                      Return(RPC_E_FAULT)));
+  ASSERT_HRESULT_FAILED(client.Connect(true));
+
+  EXPECT_CALL(client, StartServer(_))
+      .Times(1)
+      .WillOnce(DoAll(IgnoreResult(InvokeWithoutArgs(&server,
+                                                     &BrokerRpcServer::Start)),
+                      Return(RPC_S_OK)));
+  ASSERT_HRESULT_SUCCEEDED(client.Connect(true));
+}
+
+TEST_F(BrokerRpcTest, NoRestartServer) {
+  BrokerRpcServer server;
+  TestingBrokerRpcClient client(false);
+  EXPECT_CALL(client, StartServer(_))
+      .Times(1)
+      .WillOnce(DoAll(IgnoreResult(InvokeWithoutArgs(&server,
+                                                     &BrokerRpcServer::Start)),
+                      Return(RPC_S_OK)));
+  ASSERT_HRESULT_SUCCEEDED(client.Connect(true));
+  server.Stop();
+
+  EXPECT_CALL(client, StartServer(_))
+      .Times(0);
+  EXPECT_CALL(broker_rpc_mock_,
+      BrokerRpcServer_FireEvent(_, _, _, _))
+          .Times(0);
+  ASSERT_HRESULT_FAILED(client.FireEvent("A", "B"));
+}
+
+TEST_F(BrokerRpcTest, RestartServer) {
+  BrokerRpcServer server;
+  TestingBrokerRpcClient client(true);
+  EXPECT_CALL(client, StartServer(_))
+      .Times(1)
+      .WillOnce(DoAll(IgnoreResult(InvokeWithoutArgs(&server,
+                                                     &BrokerRpcServer::Start)),
+                      Return(RPC_S_OK)));
+  ASSERT_HRESULT_SUCCEEDED(client.Connect(true));
+  server.Stop();
+
+  EXPECT_CALL(client, StartServer(_))
+      .Times(1)
+      .WillOnce(DoAll(IgnoreResult(InvokeWithoutArgs(&server,
+                                                     &BrokerRpcServer::Start)),
+                      Return(RPC_S_OK)));
+  EXPECT_CALL(broker_rpc_mock_,
+      BrokerRpcServer_FireEvent(_, _, _, _))
+          .Times(1);
+  ASSERT_HRESULT_SUCCEEDED(client.FireEvent("A", "B"));
+}
+
 }  // namespace
diff --git a/ceee/ie/broker/broker_rpc_utils.cc b/ceee/ie/broker/broker_rpc_utils.cc
index dd7e71e..1c99eea 100644
--- a/ceee/ie/broker/broker_rpc_utils.cc
+++ b/ceee/ie/broker/broker_rpc_utils.cc
@@ -14,22 +14,22 @@
 // Local interprocess communication only.
 const wchar_t kRpcProtocol[] = L"ncalrpc";
 
-std::wstring GetRpcEndPointAddress() {
-  std::wstring end_point;
+std::wstring GetRpcEndpointAddress() {
+  std::wstring endpoint;
   bool running_as_admin = false;
   // CEEE running as regular user and CEEE running as elevated user will start
   // different broker processes. So make end points names different to connect
   // to appropriate one.
   process_utils_win::IsCurrentProcessUacElevated(&running_as_admin);
   if (running_as_admin)
-    end_point += L"ADMIN-";
+    endpoint += L"ADMIN-";
   std::wstring sid;
   win_util::GetUserSidString(&sid);
-  end_point += sid;
-  end_point += L"-B4630D08-4621-41A1-A8D0-F1E98DA460D6";
+  endpoint += sid;
+  endpoint += L"-B4630D08-4621-41A1-A8D0-F1E98DA460D6";
   // XP does not accept endpoints longer than 52.
-  end_point.resize(std::min(52u, end_point.size()));
-  return end_point;
+  endpoint.resize(std::min(52u, endpoint.size()));
+  return endpoint;
 }
 
 void  __RPC_FAR * __RPC_USER midl_user_allocate(size_t len) {
diff --git a/ceee/ie/broker/broker_rpc_utils.h b/ceee/ie/broker/broker_rpc_utils.h
index c6ea083..38d7e41 100644
--- a/ceee/ie/broker/broker_rpc_utils.h
+++ b/ceee/ie/broker/broker_rpc_utils.h
@@ -14,6 +14,6 @@ extern const wchar_t kRpcProtocol[];
 
 // Returns RPC end point. Depends on current session ID and
 // account used by chrome frame.
-std::wstring GetRpcEndPointAddress();
+std::wstring GetRpcEndpointAddress();
 
 #endif  // CEEE_IE_BROKER_BROKER_RPC_UTILS_H_
diff --git a/ceee/ie/common/metrics_util_unittest.cc b/ceee/ie/common/metrics_util_unittest.cc
index 082204c..2fd6843 100644
--- a/ceee/ie/common/metrics_util_unittest.cc
+++ b/ceee/ie/common/metrics_util_unittest.cc
@@ -58,7 +58,7 @@ TEST_F(CeeeMetricsUtilTest, ScopedTimerSuccess) {
 
   // Test that timing is right. This should ultimately succeed.
   // We expect less than a couple milliseconds on this call.
-  testing::MockBrokerRpcClient broker_rpc;
+  testing::MockBrokerRpcClient broker_rpc(false);
   EXPECT_CALL(broker_rpc, SendUmaHistogramTimes(_, testing::Lt(10)));
   CreateAndDestroyTimer(&broker_rpc);
 }
diff --git a/ceee/ie/plugin/bho/browser_helper_object.cc b/ceee/ie/plugin/bho/browser_helper_object.cc
index e62cd80..6be1d89 100644
--- a/ceee/ie/plugin/bho/browser_helper_object.cc
+++ b/ceee/ie/plugin/bho/browser_helper_object.cc
@@ -102,6 +102,7 @@ BrowserHelperObject::BrowserHelperObject()
       thread_id_(::GetCurrentThreadId()),
       full_tab_chrome_frame_(false),
       broker_client_queue_(this),
+      broker_rpc_(false),
       tab_events_funnel_(broker_client()) {
   TRACE_EVENT_BEGIN("ceee.bho", this, "");
 }
@@ -171,7 +172,7 @@ STDMETHODIMP BrowserHelperObject::SetSite(IUnknown* site) {
 
     // TODO(vitalybuka@chromium.org): switch to sampling when we have enough
     // users.
-    ReportAddonLoadTime("BH0", CLSID_BrowserHelperObject);
+    ReportAddonLoadTime("BHO", CLSID_BrowserHelperObject);
     ReportAddonLoadTime("ChromeFrameBHO", CLSID_ChromeFrameBHO);
     ReportAddonLoadTime("Toolband", CLSID_ToolBand);
 
@@ -840,7 +841,7 @@ HRESULT BrowserHelperObject::PostMessage(int port_id,
 
 HRESULT BrowserHelperObject::PostMessageImpl(int port_id,
                                              const std::string& message) {
-    return extension_port_manager_.PostMessage(port_id, message);
+  return extension_port_manager_.PostMessage(port_id, message);
 }
 
 HRESULT BrowserHelperObject::OnCfPrivateMessage(BSTR msg,
@@ -955,8 +956,8 @@ STDMETHODIMP_(void) BrowserHelperObject::OnNavigateComplete2(
   HandleNavigateComplete(webbrowser, url_bstr);
 
   for (std::vector<Sink*>::iterator iter = sinks_.begin();
-    iter != sinks_.end(); ++iter) {
-      (*iter)->OnNavigateComplete(webbrowser, url_bstr);
+       iter != sinks_.end(); ++iter) {
+    (*iter)->OnNavigateComplete(webbrowser, url_bstr);
   }
 }
 
diff --git a/ceee/ie/plugin/bho/browser_helper_object_unittest.cc b/ceee/ie/plugin/bho/browser_helper_object_unittest.cc
index f37ac02..fada7d8 100644
--- a/ceee/ie/plugin/bho/browser_helper_object_unittest.cc
+++ b/ceee/ie/plugin/bho/browser_helper_object_unittest.cc
@@ -80,6 +80,8 @@ class TestingBrowserHelperObject
       public InstanceCountMixin<TestingBrowserHelperObject>,
       public InitializingCoClass<TestingBrowserHelperObject> {
  public:
+  TestingBrowserHelperObject() : mock_broker_rpc_client_(false) {}
+
   HRESULT Initialize(TestingBrowserHelperObject** self) {
     // Make sure this is done early so we can mock it.
     EXPECT_HRESULT_SUCCEEDED(MockChromeFrameHost::CreateInitializedIID(
diff --git a/ceee/ie/plugin/bho/cookie_accountant.h b/ceee/ie/plugin/bho/cookie_accountant.h
index 6ae0703..46efd20 100644
--- a/ceee/ie/plugin/bho/cookie_accountant.h
+++ b/ceee/ie/plugin/bho/cookie_accountant.h
@@ -54,7 +54,7 @@ class CookieAccountant {
   // queue the events sent to the broker. They don't need to be sent to the BHO
   // because they don't need tab_id anyway.
   CookieAccountant()
-      : cookie_events_funnel_(new BrokerRpcClient),
+      : cookie_events_funnel_(new BrokerRpcClient(true)),
         patching_wininet_functions_(false) {
   }
 
diff --git a/ceee/ie/plugin/bho/executor.cc b/ceee/ie/plugin/bho/executor.cc
index cb6a553..9e2dc60 100644
--- a/ceee/ie/plugin/bho/executor.cc
+++ b/ceee/ie/plugin/bho/executor.cc
@@ -428,7 +428,7 @@ HRESULT CeeeExecutor::Initialize(CeeeWindowHandle hwnd) {
   // is ready.
   if (window_utils::GetTopLevelParent(hwnd_) != hwnd_)
     infobar_manager_.reset(
-        new infobar_api::InfobarManager(hwnd_, new BrokerRpcClient));
+        new infobar_api::InfobarManager(hwnd_, new BrokerRpcClient(false)));
 
   return S_OK;
 }
diff --git a/ceee/ie/plugin/bho/mediumtest_browser_helper_object.cc b/ceee/ie/plugin/bho/mediumtest_browser_helper_object.cc
index 8f8b8a9..e575933 100644
--- a/ceee/ie/plugin/bho/mediumtest_browser_helper_object.cc
+++ b/ceee/ie/plugin/bho/mediumtest_browser_helper_object.cc
@@ -70,7 +70,9 @@ class TestingBrowserHelperObject
       public InitializingCoClass<TestingBrowserHelperObject>,
       public InstanceCountMixin<TestingBrowserHelperObject> {
  public:
-  TestingBrowserHelperObject() : mock_chrome_frame_host_(NULL) {
+  TestingBrowserHelperObject()
+      : mock_chrome_frame_host_(NULL),
+        mock_broker_rpc_client_(false) {
   }
 
   HRESULT Initialize(TestingBrowserHelperObject** self) {
diff --git a/ceee/ie/plugin/bho/webrequest_notifier.cc b/ceee/ie/plugin/bho/webrequest_notifier.cc
index 4661616..609dc45 100644
--- a/ceee/ie/plugin/bho/webrequest_notifier.cc
+++ b/ceee/ie/plugin/bho/webrequest_notifier.cc
@@ -32,7 +32,7 @@ WebRequestNotifier::WebRequestNotifier()
         : internet_status_callback_stub_(NULL),
           start_count_(0),
           initialize_state_(NOT_INITIALIZED),
-          webrequest_events_funnel_(new BrokerRpcClient) {
+          webrequest_events_funnel_(new BrokerRpcClient(true)) {
 }
 
 WebRequestNotifier::~WebRequestNotifier() {
diff --git a/ceee/ie/testing/mock_broker_and_friends.h b/ceee/ie/testing/mock_broker_and_friends.h
index e05344f..a1c75c2 100644
--- a/ceee/ie/testing/mock_broker_and_friends.h
+++ b/ceee/ie/testing/mock_broker_and_friends.h
@@ -394,6 +394,9 @@ class MockWebRequestEventsFunnel : public WebRequestEventsFunnel {
 
 class MockBrokerRpcClient : public BrokerRpcClient {
  public:
+  explicit MockBrokerRpcClient(bool allow_restart)
+      : BrokerRpcClient(allow_restart) {}
+  MOCK_METHOD1(StartServer, HRESULT(IUnknown**));
   MOCK_METHOD1(Connect, HRESULT(bool));
   MOCK_METHOD0(Disconnect, void());
   MOCK_CONST_METHOD0(is_connected, bool());