Use content::NavigateIframeToURL in ProcessManagerBrowserTest

chrome/browser/chrome_site_per_process_browsertest.cc

Index: chrome/browser/chrome_site_per_process_browsertest.cc
diff --git a/chrome/browser/chrome_site_per_process_browsertest.cc b/chrome/browser/chrome_site_per_process_browsertest.cc
index 11e2c848c7b219674dbfc483d5b3c9e704c080c8..63d50a1ff6c1c49b566ceb31dcb571259c2062e2 100644
--- a/chrome/browser/chrome_site_per_process_browsertest.cc
+++ b/chrome/browser/chrome_site_per_process_browsertest.cc
@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -102,21 +104,43 @@ IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessTest,
 // involves querying content settings from the renderer process and using the
 // top frame's origin as one of the parameters.  See https://crbug.com/426658.
 IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessTest, PluginWithRemoteTopFrame) {
-  GURL main_url(embedded_test_server()->GetURL("a.com", "/iframe.html"));
+  // Serve from the root so that flash_object.html can load the swf file.

[Charlie Reis, 2015/12/16 18:44:58]

I don't understand why these changes are necessary.  What didn't work before? 
(Looks like this was added in Alex's
https://codereview.chromium.org/1168913002.)

[Charlie Reis, 2015/12/16 20:41:37]

Oh, sorry, I missed this part:

On 2015/12/16 09:28:34, robwu wrote:
This part looks good as well, then, if it corrects the flawed test.  Just the
one question about the check below remaining.

[alexmos, 2015/12/17 02:04:30]

Thanks for fixing this!  I probably didn't catch this when trying out the test
with and without my fix in r345104, as it was testing the plugin creation path
which used to crash even before actually starting to load the SWF.

+1 for Charlie's comment below.

+  base::FilePath test_data_dir;
+  CHECK(base::PathService::Get(base::DIR_SOURCE_ROOT, &test_data_dir));
+  embedded_test_server()->ServeFilesFromDirectory(test_data_dir);
+
+  GURL main_url(
+      embedded_test_server()->GetURL("a.com", "/chrome/test/data/iframe.html"));
   ui_test_utils::NavigateToURL(browser(), main_url);
 
   // Navigate subframe to a page with a Flash object.
   content::WebContents* active_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   GURL frame_url =
-      embedded_test_server()->GetURL("b.com", "/flash_object.html");
-  content::DOMMessageQueue msg_queue;
+      embedded_test_server()->GetURL("b.com",
+                                     "/chrome/test/data/flash_object.html");
+
   EXPECT_TRUE(NavigateIframeToURL(active_web_contents, "test", frame_url));
 
+  content::RenderFrameHost* render_frame_host =
+      content::FrameMatchingPredicate(active_web_contents,
+                                      base::Bind(&content::FrameMatchesName,
+                                                 "test"));
+  ASSERT_TRUE(render_frame_host);
+  std::string script(
+      "if (performance.timing.loadEventStart) {"
+      "  domAutomationController.send(true);"
+      "} else {"
+      "  window.addEventListener('load', function() {"
+      "    domAutomationController.send(false);"
+      "  });"
+      "}");

[Charlie Reis, 2015/12/16 18:44:58]

Is this basically a sanity check that NavigateIframeToURL is waiting long
enough?  That feels like something that should go in a general test of
NavigateIframeToURL in content/, rather than embedded in this random test.  Then
we can just call NavigateIframeToURL here and leave out any extra waiting.

[robwu, 2015/12/18 09:18:01]

I've just removed this check. NavigateIframeToURL should only complete when
onload has fired.

+  bool was_onload_triggered = false;
   // Ensure the page finishes loading without crashing.
-  std::string status;
-  while (msg_queue.WaitForMessage(&status)) {
-    if (status == "\"DONE\"")
-      break;
-  }
+  EXPECT_TRUE(ExecuteScriptAndExtractBool(render_frame_host, script,
+                                          &was_onload_triggered));
+  // NavigateIframeToURL returns when the load event was triggered on the iframe
+  // element. This usually happens when the load event was fired in the child
+  // frame, so we check it here as well.
+  EXPECT_TRUE(was_onload_triggered);
 }