Allow Signin page to open other chrome:// URLs if login content in <webview>

extensions/browser/guest_view/web_view/web_view_guest.cc

Index: extensions/browser/guest_view/web_view/web_view_guest.cc
diff --git a/extensions/browser/guest_view/web_view/web_view_guest.cc b/extensions/browser/guest_view/web_view/web_view_guest.cc
index fce1b6d0dd7301210ed877a2f3b7f2794649e4fe..081f6c9969092d788594e9e6e8f08dda3106ac03 100644
--- a/extensions/browser/guest_view/web_view/web_view_guest.cc
+++ b/extensions/browser/guest_view/web_view/web_view_guest.cc
@@ -875,35 +875,9 @@ void WebViewGuest::NavigateGuest(const std::string& src,
 
   GURL url = ResolveURL(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 =
-      (!content::ChildProcessSecurityPolicy::GetInstance()->IsWebSafeScheme(
-           url.scheme()) &&
-       !url.SchemeIs(url::kAboutScheme)) ||
-      url.SchemeIs(url::kJavaScriptScheme);
-  if (scheme_is_blocked || !url.is_valid()) {
-    LoadAbort(true /* is_top_level */, url,
-              net::ErrorToShortString(net::ERR_ABORTED));
-    NavigateGuest(url::kAboutBlankURL, true /* force_navigation */);
-    return;
-  }
-  if (!force_navigation && (src_ == url))
-    return;
-
-  GURL validated_url(url);
-  web_contents()->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,
-                    content::Referrer(),
+  LoadURLWithParams(url, content::Referrer(),
                     ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
-                    web_contents());
-
-  src_ = validated_url;
+                    force_navigation);
 }
 
 bool WebViewGuest::HandleKeyboardShortcuts(
@@ -1118,6 +1092,23 @@ void WebViewGuest::AddNewContents(content::WebContents* source,
 content::WebContents* WebViewGuest::OpenURLFromTab(
     content::WebContents* source,
     const content::OpenURLParams& params) {
+  // There are two use cases to consider from a security perspective:
+  // 1.) Renderer-initiated navigation to chrome:// must always be blocked even
+  //     if the <webview> is in WebUI. This is handled by
+  //     WebViewGuest::LoadURLWithParams. WebViewGuest::NavigateGuest will also
+  //     call LoadURLWithParams. CreateNewGuestWebViewWindow creates a new
+  //     WebViewGuest which will call NavigateGuest in DidInitialize.
+  // 2.) The Language Settings context menu item should always work, both in
+  //     Chrome Apps and WebUI. This is a browser initiated request and so
+  //     we pass it along to the embedder's WebContentsDelegate to get the
+  //     browser to perform the action for the <webview>.
+  if (!params.is_renderer_initiated) {
+    if (!owner_web_contents()->GetDelegate())
+      return nullptr;
+    return owner_web_contents()->GetDelegate()->OpenURLFromTab(
+        owner_web_contents(), params);
+  }
+
   // If the guest wishes to navigate away prior to attachment then we save the
   // navigation to perform upon attachment. Navigation initializes a lot of
   // state that assumes an embedder exists, such as RenderWidgetHostViewGuest.
@@ -1134,12 +1125,25 @@ content::WebContents* WebViewGuest::OpenURLFromTab(
     it->second = new_window_info;
     return nullptr;
   }
+
+  // This code path is taken if RenderFrameImpl::DecidePolicyForNavigation
+  // decides that a fork should happen. At the time of writing this comment,
+  // the only way a well behaving guest could hit this code path is if it
+  // navigates to a URL that's associated with the default search engine.
+  // This list of URLs is generated by chrome::GetSearchURLs. Validity checks
+  // are performed inside LoadURLWithParams such that if the guest attempts
+  // to navigate to a URL that it is not allowed to navigate to, a 'loadabort'
+  // event will fire in the embedder, and the guest will be navigated to
+  // about:blank.
   if (params.disposition == CURRENT_TAB) {
-    // This can happen for cross-site redirects.
-    LoadURLWithParams(params.url, params.referrer, params.transition, source);
-    return source;
+    LoadURLWithParams(params.url, params.referrer, params.transition,
+                      true /* force_navigation */);
+    return web_contents();
   }
 
+  // This code path is taken if Ctrl+Click, middle click or any of the
+  // keyboard/mouse combinations are used to open a link in a new tab/window.
+  // This code path is also taken on client-side redirects from about:blank.
   CreateNewGuestWebViewWindow(params);
   return nullptr;
 }
@@ -1161,8 +1165,32 @@ void WebViewGuest::WebContentsCreated(WebContents* source_contents,
 void WebViewGuest::LoadURLWithParams(const GURL& url,
                                      const content::Referrer& referrer,
                                      ui::PageTransition transition_type,
-                                     content::WebContents* web_contents) {
-  content::NavigationController::LoadURLParams load_url_params(url);
+                                     bool force_navigation) {
+  // 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.

[Charlie Reis, 2015/02/19 22:56:28]

nit: chrome://
(settings isn't part of the scheme)

[Fady Samuel, 2015/02/20 00:33:06]

Done.

+  bool scheme_is_blocked =
+      (!content::ChildProcessSecurityPolicy::GetInstance()->IsWebSafeScheme(
+           url.scheme()) &&
+       !url.SchemeIs(url::kAboutScheme)) ||
+      url.SchemeIs(url::kJavaScriptScheme);
+  if (scheme_is_blocked || !url.is_valid()) {
+    LoadAbort(true /* is_top_level */, url,
+              net::ErrorToShortString(net::ERR_ABORTED));
+    NavigateGuest(url::kAboutBlankURL, true /* force_navigation */);
+    return;
+  }
+
+  if (!force_navigation && (src_ == url))
+    return;
+
+  GURL validated_url(url);
+  web_contents()->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.
+  content::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();
@@ -1170,7 +1198,9 @@ void WebViewGuest::LoadURLWithParams(const GURL& url,
     load_url_params.override_user_agent =
         content::NavigationController::UA_OVERRIDE_TRUE;
   }
-  web_contents->GetController().LoadURLWithParams(load_url_params);
+  web_contents()->GetController().LoadURLWithParams(load_url_params);
+
+  src_ = validated_url;
 }
 
 void WebViewGuest::RequestNewWindowPermission(