ipc: Slight change to GetSenderWithProcessId().

Previously, the implementation of the method acquired a lock. This means that
the caller could not guarantee that the returned Sender was still valid. I
changed the method to require that the caller acquire the lock, so that the
caller can guarantee the validity of the returned Sender.

BUG=561734

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

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

3 files changed, 16 insertions, 7 deletions

diff --git a/ipc/attachment_broker_privileged.cc b/ipc/attachment_broker_privileged.cc
index 7631001..a6de9e6 100644
--- a/ipc/attachment_broker_privileged.cc
+++ b/ipc/attachment_broker_privileged.cc
@@ -103,7 +103,7 @@ void AttachmentBrokerPrivileged::DeregisterCommunicationChannel(
 }
 
 Sender* AttachmentBrokerPrivileged::GetSenderWithProcessId(base::ProcessId id) {
-  base::AutoLock auto_lock(*get_lock());
+  get_lock()->AssertAcquired();
   auto it = std::find_if(endpoints_.begin(), endpoints_.end(),
                          [id](Endpoint* c) { return c->GetPeerPID() == id; });
   if (it == endpoints_.end())
diff --git a/ipc/attachment_broker_privileged.h b/ipc/attachment_broker_privileged.h
index cbaaaf2..a82d581a 100644
--- a/ipc/attachment_broker_privileged.h
+++ b/ipc/attachment_broker_privileged.h
@@ -47,6 +47,9 @@ class IPC_EXPORT AttachmentBrokerPrivileged : public IPC::AttachmentBroker {
  protected:
   // Returns the sender whose peer's process id is |id|.
   // Returns nullptr if no sender is found.
+  // The lock returned by get_lock() must already be acquired before calling
+  // this method. The return value is only guaranteed to be valid while the lock
+  // is held.
   Sender* GetSenderWithProcessId(base::ProcessId id);
 
   // Errors that can be reported by subclasses.
diff --git a/ipc/attachment_broker_privileged_mac.cc b/ipc/attachment_broker_privileged_mac.cc
index f4dc0439..62e218c 100644
--- a/ipc/attachment_broker_privileged_mac.cc
+++ b/ipc/attachment_broker_privileged_mac.cc
@@ -8,6 +8,7 @@
 #include "base/memory/shared_memory.h"
 #include "base/process/port_provider_mac.h"
 #include "base/process/process.h"
+#include "base/synchronization/lock.h"
 #include "ipc/attachment_broker_messages.h"
 #include "ipc/brokerable_attachment.h"
 #include "ipc/ipc_channel.h"
@@ -210,6 +211,7 @@ bool AttachmentBrokerPrivilegedMac::RouteWireFormatToAnother(
 
   // Another process is the destination.
   base::ProcessId dest = wire_format.destination_process;
+  base::AutoLock auto_lock(*get_lock());
   Sender* sender = GetSenderWithProcessId(dest);
   if (!sender) {
     // Assuming that this message was not sent from a malicious process, the
@@ -319,6 +321,7 @@ void AttachmentBrokerPrivilegedMac::SendPrecursorsForProcess(
   bool to_self = pid == base::GetCurrentProcId();
 
   if (!to_self) {
+    base::AutoLock auto_lock(*get_lock());
     if (!GetSenderWithProcessId(pid)) {
       // If there is no sender, then the destination process is no longer
       // running, or never existed to begin with.
@@ -387,12 +390,15 @@ void AttachmentBrokerPrivilegedMac::ProcessExtractorsForProcess(
   if (it == extractors_.end())
     return;
 
-  if (!GetSenderWithProcessId(pid)) {
-    // If there is no sender, then the source process is no longer running.
-    LogError(ERROR_SOURCE_NOT_FOUND);
-    delete it->second;
-    extractors_.erase(it);
-    return;
+  {
+    base::AutoLock auto_lock(*get_lock());
+    if (!GetSenderWithProcessId(pid)) {
+      // If there is no sender, then the source process is no longer running.
+      LogError(ERROR_SOURCE_NOT_FOUND);
+      delete it->second;
+      extractors_.erase(it);
+      return;
+    }
   }
 
   mach_port_t task_port = port_provider_->TaskForPid(pid);