Clean up ServiceIPCServer and make it more testable.

* Delete unused message filter and its getter.
* Delete unused channel getter.
* Inject shutdown event in constructor to cut out one use of the global g_service_process, working towards making this class more easily testable in isolation.

BUG=402456

Review URL: https://codereview.chromium.org/1233373002

Cr-Commit-Position: refs/heads/master@{#339239}

4 files changed, 15 insertions, 26 deletions

diff --git a/chrome/service/service_ipc_server.cc b/chrome/service/service_ipc_server.cc
index 808f674..dbc0065 100644
--- a/chrome/service/service_ipc_server.cc
+++ b/chrome/service/service_ipc_server.cc
@@ -10,16 +10,17 @@
 #include "chrome/service/service_process.h"
 #include "ipc/ipc_logging.h"
 
-ServiceIPCServer::ServiceIPCServer(const IPC::ChannelHandle& channel_handle)
-    : channel_handle_(channel_handle), client_connected_(false) {
+ServiceIPCServer::ServiceIPCServer(const IPC::ChannelHandle& channel_handle,
+                                   base::WaitableEvent* shutdown_event)
+    : channel_handle_(channel_handle),
+      shutdown_event_(shutdown_event),
+      client_connected_(false) {
 }
 
 bool ServiceIPCServer::Init() {
 #ifdef IPC_MESSAGE_LOG_ENABLED
   IPC::Logging::GetInstance()->SetIPCSender(this);
 #endif
-  sync_message_filter_ =
-      new IPC::SyncMessageFilter(g_service_process->shutdown_event());
   CreateChannel();
   return true;
 }
@@ -29,17 +30,13 @@ void ServiceIPCServer::CreateChannel() {
   channel_ = IPC::SyncChannel::Create(
       channel_handle_, IPC::Channel::MODE_NAMED_SERVER, this,
       g_service_process->io_task_runner().get(), true,
-      g_service_process->shutdown_event());
-  DCHECK(sync_message_filter_.get());
-  channel_->AddFilter(sync_message_filter_.get());
+      shutdown_event_);
 }
 
 ServiceIPCServer::~ServiceIPCServer() {
 #ifdef IPC_MESSAGE_LOG_ENABLED
   IPC::Logging::GetInstance()->SetIPCSender(NULL);
 #endif
-
-  channel_->RemoveFilter(sync_message_filter_.get());
 }
 
 void ServiceIPCServer::OnChannelConnected(int32 peer_pid) {
diff --git a/chrome/service/service_ipc_server.h b/chrome/service/service_ipc_server.h
index b858df5..5e0885f 100644
--- a/chrome/service/service_ipc_server.h
+++ b/chrome/service/service_ipc_server.h
@@ -12,20 +12,21 @@
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_sync_channel.h"
-#include "ipc/ipc_sync_message_filter.h"
 #include "ipc/ipc_sender.h"
 
 namespace base {
 
 class DictionaryValue;
 class HistogramDeltaSerialization;
+class WaitableEvent;
 
 }  // namespace base
 
 // This class handles IPC commands for the service process.
 class ServiceIPCServer : public IPC::Listener, public IPC::Sender {
  public:
-  explicit ServiceIPCServer(const IPC::ChannelHandle& handle);
+  ServiceIPCServer(const IPC::ChannelHandle& handle,
+                   base::WaitableEvent* shutdown_event);
   ~ServiceIPCServer() override;
 
   bool Init();
@@ -33,17 +34,8 @@ class ServiceIPCServer : public IPC::Listener, public IPC::Sender {
   // IPC::Sender implementation.
   bool Send(IPC::Message* msg) override;
 
-  IPC::SyncChannel* channel() { return channel_.get(); }
-
-  // Safe to call on any thread, as long as it's guaranteed that the thread's
-  // lifetime is less than the main thread.
-  IPC::SyncMessageFilter* sync_message_filter() {
-    return sync_message_filter_.get();
-  }
-
   bool is_client_connected() const { return client_connected_; }
 
-
  private:
   friend class MockServiceIPCServer;
 
@@ -71,12 +63,11 @@ class ServiceIPCServer : public IPC::Listener, public IPC::Sender {
 
   IPC::ChannelHandle channel_handle_;
   scoped_ptr<IPC::SyncChannel> channel_;
+  base::WaitableEvent* shutdown_event_;
+
   // Indicates whether a client is currently connected to the channel.
   bool client_connected_;
 
-  // Allows threads other than the main thread to send sync messages.
-  scoped_refptr<IPC::SyncMessageFilter> sync_message_filter_;
-
   // Calculates histograms deltas.
   scoped_ptr<base::HistogramDeltaSerialization> histogram_delta_serializer_;
 
diff --git a/chrome/service/service_process.cc b/chrome/service/service_process.cc
index b3b9992..342d3d1 100644
--- a/chrome/service/service_process.cc
+++ b/chrome/service/service_process.cc
@@ -113,7 +113,7 @@ void PrepareRestartOnCrashEnviroment(
 }  // namespace
 
 ServiceProcess::ServiceProcess()
-  : shutdown_event_(true, false),
+  : shutdown_event_(true /* manual_reset */, false /* initially_signaled */),
     main_message_loop_(NULL),
     enabled_services_(0),
     update_available_(false) {
@@ -190,7 +190,8 @@ bool ServiceProcess::Initialize(base::MessageLoopForUI* message_loop,
 
   VLOG(1) << "Starting Service Process IPC Server";
   ipc_server_.reset(new ServiceIPCServer(
-      service_process_state_->GetServiceProcessChannel()));
+      service_process_state_->GetServiceProcessChannel(),
+      &shutdown_event_));
   ipc_server_->Init();
 
   // After the IPC server has started we signal that the service process is
diff --git a/chrome/service/service_process.h b/chrome/service/service_process.h
index f2b9e05..1aac3c9c 100644
--- a/chrome/service/service_process.h
+++ b/chrome/service/service_process.h
@@ -63,7 +63,7 @@ class ServiceProcess : public cloud_print::CloudPrintProxy::Client {
   // A global event object that is signalled when the main thread's message
   // loop exits. This gives background threads a way to observe the main
   // thread shutting down.
-  base::WaitableEvent* shutdown_event() {
+  base::WaitableEvent* GetShutdownEventForTesting() {
     return &shutdown_event_;
   }