Prevent the browser process from creating duplicate RenderViewHosts.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index e05cab0f097872512e1ae9eee65624193aa7bcf7..52d605570378a7a8f3f7ab9b16300226025e7df9 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/stats_counters.h"
+#include "base/process/process.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -73,6 +74,7 @@
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/page_zoom.h"
+#include "content/public/common/result_codes.h"
 #include "content/public/common/url_constants.h"
 #include "net/base/mime_util.h"
 #include "net/base/net_util.h"
@@ -1252,6 +1254,7 @@ void WebContentsImpl::LostMouseLock() {
 }
 
 void WebContentsImpl::CreateNewWindow(
+    int render_process_id,
     int route_id,
     int main_frame_route_id,
     const ViewHostMsg_CreateWindow_Params& params,
@@ -1267,6 +1270,21 @@ void WebContentsImpl::CreateNewWindow(
       SiteInstance::CreateForURL(GetBrowserContext(), params.target_url) :
       GetSiteInstance();
 
+  // Creating a new window can only come from the active process for this

[Charlie Reis, 2013/12/02 19:22:50]

nit: A message to create a new window can only

[nasko, 2013/12/02 20:19:13]

Done.

+  // WebContentsImpl instance. If any other process originates the request,

[Charlie Reis, 2013/12/02 19:22:50]

nit: s/originates/sends/

[nasko, 2013/12/02 20:19:13]

Done.

+  // it is invalid and must be terminated.

[Charlie Reis, 2013/12/02 19:22:50]

nit: and the process must be

(it == the request)

[nasko, 2013/12/02 20:19:13]

Done.

+  if (site_instance->HasProcess() &&
+      site_instance->GetProcess()->GetID() != render_process_id) {

[Charlie Reis, 2013/12/02 19:22:50]

This doesn't look right to me, since |site_instance| may be newly created,
rather than the being the "active process for this WebContentsImpl instance."

If we really mean to check it against the current process, we should use
GetRenderProcessHost() here.  Is that the right thing to do, or does that cause
problems in the opener_suppressed (e.g., rel=noreferrer) case?

[nasko, 2013/12/02 20:19:13]

I don't think using GetRenderProcessHost() should impact the opener_suppressed
case, since only the current RVH should be sending a CreateNewWindow message.

+    base::ProcessHandle process_handle =
+        RenderProcessHost::FromID(render_process_id)->GetHandle();
+    if (process_handle != base::kNullProcessHandle) {
+      RecordAction(
+          UserMetricsAction("Terminate_ProcessMismatch_CreateNewWindow"));
+      base::KillProcess(process_handle, content::RESULT_CODE_KILLED, false);
+    }
+    return;
+  }
+
   // We must assign the SessionStorageNamespace before calling Init().
   //
   // http://crbug.com/142685