This patch caused Chrome to be unable to load any web pages on Chrome…

… OS. BUG=chromium-os:19468 TEST=confirm chrome loads pages Revert "Fix IPC OnChannelConnected() to send correct PID on Linux/CrOS" This reverts commit 92321e0. Review URL: http://codereview.chromium.org/7712022 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@97811 0039d316-1c4b-4281-b951-d872f2087c98
Denger-Network · Aug 23, 2011 · bf84c58 · bf84c58
1 parent 09ba326
commit bf84c58
Show file tree

Hide file tree

Showing 11 changed files with 19 additions and 146 deletions.
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
@@ -163,8 +163,7 @@ bool AutomationProvider::InitializeChannel(const std::string& channel_id) {
       effective_channel_id,
       GetChannelMode(use_named_interface),
       this,
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
-      false /* needs_override_peer_pid */));
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)));
   channel_->AddFilter(automation_resource_message_filter_);
 
 #if defined(OS_CHROMEOS)

diff --git a/chrome/browser/service/service_process_control.cc b/chrome/browser/service/service_process_control.cc
@@ -47,8 +47,7 @@ void ServiceProcessControl::ConnectInternal() {
   const IPC::ChannelHandle channel_id = GetServiceProcessChannel();
   channel_.reset(new IPC::ChannelProxy(
       channel_id, IPC::Channel::MODE_NAMED_CLIENT, this,
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
-      false /* needs_override_peer_pid */));
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)));
 }
 
 void ServiceProcessControl::RunConnectDoneTasks() {

diff --git a/content/browser/browser_child_process_host.cc b/content/browser/browser_child_process_host.cc
@@ -83,10 +83,6 @@ void BrowserChildProcessHost::Launch(
   content::GetContentClient()->browser()->AppendExtraCommandLineSwitches(
       cmd_line, id());
 
-#if defined(OS_LINUX)
-  channel()->SetNeedsOverridePeerPid();
-#endif
-
   child_process_.reset(new ChildProcessLauncher(
 #if defined(OS_WIN)
       exposed_dir,
@@ -171,16 +167,11 @@ BrowserChildProcessHost::ClientHook::ClientHook(BrowserChildProcessHost* host)
 }
 
 void BrowserChildProcessHost::ClientHook::OnProcessLaunched() {
-  base::ProcessHandle child_handle = host_->child_process_->GetHandle();
-  if (!child_handle) {
+  if (!host_->child_process_->GetHandle()) {
     host_->OnChildDied();
     return;
   }
-  host_->set_handle(child_handle);
-#if defined(OS_LINUX)
-  int32 child_pid = base::GetProcId(child_handle);
-  host_->channel()->OverridePeerPid(child_pid);
-#endif
+  host_->set_handle(host_->child_process_->GetHandle());
   host_->OnProcessLaunched();
 }
 

diff --git a/content/browser/renderer_host/browser_render_process_host.cc b/content/browser/renderer_host/browser_render_process_host.cc
@@ -280,16 +280,9 @@ bool BrowserRenderProcessHost::Init(bool is_accessibility_enabled) {
   // Setup the IPC channel.
   const std::string channel_id =
       ChildProcessInfo::GenerateRandomChannelID(this);
-#if defined(OS_LINUX)
-  // See IPC::Channel::SetNeedsOverridePeerPid() for details.
-  const bool needs_override_peer_pid = true;
-#else
-  const bool needs_override_peer_pid = false;
-#endif
   channel_.reset(new IPC::ChannelProxy(
       channel_id, IPC::Channel::MODE_SERVER, this,
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
-      needs_override_peer_pid));
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)));
 
   // Call the embedder first so that their IPC filters have priority.
   content::GetContentClient()->browser()->BrowserRenderProcessHostCreated(this);
@@ -898,17 +891,8 @@ void BrowserRenderProcessHost::OnProcessLaunched() {
   if (deleting_soon_)
     return;
 
-  if (child_process_launcher_.get()) {
+  if (child_process_launcher_.get())
     child_process_launcher_->SetProcessBackgrounded(backgrounded_);
-#if defined(OS_LINUX)
-    // Inform the IPC subsystem of the global PID for this sandboxed renderer.
-    if (channel_.get()) {
-      base::ProcessHandle child_handle = child_process_launcher_->GetHandle();
-      base::ProcessId child_pid = base::GetProcId(child_handle);
-      channel_->OverridePeerPid(child_pid);
-    }
-#endif
-  }
 
   if (max_page_id_ != -1)
     Send(new ViewMsg_SetNextPageID(max_page_id_ + 1));

diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h
@@ -171,19 +171,6 @@ class IPC_EXPORT Channel : public Message::Sender {
   void ResetToAcceptingConnectionState();
 #endif  // defined(OS_POSIX) && !defined(OS_NACL)
 
-#if defined(OS_LINUX)
-  // Configures the channel to defer OnChannelConnected() until we know the
-  // global PID of the peer.  On Linux, with sandboxed renderers, the browser
-  // cannot use the process id sent by the renderer in the hello message,
-  // because the renderer is in its own private PID namespace.  With these
-  // renderers we need to defer our call to OnChannelConnected(peer_pid) until
-  // we know the global PID.
-  void SetNeedsOverridePeerPid();
-
-  // Overrides the peer PID and calls OnChannelConnected() if necessary.
-  void OverridePeerPid(int32 peer_pid);
-#endif  // defined(OS_LINUX)
-
   // Returns true if a named server channel is initialized on the given channel
   // ID. Even if true, the server may have already accepted a connection.
   static bool IsNamedServerInitialized(const std::string& channel_id);

diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
@@ -309,9 +309,7 @@ Channel::ChannelImpl::ChannelImpl(const IPC::ChannelHandle& channel_handle,
 #endif  // IPC_USES_READWRITE
       pipe_name_(channel_handle.name),
       listener_(listener),
-      must_unlink_(false),
-      needs_override_peer_pid_(false),
-      override_peer_pid_(0) {
+      must_unlink_(false) {
   memset(input_buf_, 0, sizeof(input_buf_));
   memset(input_cmsg_buf_, 0, sizeof(input_cmsg_buf_));
   if (!CreatePipe(channel_handle)) {
@@ -729,14 +727,7 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() {
             CHECK(descriptor.auto_close);
           }
 #endif  // IPC_USES_READWRITE
-          if (needs_override_peer_pid_) {
-            // If we already have the peer PID, use it.  Otherwise we'll call
-            // OnChannelConnected() in OverridePeerPid() below.
-            if (override_peer_pid_ != 0)
-              listener_->OnChannelConnected(override_peer_pid_);
-          } else {
-            listener_->OnChannelConnected(pid);
-          }
+          listener_->OnChannelConnected(pid);
         } else {
           listener_->OnMessageReceived(m);
         }
@@ -1002,27 +993,6 @@ void Channel::ChannelImpl::ResetToAcceptingConnectionState() {
   input_overflow_fds_.clear();
 }
 
-#if defined(OS_LINUX)
-void Channel::ChannelImpl::SetNeedsOverridePeerPid() {
-  needs_override_peer_pid_ = true;
-}
-
-void Channel::ChannelImpl::OverridePeerPid(int32 peer_pid) {
-  DCHECK(needs_override_peer_pid_);
-  override_peer_pid_ = peer_pid;
-
-  // The browser learns the global PID of the renderers on the UI thread, and
-  // must post the data to the IO thread for us to use it here.  Therefore
-  // there is a race between the IPC channel processing the hello message
-  // and this function being called.  If fd_pipe_ != -1 then we've already
-  // received the hello message and we skipped OnChannelConnected() above,
-  // so call it here.
-  if (fd_pipe_ != -1) {
-    listener_->OnChannelConnected(peer_pid);
-  }
-}
-#endif  // defined(OS_LINUX)
-
 // static
 bool Channel::ChannelImpl::IsNamedServerInitialized(
     const std::string& channel_id) {
@@ -1238,16 +1208,6 @@ void Channel::ResetToAcceptingConnectionState() {
   channel_impl_->ResetToAcceptingConnectionState();
 }
 
-#if defined(OS_LINUX)
-void Channel::SetNeedsOverridePeerPid() {
-  channel_impl_->SetNeedsOverridePeerPid();
-}
-
-void Channel::OverridePeerPid(int32 peer_pid) {
-  channel_impl_->OverridePeerPid(peer_pid);
-}
-#endif  // defined(OS_LINUX)
-
 // static
 bool Channel::IsNamedServerInitialized(const std::string& channel_id) {
   return ChannelImpl::IsNamedServerInitialized(channel_id);

diff --git a/ipc/ipc_channel_posix.h b/ipc/ipc_channel_posix.h
@@ -62,10 +62,6 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   bool HasAcceptedConnection() const;
   bool GetClientEuid(uid_t* client_euid) const;
   void ResetToAcceptingConnectionState();
-#if defined(OS_LINUX)
-  void SetNeedsOverridePeerPid();
-  void OverridePeerPid(int32 peer_pid);
-#endif  // defined(OS_LINUX)
   static bool IsNamedServerInitialized(const std::string& channel_id);
 
  private:
@@ -151,10 +147,6 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   // True if we are responsible for unlinking the unix domain socket file.
   bool must_unlink_;
 
-  // See IPC::Channel::SetNeedsOverridePeerPid() header comment for details.
-  bool needs_override_peer_pid_;
-  int32 override_peer_pid_;
-
   DISALLOW_IMPLICIT_CONSTRUCTORS(ChannelImpl);
 };
 

diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
@@ -71,15 +71,10 @@ ChannelProxy::Context::~Context() {
 }
 
 void ChannelProxy::Context::CreateChannel(const IPC::ChannelHandle& handle,
-                                          const Channel::Mode& mode,
-                                          bool needs_override_peer_pid) {
+                                          const Channel::Mode& mode) {
   DCHECK(channel_.get() == NULL);
   channel_id_ = handle.name;
   channel_.reset(new Channel(handle, mode, this));
-#if defined(OS_LINUX)
-  if (needs_override_peer_pid)
-    channel_->SetNeedsOverridePeerPid();
-#endif
 }
 
 bool ChannelProxy::Context::TryFilters(const Message& message) {
@@ -231,14 +226,6 @@ void ChannelProxy::Context::OnRemoveFilter(MessageFilter* filter) {
   NOTREACHED() << "filter to be removed not found";
 }
 
-#if defined(OS_LINUX)
-// Called on the IPC::Channel thread
-void ChannelProxy::Context::OnOverridePeerPid(int32 peer_pid) {
-  if (channel_.get())
-    channel_->OverridePeerPid(peer_pid);
-}
-#endif  // defined(OS_LINUX)
-
 // Called on the listener's thread
 void ChannelProxy::Context::AddFilter(MessageFilter* filter) {
   base::AutoLock auto_lock(pending_filters_lock_);
@@ -295,23 +282,20 @@ void ChannelProxy::Context::OnDispatchError() {
 ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
                            Channel::Mode mode,
                            Channel::Listener* listener,
-                           base::MessageLoopProxy* ipc_thread,
-                           bool needs_override_peer_pid)
+                           base::MessageLoopProxy* ipc_thread)
     : context_(new Context(listener, ipc_thread)),
       outgoing_message_filter_(NULL) {
-  Init(channel_handle, mode, ipc_thread, true, needs_override_peer_pid);
+  Init(channel_handle, mode, ipc_thread, true);
 }
 
 ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
                            Channel::Mode mode,
                            base::MessageLoopProxy* ipc_thread,
-                           bool needs_override_peer_pid,
                            Context* context,
                            bool create_pipe_now)
     : context_(context),
       outgoing_message_filter_(NULL) {
-  Init(channel_handle, mode, ipc_thread, create_pipe_now,
-       needs_override_peer_pid);
+  Init(channel_handle, mode, ipc_thread, create_pipe_now);
 }
 
 ChannelProxy::~ChannelProxy() {
@@ -321,8 +305,7 @@ ChannelProxy::~ChannelProxy() {
 void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
                         Channel::Mode mode,
                         base::MessageLoopProxy* ipc_thread_loop,
-                        bool create_pipe_now,
-                        bool needs_override_peer_pid) {
+                        bool create_pipe_now) {
 #if defined(OS_POSIX)
   // When we are creating a server on POSIX, we need its file descriptor
   // to be created immediately so that it can be accessed and passed
@@ -338,11 +321,10 @@ void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
     // low-level pipe so that the client can connect.  Without creating
     // the pipe immediately, it is possible for a listener to attempt
     // to connect and get an error since the pipe doesn't exist yet.
-    context_->CreateChannel(channel_handle, mode, needs_override_peer_pid);
+    context_->CreateChannel(channel_handle, mode);
   } else {
     context_->ipc_message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
-        context_.get(), &Context::CreateChannel, channel_handle, mode,
-        needs_override_peer_pid));
+        context_.get(), &Context::CreateChannel, channel_handle, mode));
   }
 
   // complete initialization on the background thread
@@ -410,13 +392,6 @@ bool ChannelProxy::GetClientEuid(uid_t* client_euid) const {
 }
 #endif
 
-#if defined(OS_LINUX)
-void ChannelProxy::OverridePeerPid(int32 peer_pid) {
-  context_->ipc_message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
-      context_.get(), &Context::OnOverridePeerPid, peer_pid));
-}
-#endif  // defined(OS_LINUX)
-
 //-----------------------------------------------------------------------------
 
 }  // namespace IPC
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
@@ -118,8 +118,7 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
   ChannelProxy(const IPC::ChannelHandle& channel_handle,
                Channel::Mode mode,
                Channel::Listener* listener,
-               base::MessageLoopProxy* ipc_thread_loop,
-               bool needs_override_peer_pid);
+               base::MessageLoopProxy* ipc_thread_loop);
 
   virtual ~ChannelProxy();
 
@@ -162,11 +161,6 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
   bool GetClientEuid(uid_t* client_euid) const;
 #endif  // defined(OS_POSIX)
 
-#if defined(OS_LINUX)
-  // Calls through to the underlying channel's method.
-  void OverridePeerPid(int32 peer_pid);
-#endif  // defined(OS_LINUX)
-
  protected:
   class Context;
   // A subclass uses this constructor if it needs to add more information
@@ -175,7 +169,6 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
   ChannelProxy(const IPC::ChannelHandle& channel_handle,
                Channel::Mode mode,
                base::MessageLoopProxy* ipc_thread_loop,
-               bool needs_override_peer_pid,
                Context* context,
                bool create_pipe_now);
 
@@ -224,16 +217,12 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
 
     // Create the Channel
     void CreateChannel(const IPC::ChannelHandle& channel_handle,
-                       const Channel::Mode& mode,
-                       bool needs_override_peer_pid);
+                       const Channel::Mode& mode);
 
     // Methods called on the IO thread.
     void OnSendMessage(Message* message_ptr);
     void OnAddFilter();
     void OnRemoveFilter(MessageFilter* filter);
-#if defined(OS_LINUX)
-    void OnOverridePeerPid(int32 peer_pid);
-#endif
 
     // Methods called on the listener thread.
     void AddFilter(MessageFilter* filter);
@@ -268,8 +257,7 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
   friend class SendTask;
 
   void Init(const IPC::ChannelHandle& channel_handle, Channel::Mode mode,
-            base::MessageLoopProxy* ipc_thread_loop, bool create_pipe_now,
-            bool needs_override_peer_pid);
+            base::MessageLoopProxy* ipc_thread_loop, bool create_pipe_now);
 
   // By maintaining this indirection (ref-counted) to our internal state, we
   // can safely be destroyed while the background thread continues to do stuff

diff --git a/ipc/ipc_sync_channel.cc b/ipc/ipc_sync_channel.cc
@@ -377,7 +377,6 @@ SyncChannel::SyncChannel(
     WaitableEvent* shutdown_event)
     : ChannelProxy(
           channel_handle, mode, ipc_message_loop,
-          false /* needs_override_peer_pid */,
           new SyncContext(listener, ipc_message_loop, shutdown_event),
           create_pipe_now),
       sync_messages_with_no_timeout_allowed_(true) {

diff --git a/ipc/ipc_tests.cc b/ipc/ipc_tests.cc
@@ -251,8 +251,7 @@ TEST_F(IPCChannelTest, ChannelProxyTest) {
   {
     // setup IPC channel proxy
     IPC::ChannelProxy chan(kTestClientChannel, IPC::Channel::MODE_SERVER,
-                           &channel_listener, thread.message_loop_proxy(),
-                           false /* needs_override_peer_pid */);
+                           &channel_listener, thread.message_loop_proxy());
 
     channel_listener.Init(&chan);