NOT YET READY: Making chrome.windows.create establish an actual "opener" relationship.

chrome/browser/ui/browser_navigator.cc

Index: chrome/browser/ui/browser_navigator.cc
diff --git a/chrome/browser/ui/browser_navigator.cc b/chrome/browser/ui/browser_navigator.cc
index abf993f818697581907ccd20f65fb0941d6c09be..ad2772485bbe36320bd045d431f84c3319e566aa 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -34,6 +34,8 @@
 #include "content/public/browser/browser_url_handler.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/features/features.h"
@@ -358,10 +360,15 @@ content::WebContents* CreateTargetContents(const chrome::NavigateParams& params,
                                            const GURL& url) {
   WebContents::CreateParams create_params(
       params.browser->profile(),
-      params.source_site_instance && !params.force_new_process_for_new_contents
-          ? params.source_site_instance

[Charlie Reis, 2017/06/05 21:06:28]

Was this really only used for chrome.windows.create?  Huh.  I suppose having
this process decision line up with whether an opener is defined is ok, though it
might help to have a comment to that effect.

One sanity check: It appears we're changing the default behavior, but I'm not
sure if that results in an observable change.  Before,
force_new_process_for_new_contents defaulted to false, and thus we would use
params.source_site_instance for any case that didn't explicitly set it to true. 
Now, opener defaults to null, so we use GetSiteInstanceForNewTab for any case
that doesn't explicitly define it.

The cases I know about are chrome.windows.create (which set it to false before
and now sets the opener, preserving behavior) and RenderFrameHostImpl::OnOpenURL
(which set it to true before and doesn't set the opener, preserving behavior).

There are over 100 calls to Navigate, so it's possible others could be affected.
 It looks like the only one that assigns source_site_instance is
chrome.windows.create, though, so I *think* all the rest hit the
GetSiteInstanceForNewTab call before and will after as well.  Does that sound
right?

[Łukasz Anforowicz, 2017/06/05 22:06:54]

I've added a comment here.
Right - the main thing here is that chrome::NavigateParams::source_site_instance
was/is only set by 1) chrome.windows.create API and 2)
NavigatorImpl::RequestOpenURL (via content::OpenURLParams).  The new behavior
should be matching expectations in these two cases.

+      params.opener
+          ? params.opener->GetSiteInstance()
           : tab_util::GetSiteInstanceForNewTab(params.browser->profile(), url));
   create_params.main_frame_name = params.frame_name;
+  if (params.opener) {
+    create_params.opener_render_frame_id = params.opener->GetRoutingID();
+    create_params.opener_render_process_id =
+        params.opener->GetProcess()->GetID();
+  }
   if (params.source_contents) {
     create_params.initial_size =
         params.source_contents->GetContainerBounds().size();
@@ -473,6 +480,7 @@ void Navigate(NavigateParams* params) {
   if (GetSourceProfile(params) != params->browser->profile()) {
     // A tab is being opened from a link from a different profile, we must reset
     // source information that may cause state to be shared.
+    params->opener = nullptr;
     params->source_contents = nullptr;
     params->source_site_instance = nullptr;
     params->referrer = content::Referrer();