Clean up public interface of AttachmentBrokerUnprivileged.

In the old interface, a static factory method returns a scoped_ptr, and the
caller had to manage the lifetime. Since this is a global object with minimal
memory footprint, and is required to outlive every IPC::Channel, it's much
easier for the global to never be destroyed. This also matches the interface for
AttachmentBrokerPrivileged.

BUG=584297

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

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

8 files changed, 95 insertions, 28 deletions

diff --git a/ipc/attachment_broker.cc b/ipc/attachment_broker.cc
index 50e3e2d9..9498215 100644
--- a/ipc/attachment_broker.cc
+++ b/ipc/attachment_broker.cc
@@ -82,6 +82,20 @@ void AttachmentBroker::DeregisterCommunicationChannel(Endpoint* endpoint) {
   NOTREACHED();
 }
 
+void AttachmentBroker::RegisterBrokerCommunicationChannel(Endpoint* endpoint) {
+  NOTREACHED();
+}
+
+void AttachmentBroker::DeregisterBrokerCommunicationChannel(
+    Endpoint* endpoint) {
+  NOTREACHED();
+}
+
+bool AttachmentBroker::IsPrivilegedBroker() {
+  NOTREACHED();
+  return false;
+}
+
 void AttachmentBroker::HandleReceivedAttachment(
     const scoped_refptr<BrokerableAttachment>& attachment) {
   {
diff --git a/ipc/attachment_broker.h b/ipc/attachment_broker.h
index 4cf1ab5..d936f44 100644
--- a/ipc/attachment_broker.h
+++ b/ipc/attachment_broker.h
@@ -97,6 +97,14 @@ class IPC_EXPORT AttachmentBroker : public Listener {
   virtual void RegisterCommunicationChannel(Endpoint* endpoint);
   virtual void DeregisterCommunicationChannel(Endpoint* endpoint);
 
+  // In each unprivileged process, exactly one channel should be used to
+  // communicate brokerable attachments with the broker process.
+  virtual void RegisterBrokerCommunicationChannel(Endpoint* endpoint);
+  virtual void DeregisterBrokerCommunicationChannel(Endpoint* endpoint);
+
+  // True if and only if this broker is privileged.
+  virtual bool IsPrivilegedBroker();
+
  protected:
   using AttachmentVector = std::vector<scoped_refptr<BrokerableAttachment>>;
 
diff --git a/ipc/attachment_broker_mac_unittest.cc b/ipc/attachment_broker_mac_unittest.cc
index bf994b7..9736c02 100644
--- a/ipc/attachment_broker_mac_unittest.cc
+++ b/ipc/attachment_broker_mac_unittest.cc
@@ -427,7 +427,7 @@ class IPCAttachmentBrokerMacTest : public IPCTestBase {
 
     broker_->AddObserver(&observer_, task_runner());
     CreateChannel(&proxy_listener_);
-    broker_->DesignateBrokerCommunicationChannel(channel());
+    broker_->RegisterBrokerCommunicationChannel(channel());
     ASSERT_TRUE(ConnectChannel());
     ASSERT_TRUE(StartClient());
 
@@ -972,7 +972,7 @@ TEST_F(IPCAttachmentBrokerMacTest, SendSharedMemoryHandleChannelProxy) {
   thread->StartWithOptions(options);
 
   CreateChannelProxy(get_proxy_listener(), thread->task_runner().get());
-  get_broker()->DesignateBrokerCommunicationChannel(channel_proxy());
+  get_broker()->RegisterBrokerCommunicationChannel(channel_proxy());
 
   ASSERT_TRUE(StartClient());
 
diff --git a/ipc/attachment_broker_privileged.cc b/ipc/attachment_broker_privileged.cc
index 85c1ac9..9006798 100644
--- a/ipc/attachment_broker_privileged.cc
+++ b/ipc/attachment_broker_privileged.cc
@@ -67,9 +67,7 @@ scoped_ptr<AttachmentBrokerPrivileged> CreateBroker() {
 // the global broker.
 class AttachmentBrokerMakeOnce {
  public:
-  AttachmentBrokerMakeOnce() {
-    attachment_broker_.reset(CreateBroker().release());
-  }
+  AttachmentBrokerMakeOnce() : attachment_broker_(CreateBroker()) {}
 
  private:
   scoped_ptr<IPC::AttachmentBrokerPrivileged> attachment_broker_;
@@ -128,6 +126,10 @@ void AttachmentBrokerPrivileged::DeregisterCommunicationChannel(
     endpoints_.erase(it);
 }
 
+bool AttachmentBrokerPrivileged::IsPrivilegedBroker() {
+  return true;
+}
+
 Sender* AttachmentBrokerPrivileged::GetSenderWithProcessId(base::ProcessId id) {
   get_lock()->AssertAcquired();
   auto it = std::find_if(endpoints_.begin(), endpoints_.end(),
diff --git a/ipc/attachment_broker_privileged.h b/ipc/attachment_broker_privileged.h
index 686bb9d..8b85599 100644
--- a/ipc/attachment_broker_privileged.h
+++ b/ipc/attachment_broker_privileged.h
@@ -50,6 +50,7 @@ class IPC_EXPORT AttachmentBrokerPrivileged : public IPC::AttachmentBroker {
   // AttachmentBroker overrides.
   void RegisterCommunicationChannel(Endpoint* endpoint) override;
   void DeregisterCommunicationChannel(Endpoint* endpoint) override;
+  bool IsPrivilegedBroker() override;
 
  protected:
   // Returns the sender whose peer's process id is |id|.
diff --git a/ipc/attachment_broker_privileged_win_unittest.cc b/ipc/attachment_broker_privileged_win_unittest.cc
index 07f37a3..870e262 100644
--- a/ipc/attachment_broker_privileged_win_unittest.cc
+++ b/ipc/attachment_broker_privileged_win_unittest.cc
@@ -267,7 +267,7 @@ class IPCAttachmentBrokerPrivilegedWinTest : public IPCTestBase {
       set_broker(new IPC::AttachmentBrokerUnprivilegedWin);
     broker_->AddObserver(&observer_, task_runner());
     CreateChannel(&proxy_listener_);
-    broker_->DesignateBrokerCommunicationChannel(channel());
+    broker_->RegisterBrokerCommunicationChannel(channel());
     ASSERT_TRUE(ConnectChannel());
     ASSERT_TRUE(StartClient());
 
diff --git a/ipc/attachment_broker_unprivileged.cc b/ipc/attachment_broker_unprivileged.cc
index 9286a89..c1e4c4a 100644
--- a/ipc/attachment_broker_unprivileged.cc
+++ b/ipc/attachment_broker_unprivileged.cc
@@ -4,6 +4,7 @@
 
 #include "ipc/attachment_broker_unprivileged.h"
 
+#include "base/lazy_instance.h"
 #include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
 #include "ipc/ipc_channel.h"
@@ -19,18 +20,15 @@
 
 namespace IPC {
 
-AttachmentBrokerUnprivileged::AttachmentBrokerUnprivileged()
-    : sender_(nullptr) {
-  IPC::AttachmentBroker::SetGlobal(this);
-}
-
-AttachmentBrokerUnprivileged::~AttachmentBrokerUnprivileged() {
-  IPC::AttachmentBroker::SetGlobal(nullptr);
-}
+namespace {
 
-// static
-scoped_ptr<AttachmentBrokerUnprivileged>
-AttachmentBrokerUnprivileged::CreateBroker() {
+// On platforms that support attachment brokering, returns a new instance of
+// a platform-specific attachment broker. Otherwise returns |nullptr|.
+// The caller takes ownership of the newly created instance, and is
+// responsible for ensuring that the attachment broker lives longer than
+// every IPC::Channel. The new instance automatically registers itself as the
+// global attachment broker.
+scoped_ptr<AttachmentBrokerUnprivileged> CreateBroker() {
 #if defined(OS_WIN)
   return scoped_ptr<AttachmentBrokerUnprivileged>(
       new IPC::AttachmentBrokerUnprivilegedWin);
@@ -42,7 +40,42 @@ AttachmentBrokerUnprivileged::CreateBroker() {
 #endif
 }
 
-void AttachmentBrokerUnprivileged::DesignateBrokerCommunicationChannel(
+// This class is wrapped in a LazyInstance to ensure that its constructor is
+// only called once. The constructor creates an attachment broker and sets it as
+// the global broker.
+class AttachmentBrokerMakeOnce {
+ public:
+  AttachmentBrokerMakeOnce() {
+    // Single process tests can cause an attachment broker to already exist.
+    if (AttachmentBroker::GetGlobal())
+      return;
+    attachment_broker_ = CreateBroker();
+  }
+
+ private:
+  scoped_ptr<IPC::AttachmentBrokerUnprivileged> attachment_broker_;
+};
+
+base::LazyInstance<AttachmentBrokerMakeOnce>::Leaky
+    g_attachment_broker_make_once = LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+AttachmentBrokerUnprivileged::AttachmentBrokerUnprivileged()
+    : sender_(nullptr) {
+  IPC::AttachmentBroker::SetGlobal(this);
+}
+
+AttachmentBrokerUnprivileged::~AttachmentBrokerUnprivileged() {
+  IPC::AttachmentBroker::SetGlobal(nullptr);
+}
+
+// static
+void AttachmentBrokerUnprivileged::CreateBrokerIfNeeded() {
+  g_attachment_broker_make_once.Get();
+}
+
+void AttachmentBrokerUnprivileged::RegisterBrokerCommunicationChannel(
     Endpoint* endpoint) {
   DCHECK(endpoint);
   DCHECK(!sender_);
@@ -50,6 +83,17 @@ void AttachmentBrokerUnprivileged::DesignateBrokerCommunicationChannel(
   endpoint->SetAttachmentBrokerEndpoint(true);
 }
 
+void AttachmentBrokerUnprivileged::DeregisterBrokerCommunicationChannel(
+    Endpoint* endpoint) {
+  DCHECK(endpoint);
+  DCHECK_EQ(endpoint, sender_);
+  sender_ = nullptr;
+}
+
+bool AttachmentBrokerUnprivileged::IsPrivilegedBroker() {
+  return false;
+}
+
 void AttachmentBrokerUnprivileged::LogError(UMAError error) {
   UMA_HISTOGRAM_ENUMERATION(
       "IPC.AttachmentBrokerUnprivileged.BrokerAttachmentError", error,
diff --git a/ipc/attachment_broker_unprivileged.h b/ipc/attachment_broker_unprivileged.h
index b572ff8..f6d520d 100644
--- a/ipc/attachment_broker_unprivileged.h
+++ b/ipc/attachment_broker_unprivileged.h
@@ -22,17 +22,15 @@ class IPC_EXPORT AttachmentBrokerUnprivileged : public IPC::AttachmentBroker {
   AttachmentBrokerUnprivileged();
   ~AttachmentBrokerUnprivileged() override;
 
-   // On platforms that support attachment brokering, returns a new instance of
-   // a platform-specific attachment broker. Otherwise returns |nullptr|.
-   // The caller takes ownership of the newly created instance, and is
-   // responsible for ensuring that the attachment broker lives longer than
-   // every IPC::Channel. The new instance automatically registers itself as the
-   // global attachment broker.
-  static scoped_ptr<AttachmentBrokerUnprivileged> CreateBroker();
+  // If there is no global attachment broker, makes a new
+  // AttachmentBrokerUnprivileged and sets it as the global attachment broker.
+  // This method is thread safe.
+  static void CreateBrokerIfNeeded();
 
-  // In each unprivileged process, exactly one channel should be used to
-  // communicate brokerable attachments with the broker process.
-  void DesignateBrokerCommunicationChannel(Endpoint* endpoint);
+  // AttachmentBroker:
+  void RegisterBrokerCommunicationChannel(Endpoint* endpoint) override;
+  void DeregisterBrokerCommunicationChannel(Endpoint* endpoint) override;
+  bool IsPrivilegedBroker() override;
 
  protected:
   IPC::Sender* get_sender() { return sender_; }