Try to restore window.opener when opening blocked popups

chrome/browser/ui/browser_navigator.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/browser_navigator.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
@@ skipping 18 matching lines @@
 #include "chrome/browser/ui/omnibox/location_bar.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/status_bubble.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_url_handler.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/session_storage_namespace.h"
+#include "content/public/browser/site_instance.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 
 #if defined(USE_AURA)
 #include "ui/aura/window.h"
 #endif
 
 using content::GlobalRequestID;
 using content::WebContents;
 
@@ skipping 192 matching lines @@
 
 void LoadURLInContents(WebContents* target_contents,
                        const GURL& url,
                        chrome::NavigateParams* params) {
   content::NavigationController::LoadURLParams load_url_params(url);
   load_url_params.referrer = params->referrer;
   load_url_params.transition_type = params->transition;
   load_url_params.extra_headers = params->extra_headers;
   load_url_params.should_replace_current_entry =
       params->should_replace_current_entry;
+  // Even if we only copy the session storage namespace and do not set the
+  // opener, we need to use the SiteInstance that was created in
+  // CreateTargetContents to make sure we created the SessionStorageNamespace
+  // in the right StoragePartition.
+  load_url_params.should_stay_in_site_instance =

[Charlie Reis, 2013/07/31 17:16:16]

This is incorrect.  Setting the opener will not always imply that you should
stay in the same SiteInstance.  More comments below.

[jochen (gone - plz use gerrit), 2013/07/31 18:30:52]

Removed.

+      params->should_copy_session_storage_namespace ||
+      params->should_set_opener;
 
   if (params->transferred_global_request_id != GlobalRequestID()) {
     load_url_params.is_renderer_initiated = params->is_renderer_initiated;
     load_url_params.transferred_global_request_id =
         params->transferred_global_request_id;
   } else if (params->is_renderer_initiated) {
     load_url_params.is_renderer_initiated = true;
   }
   target_contents->GetController().LoadURLWithParams(load_url_params);
 }
@@ skipping 48 matching lines @@
   chrome::NavigateParams* params_;
   scoped_ptr<WebContents> target_contents_owner_;
   DISALLOW_COPY_AND_ASSIGN(ScopedTargetContentsOwner);
 };
 
 content::WebContents* CreateTargetContents(const chrome::NavigateParams& params,
                                            const GURL& url) {
   WebContents::CreateParams create_params(
       params.browser->profile(),
       tab_util::GetSiteInstanceForNewTab(params.browser->profile(), url));
+  // In order for window.opener to be accessible, the target contents needs to

[Charlie Reis, 2013/07/31 17:16:16]

This reasoning is false.  The reason that we create opener RVHs is so that other
processes will have valid window.openers.

Perhaps there's additional reasoning that makes this ok for the time being?

Today, we treat "w=window.open();w.opener=null;w.location.href=url" as a
cross-process navigation that opens in a different SiteInstance but the same
BrowsingInstance.  In that case, the opener will be suppressed and perhaps we
won't hit this logic?

For rel=noreferrer target=_blank links, we also suppress the opener and open the
link in new SiteInstance and new BrowsingInstance.  The opener won't be set
there either.

Have you tested those cases to ensure they still work?  I see a test for
rel=noreferrer, but it's not using target=_blank, so I don't think
suppress_opener will be true.

A bigger problem is that our site isolation work is making it possible for the
new window to open in a new SiteInstance even with just a normal cross-site
window.open call.  I don't want to paint us into a corner here, so I don't want
to add logic that depends on something else.  

If we do have to change something here, we should keep the process decisions in
fewer places.  The tab_util::GetSiteInstanceForNewTab function above seems like
a better place to handle it.

[jochen (gone - plz use gerrit), 2013/07/31 18:30:52]

I removed the SiteInstance related logic as discussed via VC.

To summarize, by not forcing a specific site instance, we might have corner
cases where we could have a scriptable window.opener but won't have one. I guess
that's fine.

Also, not forcing the SiteInstance works fine with the current StoragePartition
model but we might need to revise that when we use more than one storage
partition. Added a comment about that.

+  // run in the same process as the source contents.
+  if (params.source_contents && params.should_set_opener)
+    create_params.site_instance = params.source_contents->GetSiteInstance();
   if (params.source_contents) {
     create_params.initial_size =
         params.source_contents->GetView()->GetContainerSize();
   }
 #if defined(USE_AURA)
   if (params.browser->window() &&
       params.browser->window()->GetNativeWindow()) {
     create_params.context =
         params.browser->window()->GetNativeWindow();
   }
 #endif
 
-  content::WebContents* target_contents = WebContents::Create(create_params);
+  content::SessionStorageNamespaceMap namespace_map;
+  scoped_refptr<content::SiteInstance> site_instance;
+  if (params.source_contents && params.should_copy_session_storage_namespace) {
+    Profile* source_profile = Profile::FromBrowserContext(
+        params.source_contents->GetBrowserContext());
+    bool is_guest =
+        params.source_contents->GetRenderViewHost()->GetProcess()->IsGuest();
+    site_instance =
+        !params.should_set_opener && !is_guest
+            ? content::SiteInstance::CreateForURL(source_profile, url)
+            : params.source_contents->GetSiteInstance();
+    create_params.site_instance = site_instance;
+
+    std::string partition_id = source_profile->GetStoragePartitionIdForSite(
+        site_instance->GetSiteURL());
+    content::StoragePartition* partition =
+        content::BrowserContext::GetStoragePartition(source_profile,
+                                                     site_instance);
+    content::SessionStorageNamespaceMap::const_iterator old_namespace =
+        params.source_contents->GetController().GetSessionStorageNamespaceMap()
+            .find(partition_id);
+    scoped_refptr<content::SessionStorageNamespace> cloned_namespace =
+        content::SessionStorageNamespace::Clone(
+            partition->GetDOMStorageContext(), old_namespace->second.get());
+    namespace_map.insert(make_pair(partition_id, cloned_namespace));
+  }
+  WebContents* target_contents = WebContents::CreateWithSessionStorage(
+      create_params,
+      namespace_map,
+      params.should_set_opener ? params.source_contents : NULL);
+
   // New tabs can have WebUI URLs that will make calls back to arbitrary
   // tab helpers, so the entire set of tab helpers needs to be set up
   // immediately.
   BrowserNavigatorWebContentsAdoption::AttachTabHelpers(target_contents);
   extensions::TabHelper::FromWebContents(target_contents)->
       SetExtensionAppById(params.extension_app_id);
   // TODO(sky): Figure out why this is needed. Without it we seem to get
   // failures in startup tests.
   // By default, content believes it is not hidden.  When adding contents
   // in the background, tell it that it's hidden.
@@ skipping 43 matching lines @@
       is_renderer_initiated(false),
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(NO_ACTION),
       user_gesture(true),
       path_behavior(RESPECT),
       ref_behavior(IGNORE_REF),
       browser(a_browser),
       initiating_profile(NULL),
       host_desktop_type(GetHostDesktop(a_browser)),
-      should_replace_current_entry(false) {
+      should_replace_current_entry(false),
+      should_copy_session_storage_namespace(false),
+      should_set_opener(false) {
 }
 
 NavigateParams::NavigateParams(Browser* a_browser,
                                WebContents* a_target_contents)
     : target_contents(a_target_contents),
       source_contents(NULL),
       disposition(CURRENT_TAB),
       transition(content::PAGE_TRANSITION_LINK),
       is_renderer_initiated(false),
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(NO_ACTION),
       user_gesture(true),
       path_behavior(RESPECT),
       ref_behavior(IGNORE_REF),
       browser(a_browser),
       initiating_profile(NULL),
       host_desktop_type(GetHostDesktop(a_browser)),
-      should_replace_current_entry(false) {
+      should_replace_current_entry(false),
+      should_copy_session_storage_namespace(false),
+      should_set_opener(false) {
 }
 
 NavigateParams::NavigateParams(Profile* a_profile,
                                const GURL& a_url,
                                content::PageTransition a_transition)
     : url(a_url),
       target_contents(NULL),
       source_contents(NULL),
       disposition(NEW_FOREGROUND_TAB),
       transition(a_transition),
       is_renderer_initiated(false),
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(SHOW_WINDOW),
       user_gesture(true),
       path_behavior(RESPECT),
       ref_behavior(IGNORE_REF),
       browser(NULL),
       initiating_profile(a_profile),
       host_desktop_type(chrome::GetActiveDesktop()),
-      should_replace_current_entry(false) {
+      should_replace_current_entry(false),
+      should_copy_session_storage_namespace(false),
+      should_set_opener(false) {
 }
 
 NavigateParams::~NavigateParams() {}
 
 void FillNavigateParamsFromOpenURLParams(chrome::NavigateParams* nav_params,
                                          const content::OpenURLParams& params) {
   nav_params->referrer = params.referrer;
   nav_params->extra_headers = params.extra_headers;
   nav_params->disposition = params.disposition;
   nav_params->override_encoding = params.override_encoding;
@@ skipping 223 matching lines @@
   bool reverse_on_redirect = false;
   content::BrowserURLHandler::GetInstance()->RewriteURLIfNecessary(
       &rewritten_url, browser_context, &reverse_on_redirect);
 
   // Some URLs are mapped to uber subpages. Do not allow them in incognito.
   return !(rewritten_url.scheme() == chrome::kChromeUIScheme &&
            rewritten_url.host() == chrome::kChromeUIUberHost);
 }
 
 }  // namespace chrome