Replace the RenderProcessHost.PID function that returns the OS-generated

process ID with an internally-generated id() function. This allows us the
guarantee that the IDs are unique over the entire run of the application.

This also cleans up some code associated with managing the PID.

The main potentially interesting change is now the PID is set uniquely for
every creation of RenderProcessHost. It used to be set cleared if the process
went away, and re-set if the process was re-created. The ID generation is in
ChildProcesInfo so it is also unique between workers and plugins. I had to
change some significant things in resource_dispatcher_host_unittest to take
into account this new generation of IDs.

BUG=17828
TEST=none
Review URL: http://codereview.chromium.org/160203

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24899 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 80 insertions, 58 deletions

diff --git a/chrome/common/child_process_host.cc b/chrome/common/child_process_host.cc
index cf67d15..5ac925a 100644
--- a/chrome/common/child_process_host.cc
+++ b/chrome/common/child_process_host.cc
@@ -65,7 +65,7 @@ class ChildNotificationTask : public Task {
 
 ChildProcessHost::ChildProcessHost(
     ProcessType type, ResourceDispatcherHost* resource_dispatcher_host)
-    : Receiver(type),
+    : Receiver(type, -1),
       ALLOW_THIS_IN_INITIALIZER_LIST(listener_(this)),
       resource_dispatcher_host_(resource_dispatcher_host),
       opening_channel_(false) {
@@ -76,7 +76,7 @@ ChildProcessHost::ChildProcessHost(
 ChildProcessHost::~ChildProcessHost() {
   Singleton<ChildProcessList>::get()->remove(this);
 
-  resource_dispatcher_host_->CancelRequestsForProcess(GetProcessId());
+  resource_dispatcher_host_->CancelRequestsForProcess(id());
 
   if (handle())
     ProcessWatcher::EnsureProcessTerminated(handle());
diff --git a/chrome/common/child_process_host.h b/chrome/common/child_process_host.h
index cb833dc..778b5a2 100644
--- a/chrome/common/child_process_host.h
+++ b/chrome/common/child_process_host.h
@@ -18,6 +18,9 @@ class NotificationType;
 
 // Plugins/workers and other child processes that live on the IO thread should
 // derive from this class.
+//
+// [Browser]RenderProcessHost is the main exception that doesn't derive from
+// this class. That project lives on the UI thread.
 class ChildProcessHost : public ResourceDispatcherHost::Receiver,
                          public IPC::Channel::Listener {
  public:
diff --git a/chrome/common/child_process_info.cc b/chrome/common/child_process_info.cc
index 819b7fc..770c9866 100644
--- a/chrome/common/child_process_info.cc
+++ b/chrome/common/child_process_info.cc
@@ -7,12 +7,34 @@
 #include <limits>
 
 #include "app/l10n_util.h"
+#include "base/atomicops.h"
 #include "base/logging.h"
 #include "base/process_util.h"
 #include "base/rand_util.h"
 #include "base/string_util.h"
 #include "grit/generated_resources.h"
 
+ChildProcessInfo::ChildProcessInfo(const ChildProcessInfo& original)
+    : type_(original.type_),
+      name_(original.name_),
+      id_(original.id_),
+      process_(original.process_) {
+}
+
+ChildProcessInfo::~ChildProcessInfo() {
+}
+
+ChildProcessInfo& ChildProcessInfo::operator=(
+    const ChildProcessInfo& original) {
+  if (&original != this) {
+    type_ = original.type_;
+    name_ = original.name_;
+    id_ = original.id_;
+    process_ = original.process_;
+  }
+  return *this;
+}
+
 std::wstring ChildProcessInfo::GetTypeNameInEnglish(
     ChildProcessInfo::ProcessType type) {
   switch (type) {
@@ -56,17 +78,11 @@ std::wstring ChildProcessInfo::GetLocalizedTitle() const {
   return l10n_util::GetStringF(message_id, title);
 }
 
-ChildProcessInfo::ChildProcessInfo(ProcessType type) {
-  // This constructor is only used by objects which derive from this class,
-  // which means *this* is a real object that refers to a child process, and not
-  // just a simple object that contains information about it.  So add it to our
-  // list of running processes.
-  type_ = type;
-  pid_ = -1;
-}
-
-
-ChildProcessInfo::~ChildProcessInfo() {
+ChildProcessInfo::ChildProcessInfo(ProcessType type, int id) : type_(type) {
+  if (id == -1)
+    id_ = GenerateChildProcessUniqueId();
+  else
+    id_ = id;
 }
 
 std::string ChildProcessInfo::GenerateRandomChannelID(void* instance) {
@@ -80,3 +96,10 @@ std::string ChildProcessInfo::GenerateRandomChannelID(void* instance) {
                       base::GetCurrentProcId(), instance,
                       base::RandInt(0, std::numeric_limits<int>::max()));
 }
+
+// static
+int ChildProcessInfo::GenerateChildProcessUniqueId() {
+  // This function must be threadsafe.
+  static base::subtle::Atomic32 last_unique_child_id = 0;
+  return base::subtle::NoBarrier_AtomicIncrement(&last_unique_child_id, 1);
+}
diff --git a/chrome/common/child_process_info.h b/chrome/common/child_process_info.h
index d79435f..3d80a1d 100644
--- a/chrome/common/child_process_info.h
+++ b/chrome/common/child_process_info.h
@@ -21,6 +21,11 @@ class ChildProcessInfo {
     UNKNOWN_PROCESS,
   };
 
+  ChildProcessInfo(const ChildProcessInfo& original);
+  virtual ~ChildProcessInfo();
+
+  ChildProcessInfo& operator=(const ChildProcessInfo& original);
+
   // Returns the type of the process.
   ProcessType type() const { return type_; }
 
@@ -31,13 +36,11 @@ class ChildProcessInfo {
   // Getter to the process handle.
   base::ProcessHandle handle() const { return process_.handle(); }
 
-  virtual int GetProcessId() const {
-    if (pid_ != -1)
-      return pid_;
+  // The unique identifier for this child process. This identifier is NOT a
+  // process ID, and will be unique for all types of child process for
+  // one run of the browser.
+  int id() const { return id_; }
 
-    pid_ = process_.pid();
-    return pid_;
-  }
   void SetProcessBackgrounded() const { process_.SetProcessBackgrounded(true); }
   void ReduceWorkingSet() const { process_.ReduceWorkingSet(); }
 
@@ -49,25 +52,6 @@ class ChildProcessInfo {
   // process would be "Plug-in: Flash" when name is "Flash".
   std::wstring GetLocalizedTitle() const;
 
-  ChildProcessInfo(const ChildProcessInfo& original) {
-    type_ = original.type_;
-    name_ = original.name_;
-    process_ = original.process_;
-    pid_ = original.pid_;
-  }
-
-  ChildProcessInfo& operator=(const ChildProcessInfo& original) {
-    if (&original != this) {
-      type_ = original.type_;
-      name_ = original.name_;
-      process_ = original.process_;
-      pid_ = original.pid_;
-    }
-    return *this;
-  }
-
-  virtual ~ChildProcessInfo();
-
   // We define the < operator so that the ChildProcessInfo can be used as a key
   // in a std::map.
   bool operator <(const ChildProcessInfo& rhs) const {
@@ -84,21 +68,30 @@ class ChildProcessInfo {
   // The "instance" pointer value is baked into the channel id.
   static std::string GenerateRandomChannelID(void* instance);
 
+  // Returns a unique ID to identify a child process. On construction, this
+  // function will be used to generate the id_, but it is also used to generate
+  // IDs for the RenderProcessHost, which doesn't inherit from us, and whose IDs
+  // must be unique for all child processes.
+  //
+  // This function is threadsafe since RenderProcessHost is on the UI thread,
+  // but normally this will be used on the IO thread.
+  static int GenerateChildProcessUniqueId();
+
  protected:
+  // Derived objects need to use this constructor so we know what type we are.
+  // If the caller has already generated a unique ID for this child process,
+  // it should pass it as the second argument. Otherwise, -1 should be passed
+  // and a unique ID will be automatically generated.
+  ChildProcessInfo(ProcessType type, int id);
+
   void set_type(ProcessType type) { type_ = type; }
   void set_name(const std::wstring& name) { name_ = name; }
-  void set_handle(base::ProcessHandle handle) {
-    process_.set_handle(handle);
-    pid_ = -1;
-  }
-
-  // Derived objects need to use this constructor so we know what type we are.
-  ChildProcessInfo(ProcessType type);
+  void set_handle(base::ProcessHandle handle) { process_.set_handle(handle); }
 
  private:
   ProcessType type_;
   std::wstring name_;
-  mutable int pid_;  // Cache of the process id.
+  int id_;
 
   // The handle to the process.
   mutable base::Process process_;
diff --git a/chrome/common/plugin_messages_internal.h b/chrome/common/plugin_messages_internal.h
index 3bc169b..7b7ce74 100644
--- a/chrome/common/plugin_messages_internal.h
+++ b/chrome/common/plugin_messages_internal.h
@@ -17,12 +17,13 @@
 // These are messages sent from the browser to the plugin process.
 IPC_BEGIN_MESSAGES(PluginProcess)
   // Tells the plugin process to create a new channel for communication with a
-  // renderer.  The channel name is returned in a
-  // PluginProcessHostMsg_ChannelCreated message.  The renderer's process_id is
-  // passed so that the plugin process reuses an existing channel to that
-  // process if it exists.
+  // given renderer.  The channel name is returned in a
+  // PluginProcessHostMsg_ChannelCreated message.  The renderer ID is passed so
+  // that the plugin process reuses an existing channel to that process if it
+  // exists. This ID is a unique opaque identifier generated by the browser
+  // process.
   IPC_MESSAGE_CONTROL2(PluginProcessMsg_CreateChannel,
-                       int /* process_id */,
+                       int /* renderer_id */,
                        bool /* off_the_record */)
 
   // Allows a chrome plugin loaded in the browser process to send arbitrary
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index 2eb355d..b1b21ec 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -315,8 +315,10 @@ struct ViewHostMsg_Resource_Request {
   // URLRequest load flags (0 by default).
   int load_flags;
 
-  // Process ID of process that originated this request.
-  int origin_pid;
+  // Unique ID of process that originated this request. For normal renderer
+  // requests, this will be the ID of the renderer. For plugin requests routed
+  // through the renderer, this will be the plugin's ID.
+  int origin_child_id;
 
   // What this resource load is for (main frame, sub-frame, sub-resource,
   // object).
@@ -1277,7 +1279,7 @@ struct ParamTraits<ViewHostMsg_Resource_Request> {
     WriteParam(m, p.main_frame_origin);
     WriteParam(m, p.headers);
     WriteParam(m, p.load_flags);
-    WriteParam(m, p.origin_pid);
+    WriteParam(m, p.origin_child_id);
     WriteParam(m, p.resource_type);
     WriteParam(m, p.request_context);
     WriteParam(m, p.appcache_host_id);
@@ -1293,7 +1295,7 @@ struct ParamTraits<ViewHostMsg_Resource_Request> {
       ReadParam(m, iter, &r->main_frame_origin) &&
       ReadParam(m, iter, &r->headers) &&
       ReadParam(m, iter, &r->load_flags) &&
-      ReadParam(m, iter, &r->origin_pid) &&
+      ReadParam(m, iter, &r->origin_child_id) &&
       ReadParam(m, iter, &r->resource_type) &&
       ReadParam(m, iter, &r->request_context) &&
       ReadParam(m, iter, &r->appcache_host_id) &&
@@ -1313,7 +1315,7 @@ struct ParamTraits<ViewHostMsg_Resource_Request> {
     l->append(L", ");
     LogParam(p.load_flags, l);
     l->append(L", ");
-    LogParam(p.origin_pid, l);
+    LogParam(p.origin_child_id, l);
     l->append(L", ");
     LogParam(p.resource_type, l);
     l->append(L", ");
diff --git a/chrome/common/resource_dispatcher.cc b/chrome/common/resource_dispatcher.cc
index e075fc3..47efd6b 100644
--- a/chrome/common/resource_dispatcher.cc
+++ b/chrome/common/resource_dispatcher.cc
@@ -106,7 +106,7 @@ IPCResourceLoaderBridge::IPCResourceLoaderBridge(
     const std::string& main_frame_origin,
     const std::string& headers,
     int load_flags,
-    int origin_pid,
+    int origin_child_id,
     ResourceType::Type resource_type,
     uint32 request_context,
     int appcache_host_id,
@@ -124,7 +124,7 @@ IPCResourceLoaderBridge::IPCResourceLoaderBridge(
   request_.main_frame_origin = main_frame_origin;
   request_.headers = headers;
   request_.load_flags = load_flags;
-  request_.origin_pid = origin_pid;
+  request_.origin_child_id = origin_child_id;
   request_.resource_type = resource_type;
   request_.request_context = request_context;
   request_.appcache_host_id = appcache_host_id;