Commit 8a4b083f authored by Istiaque Ahmed's avatar Istiaque Ahmed Committed by Commit Bot

chrome.windows.create: Do not specify source_site_instance when it isn't necessary.

When the newly created WebContents through chrome.windows.create doesn't
wish to preserve opener relationship, do not specify
NavigateParams.source_site_instance as it as it is only required and
useful when there is a need to keep two WebContentses in the same
BrowsingInstance or SiteInstance. Further, this makes WindowsCreateFunction
only use SiteInstances when necessary.

Add tests to cover the test cases that also verify that the newly created
WebContents has correct origin or not.

Bug: 986348
Change-Id: I679354bb77c317c4ecc4aeabfec8799bca69dd25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1712441
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarŁukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681913}
parent 293bb359
......@@ -592,9 +592,11 @@ ExtensionFunction::ResponseAction WindowsCreateFunction::Run() {
// https://crbug.com/713888.
bool set_self_as_opener = create_data->set_self_as_opener && // present?
*create_data->set_self_as_opener; // set to true?
navigate_params.opener = set_self_as_opener ? render_frame_host() : nullptr;
navigate_params.source_site_instance =
render_frame_host()->GetSiteInstance();
if (set_self_as_opener) {
navigate_params.opener = render_frame_host();
navigate_params.source_site_instance =
render_frame_host()->GetSiteInstance();
}
Navigate(&navigate_params);
}
......
......@@ -2238,4 +2238,87 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, WindowsCreate_NoOpener) {
// (we hope this can be limited to background pages / contents).
}
// Tests the origin of tabs created through chrome.windows.create().
IN_PROC_BROWSER_TEST_F(ExtensionApiTest, WindowsCreate_OpenerAndOrigin) {
const extensions::Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("../simple_with_file"));
ASSERT_TRUE(extension);
// Navigate a tab to an extension page.
GURL extension_url = extension->GetResourceURL("file.html");
ui_test_utils::NavigateToURL(browser(), extension_url);
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
const std::string extension_origin_str =
url::Origin::Create(extension->url()).Serialize();
constexpr char kDataURL[] = "data:text/html,<html>test</html>";
std::string extension_url_str = extension_url.spec();
struct TestCase {
// The url to use in chrome.windows.create().
std::string url;
// If set, its value will be used to specify |setSelfAsOpener|.
base::Optional<bool> set_self_as_opener;
// The origin we expect the new tab to be in, opaque origins will be "null".
std::string expected_origin_str;
} test_cases[] = {
// about:blank URLs.
// With opener relationship, about:blank urls will get the extension's
// origin, without opener relationship, they will get opaque/"null"
// origin.
{url::kAboutBlankURL, true, extension_origin_str},
{url::kAboutBlankURL, false, "null"},
{url::kAboutBlankURL, base::nullopt, "null"},
// data:... URLs.
// With opener relationship or not, "data:..." URLs always gets unique
// origin, so origin will always be "null" in these cases.
{kDataURL, true, "null"},
{kDataURL, false, "null"},
{kDataURL, base::nullopt, "null"},
// chrome-extension:// URLs.
// These always get extension origin.
{extension_url_str, true, extension_origin_str},
{extension_url_str, false, extension_origin_str},
{extension_url_str, base::nullopt, extension_origin_str},
};
auto run_test_case = [&web_contents](const TestCase& test_case) {
std::string maybe_specify_set_self_as_opener = "";
if (test_case.set_self_as_opener) {
maybe_specify_set_self_as_opener =
base::StringPrintf(", setSelfAsOpener: %s",
*test_case.set_self_as_opener ? "true" : "false");
}
std::string script = base::StringPrintf(
R"( chrome.windows.create({url: '%s'%s}); )", test_case.url.c_str(),
maybe_specify_set_self_as_opener.c_str());
content::WebContents* new_contents = nullptr;
{
content::WebContentsAddedObserver observer;
ASSERT_TRUE(content::ExecuteScript(web_contents, script));
new_contents = observer.GetWebContents();
}
ASSERT_TRUE(new_contents);
ASSERT_TRUE(content::WaitForLoadStop(new_contents));
std::string actual_origin_str;
EXPECT_TRUE(ExecuteScriptAndExtractString(
new_contents, "window.domAutomationController.send(origin);",
&actual_origin_str));
EXPECT_EQ(test_case.expected_origin_str, actual_origin_str);
const bool is_opaque_origin =
new_contents->GetMainFrame()->GetLastCommittedOrigin().opaque();
EXPECT_EQ(test_case.expected_origin_str == "null", is_opaque_origin);
};
for (size_t i = 0; i < base::size(test_cases); ++i) {
const auto& test_case = test_cases[i];
SCOPED_TRACE(
base::StringPrintf("#%" PRIuS " %s", i, test_case.url.c_str()));
run_test_case(test_case);
}
}
} // namespace extensions
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment