Commit b8d59c23 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Fixing RenderFrameHost::ViewSource() for ExtensionOptionsGuest.

After r512625, various ways of viewing page or frame source end up going
through RenderFrameHost::ViewSource which depends on ability to present
the view source WebContents to the user via WebContentsDelegate's
AddNewContent method.  This method was not overriden by
ExtensionOptionsGuest, leading to https://crbug.com/796080.

This CL fixes https://crbug.com/796080 by overriding
ExtensionOptionsGuest::AddNewContent and making it forward the call to
the embedder.  This CL also adds a regression test for this scenario.

Note that by default guest views cannot open new windows - this is
blocked (unless the embedder explicitly allows this) and results
in "<webview>: A new window was blocked." error message.  This CL
does not change this - window.open('<url>', '_blank') as well
as <a target="_blank"...> in a guest view will continue to be
blocked by default.  I've tested this manually after temporarily
overriding AddNewContent method higher in the class hierarchy
- in GuestViewBase.  See also https://crbug.com/796080#c7.

A follow-up bug has been opened to investigate if AddNewContents
method should instead be implemented in the base class of
ExtensionOptionsGuest - in GuestViewBase: https://crbug.com/797340.

Bug: 796080
Change-Id: I83ece7fc94bbf1005c5e2878159076949c48b076
Tbr: finnur@chromium.org or rdevlin.cronin@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/841623
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarLucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526041}
parent 74b63294
......@@ -4,7 +4,11 @@
#include "chrome/browser/ui/webui/extensions/extension_settings_browsertest.h"
#include <string>
#include "base/path_service.h"
#include "base/strings/string_util.h"
#include "base/threading/thread_restrictions.h"
#include "chrome/browser/extensions/chrome_test_extension_loader.h"
#include "chrome/browser/extensions/unpacked_installer.h"
#include "chrome/browser/profiles/profile.h"
......@@ -12,6 +16,10 @@
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/web_contents_sizer.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "extensions/browser/extension_dialog_auto_confirm.h"
#include "extensions/browser/extension_system.h"
......@@ -58,9 +66,12 @@ void ExtensionSettingsUIBrowserTest::InstallPlatformApp() {
test_data_dir_.AppendASCII("platform_apps").AppendASCII("minimal")));
}
void ExtensionSettingsUIBrowserTest::InstallExtensionWithInPageOptions() {
EXPECT_TRUE(
InstallExtension(test_data_dir_.AppendASCII("options_page_in_view")));
const extensions::Extension*
ExtensionSettingsUIBrowserTest::InstallExtensionWithInPageOptions() {
const extensions::Extension* extension =
InstallExtension(test_data_dir_.AppendASCII("options_page_in_view"));
EXPECT_TRUE(extension);
return extension;
}
void ExtensionSettingsUIBrowserTest::AddManagedPolicyProvider() {
......@@ -92,3 +103,59 @@ const Extension* ExtensionSettingsUIBrowserTest::InstallExtension(
loader.set_ignore_manifest_warnings(true);
return loader.LoadExtension(path).get();
}
// Tests that viewing a source of the options page works fine.
// This is a regression test for https://crbug.com/796080.
IN_PROC_BROWSER_TEST_F(ExtensionSettingsUIBrowserTest, ViewSource) {
// Navigate to an in-page (guest-view-based) extension options page
// and grab the WebContents hosting the options page.
const extensions::Extension* extension = InstallExtensionWithInPageOptions();
GURL options_url("chrome://extensions/?options=" + extension->id());
content::WebContents* options_contents = nullptr;
{
content::WebContentsAddedObserver options_contents_added_observer;
ui_test_utils::NavigateToURL(browser(), options_url);
options_contents = options_contents_added_observer.GetWebContents();
}
ASSERT_TRUE(options_contents);
content::WaitForLoadStop(options_contents);
EXPECT_EQ(extension->GetResourceURL("options.html"),
options_contents->GetLastCommittedURL());
// Open the view-source of the options page.
int old_tabs_count = browser()->tab_strip_model()->count();
content::WebContentsAddedObserver view_source_contents_added_observer;
options_contents->GetMainFrame()->ViewSource();
content::WebContents* view_source_contents =
view_source_contents_added_observer.GetWebContents();
ASSERT_TRUE(view_source_contents);
content::WaitForLoadStop(view_source_contents);
// Verify that the view-source is present in the tab-strip.
int new_tabs_count = browser()->tab_strip_model()->count();
EXPECT_EQ(new_tabs_count, old_tabs_count + 1);
EXPECT_EQ(view_source_contents,
browser()->tab_strip_model()->GetActiveWebContents());
// Verify the contents of the view-source tab.
std::string actual_source_text;
std::string view_source_extraction_script = R"(
output = "";
document.querySelectorAll(".line-content").forEach(function(elem) {
output += elem.innerText;
});
domAutomationController.send(output); )";
EXPECT_TRUE(content::ExecuteScriptAndExtractString(
view_source_contents, view_source_extraction_script,
&actual_source_text));
base::FilePath source_path =
test_data_dir().AppendASCII("options_page_in_view/options.html");
std::string expected_source_text;
{
base::ScopedAllowBlockingForTesting scoped_allow_blocking;
EXPECT_TRUE(base::ReadFileToString(source_path, &expected_source_text));
}
EXPECT_TRUE(
base::RemoveChars(expected_source_text, "\n", &expected_source_text));
EXPECT_EQ(expected_source_text, actual_source_text);
}
......@@ -39,7 +39,9 @@ class ExtensionSettingsUIBrowserTest : public WebUIBrowserTest {
void InstallPlatformApp();
void InstallExtensionWithInPageOptions();
// Installs chrome/test/data/extensions/options_page_in_view extension
// and returns it back to the caller. Can return null upon failure.
const extensions::Extension* InstallExtensionWithInPageOptions();
void AddManagedPolicyProvider();
......@@ -51,6 +53,8 @@ class ExtensionSettingsUIBrowserTest : public WebUIBrowserTest {
// Shrinks the web contents view in order to ensure vertical overflow.
void ShrinkWebContentsView();
const base::FilePath& test_data_dir() { return test_data_dir_; }
private:
const extensions::Extension* InstallExtension(const base::FilePath& path);
......
......@@ -152,6 +152,20 @@ void ExtensionOptionsGuest::OnPreferredSizeChanged(const gfx::Size& pref_size) {
options.ToValue()));
}
void ExtensionOptionsGuest::AddNewContents(WebContents* source,
WebContents* new_contents,
WindowOpenDisposition disposition,
const gfx::Rect& initial_rect,
bool user_gesture,
bool* was_blocked) {
if (!attached() || !embedder_web_contents()->GetDelegate())
return;
embedder_web_contents()->GetDelegate()->AddNewContents(
source, new_contents, disposition, initial_rect, user_gesture,
was_blocked);
}
WebContents* ExtensionOptionsGuest::OpenURLFromTab(
WebContents* source,
const content::OpenURLParams& params) {
......
......@@ -7,6 +7,8 @@
#include <stdint.h>
#include <memory>
#include "base/macros.h"
#include "components/guest_view/browser/guest_view.h"
#include "extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h"
......@@ -36,6 +38,12 @@ class ExtensionOptionsGuest
void OnPreferredSizeChanged(const gfx::Size& pref_size) final;
// content::WebContentsDelegate implementation.
void AddNewContents(content::WebContents* source,
content::WebContents* new_contents,
WindowOpenDisposition disposition,
const gfx::Rect& initial_rect,
bool user_gesture,
bool* was_blocked) final;
content::WebContents* OpenURLFromTab(
content::WebContents* source,
const content::OpenURLParams& params) final;
......
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