Use content::NavigateIframeToURL in ProcessManagerBrowserTest

chrome/browser/chrome_site_per_process_browsertest.cc

 // Copyright 2014 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 "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"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
@@ skipping 79 matching lines @@
   EXPECT_TRUE(is_object_created);
   EXPECT_TRUE(ExecuteScript(frame_host,
                             "window.webkitRequestFileSystem("
                             "window.TEMPORARY, 1024, function() {});"));
 }
 
 // Ensure that creating a plugin in a cross-site subframe doesn't crash.  This
 // 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);
 }