ipc: Use a global for the process's attachment broker.

This eliminates the need for a lot of plumbing, at the expense of yet another
global.

BUG=493414

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

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

10 files changed, 57 insertions, 48 deletions

diff --git a/ipc/attachment_broker.cc b/ipc/attachment_broker.cc
index 7450ec3..7e04bcd 100644
--- a/ipc/attachment_broker.cc
+++ b/ipc/attachment_broker.cc
@@ -6,8 +6,23 @@
 
 #include <algorithm>
 
+namespace {
+IPC::AttachmentBroker* g_attachment_broker = nullptr;
+}
+
 namespace IPC {
 
+// static
+void AttachmentBroker::SetGlobal(AttachmentBroker* broker) {
+  CHECK(!g_attachment_broker);
+  g_attachment_broker = broker;
+}
+
+// static
+AttachmentBroker* AttachmentBroker::GetGlobal() {
+  return g_attachment_broker;
+}
+
 AttachmentBroker::AttachmentBroker() {}
 AttachmentBroker::~AttachmentBroker() {}
 
diff --git a/ipc/attachment_broker.h b/ipc/attachment_broker.h
index 5b65289..577d762 100644
--- a/ipc/attachment_broker.h
+++ b/ipc/attachment_broker.h
@@ -50,6 +50,12 @@ class IPC_EXPORT AttachmentBroker : public Listener {
         const BrokerableAttachment::AttachmentId& id) = 0;
   };
 
+  // Each process has at most one attachment broker. The process is responsible
+  // for ensuring that |broker| stays alive for as long as the process is
+  // sending/receiving ipc messages.
+  static void SetGlobal(AttachmentBroker* broker);
+  static AttachmentBroker* GetGlobal();
+
   AttachmentBroker();
   ~AttachmentBroker() override;
 
diff --git a/ipc/attachment_broker_privileged_win_unittest.cc b/ipc/attachment_broker_privileged_win_unittest.cc
index 2e46f2b..480a2b4 100644
--- a/ipc/attachment_broker_privileged_win_unittest.cc
+++ b/ipc/attachment_broker_privileged_win_unittest.cc
@@ -154,6 +154,7 @@ class IPCAttachmentBrokerPrivilegedWinTest : public IPCTestBase {
   // Takes ownership of |broker|. Has no effect if called after CommonSetUp().
   void set_broker(IPC::AttachmentBrokerUnprivilegedWin* broker) {
     broker_.reset(broker);
+    IPC::AttachmentBroker::SetGlobal(broker);
   }
 
   void CommonSetUp() {
@@ -343,7 +344,7 @@ int CommonPrivilegedProcessMain(OnMessageReceivedCallback callback,
 
   // Set up IPC channel.
   IPC::AttachmentBrokerPrivilegedWin broker;
-  listener.set_listener(&broker);
+  IPC::AttachmentBroker::SetGlobal(&broker);
   scoped_ptr<IPC::Channel> channel(IPC::Channel::CreateClient(
       IPCTestBase::GetChannelName(channel_name), &listener, &broker));
   broker.RegisterCommunicationChannel(channel.get());
diff --git a/ipc/ipc_channel_nacl.cc b/ipc/ipc_channel_nacl.cc
index b8704bf..769ead0 100644
--- a/ipc/ipc_channel_nacl.cc
+++ b/ipc/ipc_channel_nacl.cc
@@ -130,8 +130,7 @@ ChannelNacl::ChannelNacl(const IPC::ChannelHandle& channel_handle,
       waiting_connect_(true),
       pipe_(-1),
       pipe_name_(channel_handle.name),
-      weak_ptr_factory_(this),
-      broker_(broker) {
+      weak_ptr_factory_(this) {
   if (!CreatePipe(channel_handle)) {
     // The pipe may have been closed already.
     const char *modestr = (mode_ & MODE_SERVER_FLAG) ? "server" : "client";
@@ -225,7 +224,7 @@ bool ChannelNacl::Send(Message* message) {
 }
 
 AttachmentBroker* ChannelNacl::GetAttachmentBroker() {
-  return broker_;
+  return nullptr;
 }
 
 void ChannelNacl::DidRecvMsg(scoped_ptr<MessageContents> contents) {
diff --git a/ipc/ipc_channel_nacl.h b/ipc/ipc_channel_nacl.h
index 8d26772..0ed0da8 100644
--- a/ipc/ipc_channel_nacl.h
+++ b/ipc/ipc_channel_nacl.h
@@ -120,9 +120,6 @@ class ChannelNacl : public Channel,
 
   base::WeakPtrFactory<ChannelNacl> weak_ptr_factory_;
 
-  // |broker_| must outlive this instance.
-  AttachmentBroker* broker_;
-
   DISALLOW_IMPLICIT_CONSTRUCTORS(ChannelNacl);
 };
 
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index 2339687..b3aee8e 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -38,6 +38,7 @@
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/synchronization/lock.h"
+#include "ipc/attachment_broker.h"
 #include "ipc/ipc_descriptors.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_logging.h"
@@ -182,8 +183,7 @@ int ChannelPosix::global_pid_ = 0;
 
 ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle,
                            Mode mode,
-                           Listener* listener,
-                           AttachmentBroker* broker)
+                           Listener* listener)
     : ChannelReader(listener),
       mode_(mode),
       peer_pid_(base::kNullProcessId),
@@ -192,8 +192,7 @@ ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle,
       message_send_bytes_written_(0),
       pipe_name_(channel_handle.name),
       in_dtor_(false),
-      must_unlink_(false),
-      broker_(broker) {
+      must_unlink_(false) {
   if (!CreatePipe(channel_handle)) {
     // The pipe may have been closed already.
     const char *modestr = (mode_ & MODE_SERVER_FLAG) ? "server" : "client";
@@ -524,7 +523,7 @@ bool ChannelPosix::Send(Message* message) {
 }
 
 AttachmentBroker* ChannelPosix::GetAttachmentBroker() {
-  return broker_;
+  return AttachmentBroker::GetGlobal();
 }
 
 int ChannelPosix::GetClientFileDescriptor() const {
@@ -1023,7 +1022,7 @@ scoped_ptr<Channel> Channel::Create(const IPC::ChannelHandle& channel_handle,
                                     Listener* listener,
                                     AttachmentBroker* broker) {
   return make_scoped_ptr(
-      new ChannelPosix(channel_handle, mode, listener, broker));
+      new ChannelPosix(channel_handle, mode, listener));
 }
 
 // static
diff --git a/ipc/ipc_channel_posix.h b/ipc/ipc_channel_posix.h
index 8760135..be47705d 100644
--- a/ipc/ipc_channel_posix.h
+++ b/ipc/ipc_channel_posix.h
@@ -29,8 +29,7 @@ class IPC_EXPORT ChannelPosix : public Channel,
   // |broker| must outlive the newly created object.
   ChannelPosix(const IPC::ChannelHandle& channel_handle,
                Mode mode,
-               Listener* listener,
-               AttachmentBroker* broker);
+               Listener* listener);
   ~ChannelPosix() override;
 
   // Channel implementation
@@ -182,9 +181,6 @@ class IPC_EXPORT ChannelPosix : public Channel,
   static int global_pid_;
 #endif  // OS_LINUX
 
-  // |broker_| must outlive this instance.
-  AttachmentBroker* broker_;
-
   DISALLOW_IMPLICIT_CONSTRUCTORS(ChannelPosix);
 };
 
diff --git a/ipc/ipc_channel_posix_unittest.cc b/ipc/ipc_channel_posix_unittest.cc
index eb78b7c..3aa9d04 100644
--- a/ipc/ipc_channel_posix_unittest.cc
+++ b/ipc/ipc_channel_posix_unittest.cc
@@ -203,7 +203,7 @@ TEST_F(IPCChannelPosixTest, BasicListen) {
   SetUpSocket(&handle, IPC::Channel::MODE_NAMED_SERVER);
   unlink(handle.name.c_str());
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      handle, IPC::Channel::MODE_NAMED_SERVER, NULL, nullptr));
+      handle, IPC::Channel::MODE_NAMED_SERVER, NULL));
   ASSERT_TRUE(channel->Connect());
   ASSERT_TRUE(channel->AcceptsConnections());
   ASSERT_FALSE(channel->HasAcceptedConnection());
@@ -222,7 +222,7 @@ TEST_F(IPCChannelPosixTest, BasicConnected) {
   base::FileDescriptor fd(pipe_fds[0], false);
   IPC::ChannelHandle handle(socket_name, fd);
   scoped_ptr<IPC::ChannelPosix> channel(
-      new IPC::ChannelPosix(handle, IPC::Channel::MODE_SERVER, NULL, nullptr));
+      new IPC::ChannelPosix(handle, IPC::Channel::MODE_SERVER, NULL));
   ASSERT_TRUE(channel->Connect());
   ASSERT_FALSE(channel->AcceptsConnections());
   channel->Close();
@@ -231,7 +231,7 @@ TEST_F(IPCChannelPosixTest, BasicConnected) {
   // Make sure that we can use the socket that is created for us by
   // a standard channel.
   scoped_ptr<IPC::ChannelPosix> channel2(new IPC::ChannelPosix(
-      socket_name, IPC::Channel::MODE_SERVER, NULL, nullptr));
+      socket_name, IPC::Channel::MODE_SERVER, NULL));
   ASSERT_TRUE(channel2->Connect());
   ASSERT_FALSE(channel2->AcceptsConnections());
 }
@@ -244,11 +244,11 @@ TEST_F(IPCChannelPosixTest, SendHangTest) {
   IPCChannelPosixTestListener in_listener(true);
   IPC::ChannelHandle in_handle("IN");
   scoped_ptr<IPC::ChannelPosix> in_chan(new IPC::ChannelPosix(
-      in_handle, IPC::Channel::MODE_SERVER, &in_listener, nullptr));
+      in_handle, IPC::Channel::MODE_SERVER, &in_listener));
   IPC::ChannelHandle out_handle(
       "OUT", base::FileDescriptor(in_chan->TakeClientFileDescriptor()));
   scoped_ptr<IPC::ChannelPosix> out_chan(new IPC::ChannelPosix(
-      out_handle, IPC::Channel::MODE_CLIENT, &out_listener, nullptr));
+      out_handle, IPC::Channel::MODE_CLIENT, &out_listener));
   ASSERT_TRUE(in_chan->Connect());
   ASSERT_TRUE(out_chan->Connect());
   in_chan->Close();  // simulate remote process dying at an unfortunate time.
@@ -269,11 +269,11 @@ TEST_F(IPCChannelPosixTest, AcceptHangTest) {
   IPCChannelPosixTestListener in_listener(true);
   IPC::ChannelHandle in_handle("IN");
   scoped_ptr<IPC::ChannelPosix> in_chan(new IPC::ChannelPosix(
-      in_handle, IPC::Channel::MODE_SERVER, &in_listener, nullptr));
+      in_handle, IPC::Channel::MODE_SERVER, &in_listener));
   IPC::ChannelHandle out_handle(
       "OUT", base::FileDescriptor(in_chan->TakeClientFileDescriptor()));
   scoped_ptr<IPC::ChannelPosix> out_chan(new IPC::ChannelPosix(
-      out_handle, IPC::Channel::MODE_CLIENT, &out_listener, nullptr));
+      out_handle, IPC::Channel::MODE_CLIENT, &out_listener));
   ASSERT_TRUE(in_chan->Connect());
   in_chan->Close();  // simulate remote process dying at an unfortunate time.
   ASSERT_FALSE(out_chan->Connect());
@@ -292,7 +292,7 @@ TEST_F(IPCChannelPosixTest, MAYBE_AdvancedConnected) {
   IPC::ChannelHandle chan_handle(GetConnectionSocketName());
   SetUpSocket(&chan_handle, IPC::Channel::MODE_NAMED_SERVER);
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      chan_handle, IPC::Channel::MODE_NAMED_SERVER, &listener, nullptr));
+      chan_handle, IPC::Channel::MODE_NAMED_SERVER, &listener));
   ASSERT_TRUE(channel->Connect());
   ASSERT_TRUE(channel->AcceptsConnections());
   ASSERT_FALSE(channel->HasAcceptedConnection());
@@ -328,7 +328,7 @@ TEST_F(IPCChannelPosixTest, MAYBE_ResetState) {
   IPC::ChannelHandle chan_handle(GetConnectionSocketName());
   SetUpSocket(&chan_handle, IPC::Channel::MODE_NAMED_SERVER);
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      chan_handle, IPC::Channel::MODE_NAMED_SERVER, &listener, nullptr));
+      chan_handle, IPC::Channel::MODE_NAMED_SERVER, &listener));
   ASSERT_TRUE(channel->Connect());
   ASSERT_TRUE(channel->AcceptsConnections());
   ASSERT_FALSE(channel->HasAcceptedConnection());
@@ -364,7 +364,7 @@ TEST_F(IPCChannelPosixTest, BadChannelName) {
   // Test empty name
   IPC::ChannelHandle handle("");
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      handle, IPC::Channel::MODE_NAMED_SERVER, NULL, nullptr));
+      handle, IPC::Channel::MODE_NAMED_SERVER, NULL));
   ASSERT_FALSE(channel->Connect());
 
   // Test name that is too long.
@@ -378,7 +378,7 @@ TEST_F(IPCChannelPosixTest, BadChannelName) {
   EXPECT_GE(strlen(kTooLongName), IPC::kMaxSocketNameLength);
   IPC::ChannelHandle handle2(kTooLongName);
   scoped_ptr<IPC::ChannelPosix> channel2(new IPC::ChannelPosix(
-      handle2, IPC::Channel::MODE_NAMED_SERVER, NULL, nullptr));
+      handle2, IPC::Channel::MODE_NAMED_SERVER, NULL));
   EXPECT_FALSE(channel2->Connect());
 }
 
@@ -394,7 +394,7 @@ TEST_F(IPCChannelPosixTest, MAYBE_MultiConnection) {
   IPC::ChannelHandle chan_handle(GetConnectionSocketName());
   SetUpSocket(&chan_handle, IPC::Channel::MODE_NAMED_SERVER);
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      chan_handle, IPC::Channel::MODE_NAMED_SERVER, &listener, nullptr));
+      chan_handle, IPC::Channel::MODE_NAMED_SERVER, &listener));
   ASSERT_TRUE(channel->Connect());
   ASSERT_TRUE(channel->AcceptsConnections());
   ASSERT_FALSE(channel->HasAcceptedConnection());
@@ -430,9 +430,9 @@ TEST_F(IPCChannelPosixTest, DoubleServer) {
   IPCChannelPosixTestListener listener2(false);
   IPC::ChannelHandle chan_handle(GetConnectionSocketName());
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      chan_handle, IPC::Channel::MODE_SERVER, &listener, nullptr));
+      chan_handle, IPC::Channel::MODE_SERVER, &listener));
   scoped_ptr<IPC::ChannelPosix> channel2(new IPC::ChannelPosix(
-      chan_handle, IPC::Channel::MODE_SERVER, &listener2, nullptr));
+      chan_handle, IPC::Channel::MODE_SERVER, &listener2));
   ASSERT_TRUE(channel->Connect());
   ASSERT_FALSE(channel2->Connect());
 }
@@ -442,7 +442,7 @@ TEST_F(IPCChannelPosixTest, BadMode) {
   IPCChannelPosixTestListener listener(false);
   IPC::ChannelHandle chan_handle(GetConnectionSocketName());
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      chan_handle, IPC::Channel::MODE_NONE, &listener, nullptr));
+      chan_handle, IPC::Channel::MODE_NONE, &listener));
   ASSERT_FALSE(channel->Connect());
 }
 
@@ -454,7 +454,7 @@ TEST_F(IPCChannelPosixTest, IsNamedServerInitialized) {
   ASSERT_FALSE(IPC::Channel::IsNamedServerInitialized(
       connection_socket_name));
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      chan_handle, IPC::Channel::MODE_NAMED_SERVER, &listener, nullptr));
+      chan_handle, IPC::Channel::MODE_NAMED_SERVER, &listener));
   ASSERT_TRUE(IPC::Channel::IsNamedServerInitialized(
       connection_socket_name));
   channel->Close();
@@ -470,7 +470,7 @@ MULTIPROCESS_TEST_MAIN(IPCChannelPosixTestConnectionProc) {
   IPC::ChannelHandle handle(IPCChannelPosixTest::GetConnectionSocketName());
   IPCChannelPosixTest::SetUpSocket(&handle, IPC::Channel::MODE_NAMED_CLIENT);
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      handle, IPC::Channel::MODE_NAMED_CLIENT, &listener, nullptr));
+      handle, IPC::Channel::MODE_NAMED_CLIENT, &listener));
   EXPECT_TRUE(channel->Connect());
   IPCChannelPosixTest::SpinRunLoop(TestTimeouts::action_max_timeout());
   EXPECT_EQ(IPCChannelPosixTestListener::MESSAGE_RECEIVED, listener.status());
@@ -484,7 +484,7 @@ MULTIPROCESS_TEST_MAIN(IPCChannelPosixFailConnectionProc) {
   IPC::ChannelHandle handle(IPCChannelPosixTest::GetConnectionSocketName());
   IPCChannelPosixTest::SetUpSocket(&handle, IPC::Channel::MODE_NAMED_CLIENT);
   scoped_ptr<IPC::ChannelPosix> channel(new IPC::ChannelPosix(
-      handle, IPC::Channel::MODE_NAMED_CLIENT, &listener, nullptr));
+      handle, IPC::Channel::MODE_NAMED_CLIENT, &listener));
 
   // In this case connect may succeed or fail depending on if the packet
   // actually gets sent at sendmsg. Since we never delay on send, we may not
diff --git a/ipc/ipc_channel_win.cc b/ipc/ipc_channel_win.cc
index 4ac5823..79b0001 100644
--- a/ipc/ipc_channel_win.cc
+++ b/ipc/ipc_channel_win.cc
@@ -18,6 +18,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_checker.h"
 #include "base/win/scoped_handle.h"
+#include "ipc/attachment_broker.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_logging.h"
 #include "ipc/ipc_message_attachment_set.h"
@@ -38,8 +39,7 @@ ChannelWin::State::~State() {
 
 ChannelWin::ChannelWin(const IPC::ChannelHandle& channel_handle,
                        Mode mode,
-                       Listener* listener,
-                       AttachmentBroker* broker)
+                       Listener* listener)
     : ChannelReader(listener),
       input_state_(this),
       output_state_(this),
@@ -48,7 +48,6 @@ ChannelWin::ChannelWin(const IPC::ChannelHandle& channel_handle,
       processing_incoming_(false),
       validate_client_(false),
       client_secret_(0),
-      broker_(broker),
       weak_factory_(this) {
   CreatePipe(channel_handle, mode);
 }
@@ -107,11 +106,12 @@ bool ChannelWin::ProcessMessageForDelivery(Message* message) {
   // both Send() and ProcessMessageForDelivery() may be re-entrant. Brokered
   // attachments must be sent before the Message itself.
   if (message->HasBrokerableAttachments()) {
-    DCHECK(broker_);
+    DCHECK(GetAttachmentBroker());
     DCHECK(peer_pid_ != base::kNullProcessId);
     for (const BrokerableAttachment* attachment :
          message->attachment_set()->PeekBrokerableAttachments()) {
-      if (!broker_->SendAttachmentToProcess(attachment, peer_pid_)) {
+      if (!GetAttachmentBroker()->SendAttachmentToProcess(attachment,
+                                                          peer_pid_)) {
         delete message;
         return false;
       }
@@ -161,7 +161,7 @@ void ChannelWin::FlushPrelimQueue() {
 }
 
 AttachmentBroker* ChannelWin::GetAttachmentBroker() {
-  return broker_;
+  return AttachmentBroker::GetGlobal();
 }
 
 base::ProcessId ChannelWin::GetPeerPID() const {
@@ -567,7 +567,7 @@ scoped_ptr<Channel> Channel::Create(const IPC::ChannelHandle& channel_handle,
                                     Listener* listener,
                                     AttachmentBroker* broker) {
   return scoped_ptr<Channel>(
-      new ChannelWin(channel_handle, mode, listener, broker));
+      new ChannelWin(channel_handle, mode, listener));
 }
 
 // static
diff --git a/ipc/ipc_channel_win.h b/ipc/ipc_channel_win.h
index 579f7f7..35a158e 100644
--- a/ipc/ipc_channel_win.h
+++ b/ipc/ipc_channel_win.h
@@ -32,8 +32,7 @@ class ChannelWin : public Channel,
   // |broker| must outlive the newly created object.
   ChannelWin(const IPC::ChannelHandle& channel_handle,
              Mode mode,
-             Listener* listener,
-             AttachmentBroker* broker);
+             Listener* listener);
   ~ChannelWin() override;
 
   // Channel implementation
@@ -133,9 +132,6 @@ class ChannelWin : public Channel,
 
   scoped_ptr<base::ThreadChecker> thread_check_;
 
-  // |broker_| must outlive this instance.
-  AttachmentBroker* broker_;
-
   base::WeakPtrFactory<ChannelWin> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(ChannelWin);
 };