Only disallow top-level navigations in platform apps

content/renderer/render_view_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_view_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <string>
 #include <vector>
@@ skipping 371 matching lines @@
     base::KillProcess(base::GetCurrentProcessHandle(), 1, false);
   } else if (url == GURL(chrome::kChromeUIHangURL)) {
     for (;;) {
       base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(1));
     }
   } else if (url == GURL(chrome::kChromeUIShorthangURL)) {
     base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(20));
   }
 }
 
+// Returns false unless this is a top-level navigation.
+static bool IsTopLevelNavigation(WebKit::WebFrame* frame) {
+  return frame->parent() == NULL;
+}
+
+// Returns false unless this is a top-level navigation that crosses origins.
+static bool IsNonLocalTopLevelNavigation(const GURL& url,
+                                         WebKit::WebFrame* frame,

[darin (slow to review), 2012/05/21 22:09:54]

nit: should not need the WebKit:: prefixes due to 'using WebKit::WebFoo" at the
top of the file.

[Mihai Parparita -not on Chrome, 2012/05/21 22:13:01]

Done.

+                                         WebKit::WebNavigationType type) {
+  if (!IsTopLevelNavigation(frame))
+    return false;
+
+  // Navigations initiated within Webkit are not sent out to the external host
+  // in the following cases.
+  // 1. The url scheme is not http/https
+  // 2. The origin of the url and the opener is the same in which case the
+  //    opener relationship is maintained.
+  // 3. Reloads/form submits/back forward navigations
+  if (!url.SchemeIs(chrome::kHttpScheme) && !url.SchemeIs(chrome::kHttpsScheme))

[abarth-chromium, 2012/05/21 20:44:19]

What about FTP?  Whenever you find yourself listing schemes like this, you've
most likely got a bug.

+    return false;
+
+  // Not interested in reloads/form submits/resubmits/back forward navigations.
+  if (type != WebKit::WebNavigationTypeReload &&
+      type != WebKit::WebNavigationTypeFormSubmitted &&
+      type != WebKit::WebNavigationTypeFormResubmitted &&
+      type != WebKit::WebNavigationTypeBackForward) {
+    // The opener relationship between the new window and the parent allows the
+    // new window to script the parent and vice versa. This is not allowed if
+    // the origins of the two domains are different. This can be treated as a
+    // top level navigation and routed back to the host.
+    WebKit::WebFrame* opener = frame->opener();
+    if (!opener) {
+      return true;
+    } else {

[darin (slow to review), 2012/05/21 22:09:54]

nit: no need for "else" after "return"

[Mihai Parparita -not on Chrome, 2012/05/21 22:13:01]

Done.

+      if (url.GetOrigin() != GURL(opener->document().url()).GetOrigin())

[abarth-chromium, 2012/05/21 20:44:19]

This isn't the right way to do an origin comparison.  You need to use the
WebSecurityOrigin class to do origin comparisons.

+        return true;
+    }
+  }
+  return false;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 struct RenderViewImpl::PendingFileChooser {
   PendingFileChooser(const content::FileChooserParams& p,
                      WebFileChooserCompletion* c)
       : params(p),
         completion(c) {
   }
   content::FileChooserParams params;
   WebFileChooserCompletion* completion;  // MAY BE NULL to skip callback.
@@ skipping 1966 matching lines @@
     WebString origin_str = frame->document().securityOrigin().toString();
     GURL frame_url(origin_str.utf8().data());
     // TODO(cevans): revisit whether this origin check is still necessary once
     // crbug.com/101395 is fixed.
     if (frame_url.GetOrigin() != url.GetOrigin()) {
       OpenURL(frame, url, referrer, default_policy);
       return WebKit::WebNavigationPolicyIgnore;
     }
   }
 
-  // If the browser is interested, then give it a chance to look at top level
-  // navigations.
+  // If the browser is interested, then give it a chance to look at the request.
   if (is_content_initiated) {
-    bool browser_handles_top_level_requests =
-        renderer_preferences_.browser_handles_top_level_requests &&
+    bool browser_handles_request =
+        renderer_preferences_.browser_handles_non_local_top_level_requests &&
         IsNonLocalTopLevelNavigation(url, frame, type);
-    if (browser_handles_top_level_requests ||
-        renderer_preferences_.browser_handles_all_requests) {
+    if (!browser_handles_request) {
+      browser_handles_request =
+          renderer_preferences_.browser_handles_all_top_level_requests &&
+          IsTopLevelNavigation(frame);
+    }
+
+    if (browser_handles_request) {
       // Reset these counters as the RenderView could be reused for the next
       // navigation.
       page_id_ = -1;
       last_page_id_sent_to_browser_ = -1;
       OpenURL(frame, url, referrer, default_policy);
       return WebKit::WebNavigationPolicyIgnore;  // Suppress the load here.
     }
   }
 
   // Detect when we're crossing a permission-based boundary (e.g. into or out of
@@ skipping 2921 matching lines @@
                                             &override_state))
     return override_state;
   return current_state;
 }
 
 WebKit::WebUserMediaClient* RenderViewImpl::userMediaClient() {
   EnsureMediaStreamImpl();
   return media_stream_impl_;
 }
 
-bool RenderViewImpl::IsNonLocalTopLevelNavigation(
-    const GURL& url, WebKit::WebFrame* frame, WebKit::WebNavigationType type) {
-  // Must be a top level frame.
-  if (frame->parent() != NULL)
-    return false;
-
-  // Navigations initiated within Webkit are not sent out to the external host
-  // in the following cases.
-  // 1. The url scheme is not http/https
-  // 2. The origin of the url and the opener is the same in which case the
-  //    opener relationship is maintained.
-  // 3. Reloads/form submits/back forward navigations
-  if (!url.SchemeIs(chrome::kHttpScheme) && !url.SchemeIs(chrome::kHttpsScheme))
-    return false;
-
-  // Not interested in reloads/form submits/resubmits/back forward navigations.
-  if (type != WebKit::WebNavigationTypeReload &&
-      type != WebKit::WebNavigationTypeFormSubmitted &&
-      type != WebKit::WebNavigationTypeFormResubmitted &&
-      type != WebKit::WebNavigationTypeBackForward) {
-    // The opener relationship between the new window and the parent allows the
-    // new window to script the parent and vice versa. This is not allowed if
-    // the origins of the two domains are different. This can be treated as a
-    // top level navigation and routed back to the host.
-    WebKit::WebFrame* opener = frame->opener();
-    if (!opener) {
-      return true;
-    } else {
-      if (url.GetOrigin() != GURL(opener->document().url()).GetOrigin())
-        return true;
-    }
-  }
-  return false;
-}
-
 void RenderViewImpl::OnAsyncFileOpened(
     base::PlatformFileError error_code,
     IPC::PlatformFileForTransit file_for_transit,
     int message_id) {
   pepper_delegate_.OnAsyncFileOpened(
       error_code,
       IPC::PlatformFileForTransitToPlatformFile(file_for_transit),
       message_id);
 }
 
@@ skipping 39 matching lines @@
 bool RenderViewImpl::WebWidgetHandlesCompositorScheduling() const {
   return !!RenderThreadImpl::current()->compositor_thread();
 }
 
 void RenderViewImpl::OnJavaBridgeInit() {
   DCHECK(!java_bridge_dispatcher_);
 #if defined(ENABLE_JAVA_BRIDGE)
   java_bridge_dispatcher_ = new JavaBridgeDispatcher(this);
 #endif
 }