diff --git a/chrome/browser/android/tab_web_contents_delegate_android.cc b/chrome/browser/android/tab_web_contents_delegate_android.cc index d29d111f01d13..02b1440f77e59 100644 --- a/chrome/browser/android/tab_web_contents_delegate_android.cc +++ b/chrome/browser/android/tab_web_contents_delegate_android.cc @@ -360,7 +360,7 @@ WebContents* TabWebContentsDelegateAndroid::OpenURLFromTab( return WebContentsDelegateAndroid::OpenURLFromTab(source, params); } - popup_delegate->nav_params()->created_with_opener = true; + popup_delegate->nav_params()->opened_by_another_window = true; TabModelList::HandlePopupNavigation(popup_delegate->nav_params()); return nullptr; } diff --git a/chrome/browser/sync/sync_ui_util.cc b/chrome/browser/sync/sync_ui_util.cc index 7bbc08ba34f2f..98d72c269eb85 100644 --- a/chrome/browser/sync/sync_ui_util.cc +++ b/chrome/browser/sync/sync_ui_util.cc @@ -193,7 +193,7 @@ void OpenTabForSyncTrustedVaultUserAction(Browser* browser, const GURL& url) { NavigateParams params(GetSingletonTabNavigateParams(browser, url)); // Allow the window to close itself. - params.created_with_opener = true; + params.opened_by_another_window = true; Navigate(¶ms); } diff --git a/chrome/browser/ui/android/tab_model/tab_model_jni_bridge.cc b/chrome/browser/ui/android/tab_model/tab_model_jni_bridge.cc index d0d9ebf4cf56f..0f547f3c314dc 100644 --- a/chrome/browser/ui/android/tab_model/tab_model_jni_bridge.cc +++ b/chrome/browser/ui/android/tab_model/tab_model_jni_bridge.cc @@ -122,8 +122,8 @@ void TabModelJniBridge::HandlePopupNavigation(TabAndroid* parent, content::ConvertResourceRequestBodyToJavaObject(env, params->post_data); Java_TabModelJniBridge_openNewTab( env, jobj, parent->GetJavaObject(), jurl, jinitiator_origin, jheaders, - jpost_data, static_cast<int>(disposition), params->created_with_opener, - params->is_renderer_initiated); + jpost_data, static_cast<int>(disposition), + params->opened_by_another_window, params->is_renderer_initiated); } WebContents* TabModelJniBridge::GetWebContentsAt(int index) const { diff --git a/chrome/browser/ui/blocked_content/blocked_window_params.cc b/chrome/browser/ui/blocked_content/blocked_window_params.cc index 1823141641b38..1ae4fe0d56a35 100644 --- a/chrome/browser/ui/blocked_content/blocked_window_params.cc +++ b/chrome/browser/ui/blocked_content/blocked_window_params.cc @@ -52,7 +52,7 @@ NavigateParams BlockedWindowParams::CreateNavigateParams( nav_params.is_renderer_initiated = true; nav_params.window_action = NavigateParams::SHOW_WINDOW; nav_params.user_gesture = user_gesture_; - nav_params.created_with_opener = !opener_suppressed_; + nav_params.opened_by_another_window = !opener_suppressed_; nav_params.window_bounds = web_contents->GetContainerBounds(); if (features_.has_x) nav_params.window_bounds.set_x(features_.x); diff --git a/chrome/browser/ui/browser_navigator.cc b/chrome/browser/ui/browser_navigator.cc index 1fb512b9f9c2d..1c874573619ca 100644 --- a/chrome/browser/ui/browser_navigator.cc +++ b/chrome/browser/ui/browser_navigator.cc @@ -443,7 +443,7 @@ std::unique_ptr<content::WebContents> CreateTargetContents( params.opener->GetProcess()->GetID(); } if (params.source_contents) { - create_params.created_with_opener = params.created_with_opener; + create_params.opened_by_another_window = params.opened_by_another_window; } if (params.disposition == WindowOpenDisposition::NEW_BACKGROUND_TAB) create_params.initially_hidden = true; diff --git a/chrome/browser/ui/browser_navigator_params.cc b/chrome/browser/ui/browser_navigator_params.cc index 85dd2dbb7066b..d8149ec70af10 100644 --- a/chrome/browser/ui/browser_navigator_params.cc +++ b/chrome/browser/ui/browser_navigator_params.cc @@ -86,7 +86,7 @@ void NavigateParams::FillNavigateParamsFromOpenURLParams( // The following NavigateParams don't have an equivalent in OpenURLParams: // browser // contents_to_insert - // created_with_opener + // opened_by_another_window // extension_app_id // frame_name // group diff --git a/chrome/browser/ui/browser_navigator_params.h b/chrome/browser/ui/browser_navigator_params.h index 8cf44a922efd2..b5588a3c2cfd1 100644 --- a/chrome/browser/ui/browser_navigator_params.h +++ b/chrome/browser/ui/browser_navigator_params.h @@ -262,9 +262,10 @@ struct NavigateParams { // navigation entry. bool should_replace_current_entry = false; - // Indicates whether |contents_to_insert| is being created with a - // window.opener. - bool created_with_opener = false; + // Indicates whether |contents_to_insert| is being created by another window, + // and thus can be closed via window.close(). This may be true even when + // "noopener" was used. + bool opened_by_another_window = false; // Whether or not the related navigation was started in the context menu. bool started_from_context_menu = false; diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc index 9786cb8ef6633..e4f71e2ce9cf7 100644 --- a/content/browser/renderer_host/render_view_host_impl.cc +++ b/content/browser/renderer_host/render_view_host_impl.cc @@ -369,7 +369,7 @@ RenderViewHostDelegate* RenderViewHostImpl::GetDelegate() { bool RenderViewHostImpl::CreateRenderView( const absl::optional<blink::FrameToken>& opener_frame_token, int proxy_route_id, - bool window_was_created_with_opener) { + bool window_was_opened_by_another_window) { TRACE_EVENT0("renderer_host,navigation", "RenderViewHostImpl::CreateRenderView"); DCHECK(!IsRenderViewLive()) << "Creating view twice"; @@ -461,7 +461,8 @@ bool RenderViewHostImpl::CreateRenderView( frame_tree_->controller().GetSessionStorageNamespace(site_info_)->id(); params->hidden = frame_tree_->delegate()->IsHidden(); params->never_composited = delegate_->IsNeverComposited(); - params->window_was_created_with_opener = window_was_created_with_opener; + params->window_was_opened_by_another_window = + window_was_opened_by_another_window; params->base_background_color = delegate_->GetBaseBackgroundColor(); bool is_portal = delegate_->IsPortal(); diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h index 0e95eaa02ae02..b9a91d4cf2237 100644 --- a/content/browser/renderer_host/render_view_host_impl.h +++ b/content/browser/renderer_host/render_view_host_impl.h @@ -152,14 +152,16 @@ class CONTENT_EXPORT RenderViewHostImpl // TestRenderViewHost. // |opener_route_id| parameter indicates which RenderView created this // (MSG_ROUTING_NONE if none). - // |window_was_created_with_opener| is true if this top-level frame was - // created with an opener. (The opener may have been closed since.) + // |window_was_opened_by_another_window| is true if this top-level frame was + // created by another window, as opposed to independently created (through + // the browser UI, etc). This is true even when the window is opened with + // "noopener", and even if the opener has been closed since. // |proxy_route_id| is only used when creating a RenderView in an inactive // state. virtual bool CreateRenderView( const absl::optional<blink::FrameToken>& opener_frame_token, int proxy_route_id, - bool window_was_created_with_opener); + bool window_was_opened_by_another_window); // Tracks whether this RenderViewHost is in an active state (rather than // pending unload or unloaded), according to its main frame diff --git a/content/browser/web_contents/opened_by_dom_browsertest.cc b/content/browser/web_contents/opened_by_dom_browsertest.cc index 4dab737256977..d92e94fb952d9 100644 --- a/content/browser/web_contents/opened_by_dom_browsertest.cc +++ b/content/browser/web_contents/opened_by_dom_browsertest.cc @@ -115,6 +115,31 @@ IN_PROC_BROWSER_TEST_F(OpenedByDOMTest, Popup) { EXPECT_TRUE(AttemptCloseFromJavaScript(popup->web_contents())); } +// Tests that window.close() works in a popup window that's opened with noopener +// that has navigated a few times. +IN_PROC_BROWSER_TEST_F(OpenedByDOMTest, NoOpenerPopup) { + ASSERT_TRUE(embedded_test_server()->Start()); + + GURL url1 = embedded_test_server()->GetURL("/site_isolation/blank.html?1"); + GURL url2 = embedded_test_server()->GetURL("/site_isolation/blank.html?2"); + GURL url3 = embedded_test_server()->GetURL("/site_isolation/blank.html?3"); + EXPECT_TRUE(NavigateToURL(shell(), url1)); + + // Create a popup through window.open() and 'noopener'. + ShellAddedObserver new_shell_observer; + TestNavigationObserver nav_observer(nullptr); + nav_observer.StartWatchingNewWebContents(); + CHECK( + ExecJs(shell(), JsReplace("window.open($1, '_blank','noopener')", url2))); + nav_observer.Wait(); + Shell* popup = new_shell_observer.GetShell(); + + // Navigate the popup. + EXPECT_TRUE(NavigateToURL(popup, url3)); + // Closing the popup should still work. + EXPECT_TRUE(AttemptCloseFromJavaScript(popup->web_contents())); +} + // Tests that window.close() works in a popup window that has navigated a few // times and swapped processes. IN_PROC_BROWSER_TEST_F(OpenedByDOMTest, CrossProcessPopup) { diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 357c6a58efccf..6f0d2cee13b39 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -824,7 +824,7 @@ void WebContentsImpl::WebContentsObserverList::RemoveObserver( WebContentsImpl::WebContentsImpl(BrowserContext* browser_context) : delegate_(nullptr), render_view_host_delegate_view_(nullptr), - created_with_opener_(false), + opened_by_another_window_(false), node_(this), frame_tree_(browser_context, this, @@ -1038,8 +1038,8 @@ std::unique_ptr<WebContentsImpl> WebContentsImpl::CreateWithOpener( // This may be true even when opener is null, such as when opening blocked // popups. - if (params.created_with_opener) - new_contents->created_with_opener_ = true; + if (params.opened_by_another_window) + new_contents->opened_by_another_window_ = true; WebContentsImpl* outer_web_contents = nullptr; if (params.guest_delegate) { @@ -7845,9 +7845,9 @@ bool WebContentsImpl::CreateRenderViewForRenderManager( const auto proxy_routing_id = proxy_host ? proxy_host->GetRoutingID() : MSG_ROUTING_NONE; // TODO(https://crbug.com/1171646): Given MPArch, should we pass - // created_with_opener_ for non primary FrameTrees? + // opened_by_another_window_ for non primary FrameTrees? if (!rvh_impl->CreateRenderView(opener_frame_token, proxy_routing_id, - created_with_opener_)) { + opened_by_another_window_)) { return false; } // Set the TextAutosizer state from the main frame's renderer on the new view, @@ -8806,10 +8806,10 @@ void WebContentsImpl::SetOpenerForNewContents(FrameTreeNode* opener, // https://crbug.com/705316 new_root->SetOriginalOpener(opener->frame_tree()->root()); new_root->SetOpenerDevtoolsFrameToken(opener->devtools_frame_token()); + opened_by_another_window_ = true; if (!opener_suppressed) { new_root->SetOpener(opener); - created_with_opener_ = true; } } } diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 59fcd30e80b9f..7183994d0bbcd 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -1833,9 +1833,9 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, // the observer list then. WebContentsObserverList observers_; - // True if this tab was opened by another tab. This is not unset if the opener - // is closed. - bool created_with_opener_; + // True if this tab was opened by another window. This is true even if the tab + // is opened with "noopener", and won't be unset if the opener is closed. + bool opened_by_another_window_; #if defined(OS_ANDROID) std::unique_ptr<WebContentsAndroid> web_contents_android_; diff --git a/content/common/frame.mojom b/content/common/frame.mojom index 192a5084eac3e..72fd11b1733aa 100644 --- a/content/common/frame.mojom +++ b/content/common/frame.mojom @@ -118,8 +118,8 @@ struct CreateViewParams { // for displaying graphical output. bool never_composited; - // Whether the window associated with this view was created with an opener. - bool window_was_created_with_opener; + // Whether the window associated with this view was created by another window. + bool window_was_opened_by_another_window; // Whether lookup of frames in the created RenderView (e.g. lookup via // window.open or via <a target=...>) should be renderer-wide (i.e. going diff --git a/content/public/browser/web_contents.cc b/content/public/browser/web_contents.cc index f1dcf53ea481b..e40e14fd4913f 100644 --- a/content/public/browser/web_contents.cc +++ b/content/public/browser/web_contents.cc @@ -22,7 +22,7 @@ WebContents::CreateParams::CreateParams(BrowserContext* context, opener_render_process_id(content::ChildProcessHost::kInvalidUniqueID), opener_render_frame_id(MSG_ROUTING_NONE), opener_suppressed(false), - created_with_opener(false), + opened_by_another_window(false), initially_hidden(false), guest_delegate(nullptr), context(nullptr), diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h index 91c8bb57fb045..ddf33e29d8f06 100644 --- a/content/public/browser/web_contents.h +++ b/content/public/browser/web_contents.h @@ -141,11 +141,11 @@ class WebContents : public PageNavigator, // reference to its opener. bool opener_suppressed; - // Indicates whether this WebContents was created with a window.opener. + // Indicates whether this WebContents was created by another window. // This is used when determining whether the WebContents is allowed to be // closed via window.close(). This may be true even with a null |opener| - // (e.g., for blocked popups). - bool created_with_opener; + // (e.g., for blocked popups), or when the window is opened with "noopener". + bool opened_by_another_window; // The name of the top-level frame of the new window. It is non-empty // when creating a named window (e.g. <a target="foo"> or diff --git a/content/public/test/render_view_test.cc b/content/public/test/render_view_test.cc index 2d2ff399ff5f4..aa13a70f24c42 100644 --- a/content/public/test/render_view_test.cc +++ b/content/public/test/render_view_test.cc @@ -496,7 +496,7 @@ void RenderViewTest::SetUp() { mojom::CreateViewParamsPtr view_params = mojom::CreateViewParams::New(); view_params->opener_frame_token = absl::nullopt; - view_params->window_was_created_with_opener = false; + view_params->window_was_opened_by_another_window = false; view_params->renderer_preferences = blink::RendererPreferences(); view_params->web_preferences = blink::web_pref::WebPreferences(); view_params->view_id = render_thread_->GetNextRoutingID(); diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index afec7767d9054..092352d96858b 100644 --- a/content/renderer/render_view_impl.cc +++ b/content/renderer/render_view_impl.cc @@ -152,7 +152,7 @@ void RenderViewImpl::Initialize( } // TODO(davidben): Move this state from Blink into content. - if (params->window_was_created_with_opener) + if (params->window_was_opened_by_another_window) GetWebView()->SetOpenedByDOM(); webview_->SetRendererPreferences(params->renderer_preferences); @@ -339,7 +339,7 @@ WebView* RenderViewImpl::CreateView( view_params->opener_frame_token = creator->GetFrameToken(); DCHECK_EQ(GetRoutingID(), creator_frame->render_view()->GetRoutingID()); - view_params->window_was_created_with_opener = true; + view_params->window_was_opened_by_another_window = true; view_params->renderer_preferences = webview_->GetRendererPreferences(); view_params->web_preferences = webview_->GetWebPreferences(); view_params->view_id = reply->route_id;