OOPIF: Data URLs are now rendered in the renderer that initiated the navigation.

content/browser/site_per_process_browsertest.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/browser/site_per_process_browsertest.h"
 
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/frame_host/render_widget_host_view_child_frame.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ skipping 139 matching lines @@
 SitePerProcessBrowserTest::SitePerProcessBrowserTest() {
 };
 
 void SitePerProcessBrowserTest::StartFrameAtDataURL() {
   std::string data_url_script =
       "var iframes = document.getElementById('test');iframes.src="
       "'data:text/html,dataurl';";
   ASSERT_TRUE(ExecuteScript(shell()->web_contents(), data_url_script));
 }
 
-bool SitePerProcessBrowserTest::NavigateIframeToURL(Shell* window,
+void SitePerProcessBrowserTest::NavigateIframeToURL(Shell* window,

[Charlie Reis, 2014/12/06 00:18:51]

Please don't change this.  It's more useful to have the EXPECT_TRUE line in the
test than in the helper function, because the top line number will be the line
of the test that's to blame.

[lfg, 2014/12/08 20:45:33]

I agree it's more useful to see the test immediately, however the way it was
currently implemented is broken, because if ExecuteScript() fails, then the
load_observer will never return from the message loop and the test will time
out.

[Charlie Reis, 2014/12/09 19:14:37]

Ah, good point.  Instead, let's return false before calling load_observer.Wait()
if ExecuteScript returns false.  That would let this still return a bool and
have the EXPECT_TRUE in the test itself.

                                                     const GURL& url,
                                                     std::string iframe_id) {
   // TODO(creis): This should wait for LOAD_STOP, but cross-site subframe
   // navigations generate extra DidStartLoading and DidStopLoading messages.
   // Until we replace swappedout:// with frame proxies, we need to listen for
   // something else.  For now, we trigger NEW_SUBFRAME navigations and listen
   // for commit.
   std::string script = base::StringPrintf(
       "setTimeout(\""
       "var iframes = document.getElementById('%s');iframes.src='%s';"
       "\",0)",
       iframe_id.c_str(), url.spec().c_str());
   WindowedNotificationObserver load_observer(
       NOTIFICATION_NAV_ENTRY_COMMITTED,
       Source<NavigationController>(
           &window->web_contents()->GetController()));
-  bool result = ExecuteScript(window->web_contents(), script);
+  EXPECT_TRUE(ExecuteScript(window->web_contents(), script));
   load_observer.Wait();
-  return result;
 }
 
 void SitePerProcessBrowserTest::SetUpCommandLine(CommandLine* command_line) {
   command_line->AppendSwitch(switches::kSitePerProcess);
 };
 
 void SitePerProcessBrowserTest::SetUpOnMainThread() {
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
   SetupCrossSiteRedirector(embedded_test_server());
@@ skipping 156 matching lines @@
   GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
   NavigateToURL(shell(), main_url);
 
   StartFrameAtDataURL();
 
   // These must stay in scope with replace_host.
   GURL::Replacements replace_host;
   std::string foo_com("foo.com");
 
   // Load cross-site page into iframe.
-  EXPECT_TRUE(NavigateIframeToURL(
-      shell(),
-      embedded_test_server()->GetURL("/cross-site/foo.com/title2.html"),
-      "test"));
+  NavigateIframeToURL(shell(), embedded_test_server()->GetURL(
+                                   "/cross-site/foo.com/title2.html"),
+                      "test");
 
   // Check the subframe process.
   FrameTreeNode* root =
       static_cast<WebContentsImpl*>(shell()->web_contents())->
           GetFrameTree()->root();
   ASSERT_EQ(1U, root->child_count());
   FrameTreeNode* child = root->child_at(0);
   EXPECT_EQ(main_url, root->current_url());
   EXPECT_EQ("foo.com", child->current_url().host());
   EXPECT_EQ("/title2.html", child->current_url().path());
@@ skipping 53 matching lines @@
   GURL https_url(https_server.GetURL("files/title1.html"));
 
   NavigateToURL(shell(), main_url);
 
   SitePerProcessWebContentsObserver observer(shell()->web_contents());
   {
     // Load cross-site client-redirect page into Iframe.
     // Should be blocked.
     GURL client_redirect_https_url(https_server.GetURL(
         "client-redirect?files/title1.html"));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    client_redirect_https_url, "test"));
+    NavigateIframeToURL(shell(), client_redirect_https_url, "test");
     // DidFailProvisionalLoad when navigating to client_redirect_https_url.
     EXPECT_EQ(observer.navigation_url(), client_redirect_https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
   }
 
   {
     // Load cross-site server-redirect page into Iframe,
     // which redirects to same-site page.
     GURL server_redirect_http_url(https_server.GetURL(
         "server-redirect?" + http_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), server_redirect_http_url, "test");
     EXPECT_EQ(observer.navigation_url(), http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
   }
 
   {
     // Load cross-site server-redirect page into Iframe,
     // which redirects to cross-site page.
     GURL server_redirect_http_url(https_server.GetURL(
         "server-redirect?files/title1.html"));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), server_redirect_http_url, "test");
     // DidFailProvisionalLoad when navigating to https_url.
     EXPECT_EQ(observer.navigation_url(), https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
   }
 
   {
     // Load same-site server-redirect page into Iframe,
     // which redirects to cross-site page.
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?" + https_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), server_redirect_http_url, "test");
 
     EXPECT_EQ(observer.navigation_url(), https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
    }
 
   {
     // Load same-site client-redirect page into Iframe,
     // which redirects to cross-site page.
     GURL client_redirect_http_url(test_server()->GetURL(
         "client-redirect?" + https_url.spec()));
 
     RedirectNotificationObserver load_observer2(
         NOTIFICATION_LOAD_STOP,
         Source<NavigationController>(
             &shell()->web_contents()->GetController()));
 
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    client_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), client_redirect_http_url, "test");
 
     // Same-site Client-Redirect Page should be loaded successfully.
     EXPECT_EQ(observer.navigation_url(), client_redirect_http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
 
     // Redirecting to Cross-site Page should be blocked.
     load_observer2.Wait();
     EXPECT_EQ(observer.navigation_url(), https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
   }
 
   {
     // Load same-site server-redirect page into Iframe,
     // which redirects to same-site page.
-    GURL server_redirect_http_url(test_server()->GetURL(
-        "server-redirect?files/title1.html"));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    GURL server_redirect_http_url(
+        test_server()->GetURL("server-redirect?files/title1.html"));
+    NavigateIframeToURL(shell(), server_redirect_http_url, "test");
     EXPECT_EQ(observer.navigation_url(), http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
    }
 
   {
     // Load same-site client-redirect page into Iframe,
     // which redirects to same-site page.
     GURL client_redirect_http_url(test_server()->GetURL(
         "client-redirect?" + http_url.spec()));
     RedirectNotificationObserver load_observer2(
         NOTIFICATION_LOAD_STOP,
         Source<NavigationController>(
             &shell()->web_contents()->GetController()));
 
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    client_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), client_redirect_http_url, "test");
 
     // Same-site Client-Redirect Page should be loaded successfully.
     EXPECT_EQ(observer.navigation_url(), client_redirect_http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
 
     // Redirecting to Same-site Page should be loaded successfully.
     load_observer2.Wait();
     EXPECT_EQ(observer.navigation_url(), http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
   }
@@ skipping 26 matching lines @@
         "client-redirect?" + http_url.spec()));
     GURL client_redirect_http_url(test_server()->GetURL(
         "client-redirect?" + client_redirect_https_url.spec()));
 
     // We should wait until second client redirect get cancelled.
     RedirectNotificationObserver load_observer2(
         NOTIFICATION_LOAD_STOP,
         Source<NavigationController>(
             &shell()->web_contents()->GetController()));
 
-    EXPECT_TRUE(NavigateIframeToURL(shell(), client_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), client_redirect_http_url, "test");
 
     // DidFailProvisionalLoad when navigating to client_redirect_https_url.
     load_observer2.Wait();
     EXPECT_EQ(observer.navigation_url(), client_redirect_https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
   }
 
   {
     // Load server-redirect page pointing to a cross-site server-redirect page,
     // which eventually redirect back to same-site page.
     GURL server_redirect_https_url(https_server.GetURL(
         "server-redirect?" + http_url.spec()));
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?" + server_redirect_https_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(),
-                                    server_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), server_redirect_http_url, "test");
     EXPECT_EQ(observer.navigation_url(), http_url);
     EXPECT_TRUE(observer.navigation_succeeded());
   }
 
   {
     // Load server-redirect page pointing to a cross-site server-redirect page,
     // which eventually redirects back to cross-site page.
     GURL server_redirect_https_url(https_server.GetURL(
         "server-redirect?" + https_url.spec()));
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?" + server_redirect_https_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(), server_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), server_redirect_http_url, "test");
 
     // DidFailProvisionalLoad when navigating to https_url.
     EXPECT_EQ(observer.navigation_url(), https_url);
     EXPECT_FALSE(observer.navigation_succeeded());
   }
 
   {
     // Load server-redirect page pointing to a cross-site client-redirect page,
     // which eventually redirects back to same-site page.
     GURL client_redirect_http_url(https_server.GetURL(
         "client-redirect?" + http_url.spec()));
     GURL server_redirect_http_url(test_server()->GetURL(
         "server-redirect?" + client_redirect_http_url.spec()));
-    EXPECT_TRUE(NavigateIframeToURL(shell(), server_redirect_http_url, "test"));
+    NavigateIframeToURL(shell(), server_redirect_http_url, "test");
 
     // DidFailProvisionalLoad when navigating to client_redirect_http_url.
     EXPECT_EQ(observer.navigation_url(), client_redirect_http_url);
     EXPECT_FALSE(observer.navigation_succeeded());
   }
 }
 
 // Ensure that when navigating a frame cross-process RenderFrameProxyHosts are
 // created in the FrameTree skipping the subtree of the navigating frame.
 //
@@ skipping 99 matching lines @@
           child->child_at(i)->render_manager()->GetRenderFrameProxyHost(site2));
     }
 
     navigation_observer.Wait();
     EXPECT_TRUE(observer.navigation_succeeded());
     EXPECT_EQ(cross_site_url, observer.navigation_url());
     EXPECT_EQ(0U, child->child_count());
   }
 }
 
+// Disabled because of http://crbug.com/433012 .

[Charlie Reis, 2014/12/06 00:18:51]

If we can avoid using script to check the title below, will this pass (similarly
to NavigateRemoteFrame)?

[lfg, 2014/12/08 20:45:33]

Yes, it would. If we don't depend on the renderer after we send the load
request, it'll pass.

+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       DISABLED_NavigateRemoteToDataURL) {
+  GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
+  NavigateToURL(shell(), main_url);
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+
+  SitePerProcessWebContentsObserver observer(shell()->web_contents());
+
+  // Load cross-site page into iframe.
+  GURL url = embedded_test_server()->GetURL("foo.com", "/title1.html");
+  NavigateFrameToURL(root->child_at(0), url);
+  EXPECT_TRUE(observer.navigation_succeeded());
+  EXPECT_EQ(url, observer.navigation_url());
+
+  // Ensure that we have created a new process for the subframe.
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
+            root->child_at(0)->current_frame_host()->GetSiteInstance());
+
+  // Navigate iframe to a data URL. The navigation happens from a script in the
+  // parent renderer, so the data URL should be rendered in the parent renderer.
+  GURL data_url("data:text/html,dataurl");
+  std::string script = base::StringPrintf(
+      "setTimeout(function() {"
+      "var iframe = document.getElementById('test');"
+      "iframe.onload = function() { document.title = 'LOADED'; };"

[Charlie Reis, 2014/12/06 00:18:51]

Can we avoid using the title?  I don't understand your comment about
NAV_ENTRY_COMMITTED not working due to crbug.com/236848.  It should work for
anything past the first navigation in the frame, and we're using it in other
tests in this class.

[lfg, 2014/12/08 20:45:33]

RenderFrameHostImpl::IsCrossProcessSubframe() returns false, since we are
navigating to the same site instance as the parent frame. That causes
NavigationControllerImpl::ClassifyNavigation() to classify the navigation as
NAVIGATION_TYPE_AUTO_SUBFRAME, which causes
NavigationControllerImpl::RendererDidNavigate() to early return and not send the
notification.

[Charlie Reis, 2014/12/09 19:14:37]

Ah!  Good find.

We could try to revise that check in ClassifyNavigation, since it's just a
temporary hack.  That will let these tests listen for commit and pass.

And actually, I'm curious about why we get page ID -1 in that case.  When it
comes back into the parent process, it should get the page ID from the existing
RenderView (shared with the main frame's RenderFrame).  I wonder if we're
incorrectly constructing a new RenderView?

Fixing that might be better than changing ClassifyNavigation.

+      "iframe.src=\"%s\";"
+      "},0);",
+      data_url.spec().c_str());
+  base::string16 passed_string(base::UTF8ToUTF16("LOADED"));
+  TitleWatcher title_watcher(shell()->web_contents(), passed_string);
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
+  (void)title_watcher.WaitAndGetTitle();
+  EXPECT_TRUE(observer.navigation_succeeded());
+  EXPECT_EQ(data_url, observer.navigation_url());
+
+  // Ensure that we have navigated using the top level process.
+  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
+            root->child_at(0)->current_frame_host()->GetSiteInstance());
+}
+
+// Disabled because of http://crbug.com/433012 .
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       DISABLED_NavigateRemoteBlankURL) {

[Charlie Reis, 2014/12/06 00:18:51]

nit: NavigateRemoteToBlankURL

[lfg, 2014/12/08 20:45:33]

Done.

+  GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
+  NavigateToURL(shell(), main_url);
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+
+  SitePerProcessWebContentsObserver observer(shell()->web_contents());
+
+  // Load cross-site page into iframe.
+  GURL url = embedded_test_server()->GetURL("foo.com", "/title1.html");
+  NavigateFrameToURL(root->child_at(0), url);
+  EXPECT_TRUE(observer.navigation_succeeded());
+  EXPECT_EQ(url, observer.navigation_url());
+
+  // Ensure that we have created a new process for the subframe.
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
+            root->child_at(0)->current_frame_host()->GetSiteInstance());
+
+  // Navigate iframe to a data URL. The navigation happens from a script in the
+  // parent renderer, so the data URL should be rendered in the parent renderer.
+  GURL data_url("about:blank");
+  std::string script = base::StringPrintf(
+      "setTimeout(function() {"
+      "var iframe = document.getElementById('test');"
+      "iframe.onload = function() { document.title = 'LOADED'; };"
+      "iframe.src=\"%s\";"
+      "},0);",
+      data_url.spec().c_str());
+  base::string16 passed_string(base::UTF8ToUTF16("LOADED"));
+  TitleWatcher title_watcher(shell()->web_contents(), passed_string);
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
+  (void)title_watcher.WaitAndGetTitle();
+  EXPECT_TRUE(observer.navigation_succeeded());
+  EXPECT_EQ(data_url, observer.navigation_url());
+
+  // Ensure that we have navigated using the top level process.
+  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
+            root->child_at(0)->current_frame_host()->GetSiteInstance());
+}
+
 }  // namespace content