Don't rewrite subframe navigation URLs

This CL makes sure we do not attempt to rewrite a subframe navigation URL in
should only be performed on main frame navigations.

NavigationControllerImpl: :CreateNavigationRequestFromLoadParams. Rewrites
Bug: 895065, 803859, 896028
Change-Id: I2a2326d802b55655d59f0c6d3d73e3060c58152b
Reviewed-on: https://chromium-review.googlesource.com/c/1282992
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601183}

content/browser/browser_url_handler_impl.cc

@@ -153,4 +153,8 @@ bool BrowserURLHandlerImpl::ReverseURLRewrite(
   return false;
 }
 
+void BrowserURLHandlerImpl::SetFixupHandlerForTesting(URLHandler handler) {
+  fixup_handler_ = handler;
+}
+
 }  // namespace content

content/browser/browser_url_handler_impl.h

@@ -38,6 +38,10 @@ class CONTENT_EXPORT BrowserURLHandlerImpl : public BrowserURLHandler {
   bool ReverseURLRewrite(GURL* url, const GURL& original,
                          BrowserContext* browser_context);
 
+  // Sets the fixup handler during tests. Unlike |SetFixupHandler|, this can be
+  // called multiple time during tests.
+  void SetFixupHandlerForTesting(URLHandler handler);
+
  private:
   // This object is a singleton:
   BrowserURLHandlerImpl();

content/browser/frame_host/navigation_controller_impl.cc

@@ -2710,23 +2710,31 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams(
   DCHECK_EQ(-1, GetIndexOfEntry(&entry));
   GURL url_to_load;
   GURL virtual_url;
-  bool reverse_on_redirect = false;
-  RewriteUrlForNavigation(params.url, browser_context_, &url_to_load,
-                          &virtual_url, &reverse_on_redirect);
-
-  // For DATA loads, override the virtual URL.
-  if (params.load_type == LOAD_TYPE_DATA)
-    virtual_url = params.virtual_url_for_data_url;
-
-  if (virtual_url.is_empty())
-    virtual_url = url_to_load;
+  // For main frames, rewrite the URL if necessary and compute the virtual URL
+  // that should be shown in the address bar.
+  if (node->IsMainFrame()) {
+    bool reverse_on_redirect = false;
+    RewriteUrlForNavigation(params.url, browser_context_, &url_to_load,
+                            &virtual_url, &reverse_on_redirect);
+
+    // For DATA loads, override the virtual URL.
+    if (params.load_type == LOAD_TYPE_DATA)
+      virtual_url = params.virtual_url_for_data_url;
+
+    if (virtual_url.is_empty())
+      virtual_url = url_to_load;
+
+    // TODO(clamy): In order to remove the pending NavigationEntry,
+    // |virtual_url| and |reverse_on_redirect| should be stored in the
+    // NavigationRequest.
+  } else {
+    url_to_load = params.url;
+    virtual_url = params.url;
+  }
 
   CHECK(!node->IsMainFrame() || virtual_url == entry.GetVirtualURL());
   CHECK_EQ(url_to_load, frame_entry->url());
 
-  // TODO(clamy): In order to remove the pending NavigationEntry, |virtual_url|
-  // and |reverse_on_redirect| should be stored in the NavigationRequest.
-
   if (!IsValidURLForNavigation(node->IsMainFrame(), virtual_url, url_to_load))
     return nullptr;
 

content/browser/frame_host/navigation_controller_impl_unittest.cc

@@ -23,6 +23,7 @@
 #include "base/test/gtest_util.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "content/browser/browser_url_handler_impl.h"
 #include "content/browser/frame_host/frame_navigation_entry.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigation_entry_screenshot_manager.h"
@@ -5306,4 +5307,42 @@ TEST_F(NavigationControllerTest, SubFrameNavigationUIData) {
 #endif
 }
 
+bool SrcDocRewriter(GURL* url, BrowserContext* browser_context) {
+  if (*url == GURL("about:srcdoc")) {
+    *url = GURL("chrome://srcdoc");
+    return true;
+  }
+  return false;
+}
+
+// Tests that receiving a request to navigate a subframe will not rewrite the
+// subframe URL. Regression test for https://crbug.com/895065.
+TEST_F(NavigationControllerTest, NoURLRewriteForSubframes) {
+  const GURL kUrl1("http://google.com");
+  const GURL kUrl2("http://chromium.org");
+  const GURL kSrcDoc("about:srcdoc");
+
+  // First, set up a handler that will rewrite srcdoc urls.
+  BrowserURLHandlerImpl::GetInstance()->SetFixupHandlerForTesting(
+      &SrcDocRewriter);
+
+  // Simulate navigating to a page that has a subframe.
+  NavigationSimulator::NavigateAndCommitFromDocument(kUrl1, main_test_rfh());
+  TestRenderFrameHost* subframe = main_test_rfh()->AppendChild("subframe");
+  NavigationSimulator::NavigateAndCommitFromDocument(kUrl2, subframe);
+
+  // Simulate the subframe receiving a request from a RenderFrameProxyHost to
+  // navigate to about:srcdoc. This should not crash.
+  FrameTreeNode* subframe_node =
+      main_test_rfh()->frame_tree_node()->child_at(0);
+  controller_impl().NavigateFromFrameProxy(
+      subframe_node->current_frame_host(), kSrcDoc,
+      true /* is_renderer_initiated */, main_test_rfh()->GetSiteInstance(),
+      Referrer(), ui::PAGE_TRANSITION_LINK,
+      false /* should_replace_current_entry */, "GET", nullptr, "", nullptr);
+
+  // Clean up the handler.
+  BrowserURLHandlerImpl::GetInstance()->SetFixupHandlerForTesting(nullptr);
+}
+
 }  // namespace content