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 205 matching lines @@
   // 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);
+  if (!ExecuteScript(window->web_contents(), script))
+    return false;
   load_observer.Wait();
-  return result;
+
+  return true;
 }
 
 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 614 matching lines @@
       "window.domAutomationController.send(location.ancestorOrigins[0]);",
       &result));
   EXPECT_EQ(result + "/", foo_url.GetOrigin().spec());
   EXPECT_TRUE(ExecuteScriptAndExtractString(
       bottom_child->current_frame_host(),
       "window.domAutomationController.send(location.ancestorOrigins[1]);",
       &result));
   EXPECT_EQ(result + "/", main_url.GetOrigin().spec());
 }
 
+// Disabled because of http://crbug.com/433012 .

[Charlie Reis, 2014/12/11 00:43:39]

nit: No space before period.

Let's also rephrase, since there's a few ways this could be fixed:
Disabled, because the script call for the title watcher triggers
http://crbug.com/433012.  Alternatively, this test could use NavigateIframeToURL
if we classified the navigation as MANUAL_SUBFRAME (and thus generated a commit
notification) or if it waited for DidStopLoading (currently broken).

[lfg, 2014/12/11 23:55:24]

Done.

+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.

[nasko, 2014/12/11 01:14:44]

I'd rephrase it a bit to be more specific.

The navigation is initiated from a script in the parent frame, so the data URL
should be committed in the same SiteInstance as the parent frame.

[lfg, 2014/12/11 23:55:24]

Done.

+  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'; };"
+      "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 .

[Charlie Reis, 2014/12/11 00:43:39]

Same as above.

[lfg, 2014/12/11 23:55:24]

Done.

+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       DISABLED_NavigateRemoteToBlankURL) {
+  GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));

[nasko, 2014/12/11 01:14:44]

Both this method and the above differ only in the URL that is being loaded. That
is too much code duplication. Why not pull all the code into a common method and
call it with the respective URL as input?

[lfg, 2014/12/11 23:55:24]

I think those two should be in the same test. The only reason I tried to split
them is to try to get around the renderer crash (which I couldn't find any way).

Since it's also very similar to NavigateRemoteFrame test, I put a TODO to merge
them all together when these issues are fixed.

+  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");

[nasko, 2014/12/11 01:14:44]

nit: this is no longer data_url

[lfg, 2014/12/11 23:55:24]

Done.

+  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