Keep web popups opened from extensions in same BrowsingInstance.

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 b90525ab3d24eeff8bec4cd54b8f6e262f7da2f1..f86afec25d4187449633205d614e98d1cc73be49 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -248,6 +248,11 @@ void NormalizeDisposition(chrome::NavigateParams* params) {
 
 // Obtain the profile used by the code that originated the Navigate() request.
 Profile* GetSourceProfile(chrome::NavigateParams* params) {
+  if (params->source_site_instance) {

[alexmos, 2016/10/04 21:24:44]

Turned out the source_site_instance check needed to precede the source_contents
check.  This mattered in the case where chrome.windows.create was used with
multiple URLs to be opened (i.e. chrome.windows.create({url: ['http://foo.com',
'http://bar.com'], incognito: true}); see ExtensionApiTest.TabQuery [1]).  In
that case, a navigation to the first URL in the list could've modified
source_contents in |params| (to the first URL's WebContents, I think) which
would've been used for subsequent URLs.  See the loop in [2] and how it reuses
the params.  I think in that case, each navigation's source profile should be
considered to be the source site instance's profile.

[1]
https://cs.chromium.org/chromium/src/chrome/test/data/extensions/api_test/tab...

[2]
https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/tabs/tabs_...

[Lei Zhang, 2016/10/06 00:36:47]

You may want to write down some of your notes here as an actual comment in the
code. It's not obvious that the order matters.

[alexmos, 2016/10/06 00:42:00]

Thanks!  I'll add the comment in a followup so I can land this sooner and
hopefully make tomorrow's canary.

+    return Profile::FromBrowserContext(
+        params->source_site_instance->GetBrowserContext());
+  }
+
   if (params->source_contents) {
     return Profile::FromBrowserContext(
         params->source_contents->GetBrowserContext());
@@ -465,7 +470,8 @@ 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->source_contents = NULL;
+    params->source_contents = nullptr;
+    params->source_site_instance = nullptr;
     params->referrer = content::Referrer();
   }