Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(248)

Unified Diff: content/browser/browser_plugin/browser_plugin_guest.cc

Issue 140073002: <webview>: navigating to WebStore should fire a loadabort instead of crashing. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Merge with ToT Created 6 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/browser_plugin/browser_plugin_guest.cc
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index c27713b236d6fdf39054870d04ee72733b72b891..2e79b6335e3c8815151c1264fd21b4362542eb13 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -406,7 +406,33 @@ void BrowserPluginGuest::LoadURLWithParams(const GURL& url,
const Referrer& referrer,
PageTransition transition_type,
WebContents* web_contents) {
- NavigationController::LoadURLParams load_url_params(url);
+ // Do not allow navigating a guest to schemes other than known safe schemes.
+ // This will block the embedder trying to load unwanted schemes, e.g.
+ // chrome://settings.
+ bool scheme_is_blocked =
+ (!ChildProcessSecurityPolicyImpl::GetInstance()->IsWebSafeScheme(
+ url.scheme()) &&
+ !ChildProcessSecurityPolicyImpl::GetInstance()->IsPseudoScheme(
+ url.scheme())) ||
+ url.SchemeIs(kJavaScriptScheme);
+ bool can_commit =
+ GetContentClient()->browser()->CanCommitURL(
+ GetWebContents()->GetRenderProcessHost(), url);
+ if (scheme_is_blocked || !url.is_valid() || !can_commit) {
+ if (delegate_) {
+ // TODO(fsamuel): Need better error reporting here.
+ std::string error_type;
+ base::RemoveChars(net::ErrorToString(net::ERR_ABORTED), "net::",
+ &error_type);
+ delegate_->LoadAbort(true /* is_top_level */, url, error_type);
+ }
+ return;
+ }
+
+ GURL validated_url(url);
+ GetWebContents()->GetRenderProcessHost()->FilterURL(false, &validated_url);
+
+ NavigationController::LoadURLParams load_url_params(validated_url);
load_url_params.referrer = referrer;
load_url_params.transition_type = transition_type;
load_url_params.extra_headers = std::string();
@@ -604,9 +630,10 @@ void BrowserPluginGuest::Initialize(
// update the BrowserPlugin's corresponding 'name' attribute.
// TODO(fsamuel): Remove this once http://crbug.com/169110 is addressed.
renderer_prefs->report_frame_name_changes = true;
- // Navigation is disabled in Chrome Apps. We want to make sure guest-initiated
- // navigations still continue to function inside the app.
- renderer_prefs->browser_handles_all_top_level_requests = false;
+ // Top-level guest-initiated navigations are all plumbed through
+ // BrowserPluginGuest::OpenURLFromTab. There, it is determined whether a
+ // particular navigation will be allowed to proceed or whether it is aborted.
+ renderer_prefs->browser_handles_all_top_level_requests = true;
// Disable "client blocked" error page for browser plugin.
renderer_prefs->disable_client_blocked_error_page = true;
@@ -828,14 +855,12 @@ WebContents* BrowserPluginGuest::OpenURLFromTab(WebContents* source,
PendingWindowMap::iterator it = opener()->pending_new_windows_.find(this);
if (it == opener()->pending_new_windows_.end())
return NULL;
- const NewWindowInfo& old_target_url = it->second;
- NewWindowInfo new_window_info(params.url, old_target_url.name);
- new_window_info.changed = new_window_info.url != old_target_url.url;
- it->second = new_window_info;
+ const NewWindowInfo& old_info = it->second;
+ it->second = NewWindowInfo(params.url, old_info.name);
return NULL;
}
if (params.disposition == CURRENT_TAB) {
- // This can happen for cross-site redirects.
+ // This can happen for cross-site redirects and top-level frame navigations.
LoadURLWithParams(params.url, params.referrer, params.transition, source);
return source;
}
@@ -1252,15 +1277,10 @@ void BrowserPluginGuest::Attach(
new_view->CreateViewForWidget(web_contents()->GetRenderViewHost());
}
- // We need to do a navigation here if the target URL has changed between
- // the time the WebContents was created and the time it was attached.
- // We also need to do an initial navigation if a RenderView was never
- // created for the new window in cases where there is no referrer.
+ // Grab the URL for the initial navigation.
PendingWindowMap::iterator it = opener()->pending_new_windows_.find(this);
if (it != opener()->pending_new_windows_.end()) {
- const NewWindowInfo& new_window_info = it->second;
- if (new_window_info.changed || !has_render_view_)
- params.src = it->second.url.spec();
+ params.src = it->second.url.spec();
} else {
NOTREACHED();
}
@@ -1452,32 +1472,11 @@ void BrowserPluginGuest::OnNavigateGuest(
const std::string& src) {
GURL url = delegate_ ? delegate_->ResolveURL(src) : GURL(src);
- // Do not allow navigating a guest to schemes other than known safe schemes.
- // This will block the embedder trying to load unwanted schemes, e.g.
- // chrome://settings.
- bool scheme_is_blocked =
- (!ChildProcessSecurityPolicyImpl::GetInstance()->IsWebSafeScheme(
- url.scheme()) &&
- !ChildProcessSecurityPolicyImpl::GetInstance()->IsPseudoScheme(
- url.scheme())) ||
- url.SchemeIs(kJavaScriptScheme);
- if (scheme_is_blocked || !url.is_valid()) {
- if (delegate_) {
- std::string error_type;
- base::RemoveChars(net::ErrorToString(net::ERR_ABORTED), "net::",
- &error_type);
- delegate_->LoadAbort(true /* is_top_level */, url, error_type);
- }
- return;
- }
-
- GURL validated_url(url);
- GetWebContents()->GetRenderProcessHost()->FilterURL(false, &validated_url);
// As guests do not swap processes on navigation, only navigations to
// normal web URLs are supported. No protocol handlers are installed for
// other schemes (e.g., WebUI or extensions), and no permissions or bindings
// can be granted to the guest process.
- LoadURLWithParams(validated_url, Referrer(), PAGE_TRANSITION_AUTO_TOPLEVEL,
+ LoadURLWithParams(url, Referrer(), PAGE_TRANSITION_AUTO_TOPLEVEL,
GetWebContents());
}
« no previous file with comments | « content/browser/browser_plugin/browser_plugin_guest.h ('k') | content/browser/renderer_host/render_view_host_impl.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698