Reland chromium#1: Clean up interface of attachment broker.

I forgot to initialize a member variable, which is why the original CL flakily
succeeded (and managed to pass the CQ).

Original issue's description:
> ipc: Clean up interface of attachment broker.
>
> AttachmentBrokerUnprivileged now has a method
> DesignateBrokerCommunicationChannel which is used by non-broker processes to
> designate an IPC::Channel as the communication medium for brokerable attachment
> messages. IPC::Channel has a new member attachment_broker_endpoint_ which causes
> it to pass all messages through the attachment broker before forwarding them to
> the listener.
>
> BUG=493414
>
> Committed: https://crrev.com/9a06836982214f2edced21bbd1615b49e7f231bf
> Cr-Commit-Position: refs/heads/master@{#341143}

BUG=493414
TBR=tsepez@chromium.org

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

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

ipc/attachment_broker_privileged.cc

@@ -16,6 +16,7 @@ AttachmentBrokerPrivileged::~AttachmentBrokerPrivileged() {}
 
 void AttachmentBrokerPrivileged::RegisterCommunicationChannel(
     Channel* channel) {
+  channel->set_attachment_broker_endpoint(true);
   auto it = std::find(channels_.begin(), channels_.end(), channel);
   DCHECK(channels_.end() == it);
   channels_.push_back(channel);

ipc/attachment_broker_privileged_win_unittest.cc

@@ -161,7 +161,7 @@ class IPCAttachmentBrokerPrivilegedWinTest : public IPCTestBase {
     broker_->AddObserver(&observer_);
     set_attachment_broker(broker_.get());
     CreateChannel(&proxy_listener_);
-    broker_->set_sender(channel());
+    broker_->DesignateBrokerCommunicationChannel(channel());
     ASSERT_TRUE(ConnectChannel());
     ASSERT_TRUE(StartClient());
   }
@@ -278,6 +278,9 @@ TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendHandleToSelf) {
 
   set_broker(new MockBroker);
   CommonSetUp();
+  // Technically, the channel is an endpoint, but we need the proxy listener to
+  // receive the messages so that it can quit the message loop.
+  channel()->set_attachment_broker_endpoint(false);
   get_proxy_listener()->set_listener(get_broker());
 
   HANDLE h = CreateTempFile();

ipc/attachment_broker_unprivileged.cc

@@ -4,10 +4,21 @@
 
 #include "ipc/attachment_broker_unprivileged.h"
 
+#include "ipc/ipc_channel.h"
+
 namespace IPC {
 
-AttachmentBrokerUnprivileged::AttachmentBrokerUnprivileged() {}
+AttachmentBrokerUnprivileged::AttachmentBrokerUnprivileged()
+    : sender_(nullptr) {}
 
 AttachmentBrokerUnprivileged::~AttachmentBrokerUnprivileged() {}
 
+void AttachmentBrokerUnprivileged::DesignateBrokerCommunicationChannel(
+    IPC::Channel* channel) {
+  DCHECK(channel);
+  DCHECK(!sender_);
+  sender_ = channel;
+  channel->set_attachment_broker_endpoint(true);
+}
+
 }  // namespace IPC

ipc/attachment_broker_unprivileged.h

@@ -10,6 +10,7 @@
 
 namespace IPC {
 
+class Channel;
 class Sender;
 
 // This abstract subclass of AttachmentBroker is intended for use in
@@ -19,7 +20,9 @@ class IPC_EXPORT AttachmentBrokerUnprivileged : public IPC::AttachmentBroker {
   AttachmentBrokerUnprivileged();
   ~AttachmentBrokerUnprivileged() override;
 
-  void set_sender(IPC::Sender* sender) { sender_ = sender; }
+  // In each unprivileged process, exactly one channel should be used to
+  // communicate brokerable attachments with the broker process.
+  void DesignateBrokerCommunicationChannel(IPC::Channel* channel);
 
  protected:
   IPC::Sender* get_sender() { return sender_; }

ipc/ipc_channel.h

@@ -165,6 +165,7 @@ class IPC_EXPORT Channel : public Sender {
       Listener* listener,
       AttachmentBroker* broker = nullptr);
 
+  Channel() : attachment_broker_endpoint_(false) {}
   ~Channel() override;
 
   // Connect the pipe.  On the server side, this will initiate
@@ -252,6 +253,17 @@ class IPC_EXPORT Channel : public Sender {
   static void NotifyProcessForkedForTesting();
 #endif
 
+  void set_attachment_broker_endpoint(bool is_endpoint) {
+    attachment_broker_endpoint_ = is_endpoint;
+  }
+
+ protected:
+  bool is_attachment_broker_endpoint() { return attachment_broker_endpoint_; }
+
+ private:
+  // Whether this channel is used as an endpoint for sending and receiving
+  // brokerable attachment messages to/from the broker process.
+  bool attachment_broker_endpoint_;
 };
 
 #if defined(OS_POSIX)

ipc/ipc_channel_nacl.cc

@@ -385,6 +385,10 @@ base::ProcessId ChannelNacl::GetSenderPID() {
   return base::kNullProcessId;
 }
 
+bool ChannelNacl::IsAttachmentBrokerEndpoint() {
+  return is_attachment_broker_endpoint();
+}
+
 // Channel's methods
 
 // static

ipc/ipc_channel_nacl.h

@@ -70,6 +70,7 @@ class ChannelNacl : public Channel,
   bool DidEmptyInputBuffers() override;
   void HandleInternalMessage(const Message& msg) override;
   base::ProcessId GetSenderPID() override;
+  bool IsAttachmentBrokerEndpoint() override;
 
   Mode mode_;
   bool waiting_connect_;

ipc/ipc_channel_posix.cc

@@ -955,6 +955,10 @@ base::ProcessId ChannelPosix::GetSenderPID() {
   return GetPeerPID();
 }
 
+bool ChannelPosix::IsAttachmentBrokerEndpoint() {
+  return is_attachment_broker_endpoint();
+}
+
 void ChannelPosix::Close() {
   // Close can be called multiple time, so we need to make sure we're
   // idempotent.

ipc/ipc_channel_posix.h

@@ -84,6 +84,7 @@ class IPC_EXPORT ChannelPosix : public Channel,
   bool DidEmptyInputBuffers() override;
   void HandleInternalMessage(const Message& msg) override;
   base::ProcessId GetSenderPID() override;
+  bool IsAttachmentBrokerEndpoint() override;
 
   // Finds the set of file descriptors in the given message.  On success,
   // appends the descriptors to the input_fds_ member and returns true

ipc/ipc_channel_reader.cc

@@ -163,9 +163,18 @@ void ChannelReader::DispatchMessage(Message* m) {
                IPC_MESSAGE_ID_LINE(m->type()));
 #endif
   m->TraceMessageEnd();
-  if (IsInternalMessage(*m))
+
+  bool handled = false;
+  if (IsInternalMessage(*m)) {
     HandleInternalMessage(*m);
-  else
+    handled = true;
+  }
+#if USE_ATTACHMENT_BROKER
+  if (!handled && IsAttachmentBrokerEndpoint() && GetAttachmentBroker()) {
+    handled = GetAttachmentBroker()->OnMessageReceived(*m);
+  }
+#endif  // USE_ATTACHMENT_BROKER
+  if (!handled)
     listener_->OnMessageReceived(*m);
   if (m->dispatch_error())
     listener_->OnBadMessageReceived(*m);

ipc/ipc_channel_reader.h

@@ -117,6 +117,9 @@ class IPC_EXPORT ChannelReader : public SupportsAttachmentBrokering,
   // Get the process ID for the sender of the message.
   virtual base::ProcessId GetSenderPID() = 0;
 
+  // Whether the channel is an endpoint of attachment brokering.
+  virtual bool IsAttachmentBrokerEndpoint() = 0;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(ChannelReaderTest, AttachmentAlreadyBrokered);
   FRIEND_TEST_ALL_PREFIXES(ChannelReaderTest, AttachmentNotYetBrokered);

ipc/ipc_channel_reader_unittest.cc

@@ -81,6 +81,8 @@ class MockChannelReader : public ChannelReader {
 
   base::ProcessId GetSenderPID() override { return base::kNullProcessId; }
 
+  bool IsAttachmentBrokerEndpoint() override { return false; }
+
   AttachmentBroker* GetAttachmentBroker() override { return broker_; }
 
   // This instance takes ownership of |m|.

ipc/ipc_channel_win.cc

@@ -247,6 +247,10 @@ base::ProcessId ChannelWin::GetSenderPID() {
   return GetPeerPID();
 }
 
+bool ChannelWin::IsAttachmentBrokerEndpoint() {
+  return is_attachment_broker_endpoint();
+}
+
 bool ChannelWin::DidEmptyInputBuffers() {
   // We don't need to do anything here.
   return true;

ipc/ipc_channel_win.h

@@ -53,6 +53,7 @@ class ChannelWin : public Channel,
   bool DidEmptyInputBuffers() override;
   void HandleInternalMessage(const Message& msg) override;
   base::ProcessId GetSenderPID() override;
+  bool IsAttachmentBrokerEndpoint() override;
 
   static const base::string16 PipeName(const std::string& channel_id,
                                        int32* secret);