Use content::NavigateIframeToURL in ProcessManagerBrowserTest

content/public/test/browser_test_utils.cc

Index: content/public/test/browser_test_utils.cc
diff --git a/content/public/test/browser_test_utils.cc b/content/public/test/browser_test_utils.cc
index 79fd6170245e383d4f99aa70ed7ff58aec365016..895bfb41ddb99da42b6d8ca40c973b5f259ff6c3 100644
--- a/content/public/test/browser_test_utils.cc
+++ b/content/public/test/browser_test_utils.cc
@@ -313,19 +313,23 @@ bool NavigateIframeToURL(WebContents* web_contents,
   // 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.  See https://crbug.com/436250.
+  // something else. See https://crbug.com/436250.

[Charlie Reis, 2015/12/11 22:15:29]

I've started reviewing this CL, but I'm wondering why we can't go further and
resolve this TODO.  At the time this method was added, we generated too many
DidStartLoading/DidStopLoading events for OOPIFs.  I think fdegans resolved that
in https://crbug.com/470082.

I'm giving it a try in https://codereview.chromium.org/1522443004/.  If that
ends up having problems, we can discuss the approach you're considering here.

[robwu, 2015/12/16 09:28:34]

Your patch works, so I have reduced the patch to a fix for the
PluginWithRemoteTopFrame test + getting ProcessManagerBrowserTest to use your
upgraded NavigateIframeToURL.

(the functional difference between my method and your method is that your method
ignores unsuccessful load, while mine treats "load errors" as a failure. This is
also how I found that the PluginWithRemoteTopFrame test was flawed).

   std::string script = base::StringPrintf(
-      "setTimeout(\""
-      "var iframes = document.getElementById('%s');iframes.src='%s';"
-      "\",0)",
+      "var frame = document.getElementById('%s'), timer;"
+      "frame.onload = frame.onerror = function(event) {"
+      // This timer is ugly, but it is a general way to deal with the issue
+      // mentioned above. Assume that the load really finished when we don't
+      // receive any notifications for some arbitrarily chosen time.
+      "  clearTimeout(timer);"
+      "  timer = setTimeout(function() {"
+      "    domAutomationController.send(event.type === 'load');"
+      "  }, 500);"
+      "};"
+      "frame.src = '%s';",
       iframe_id.c_str(), url.spec().c_str());
-  WindowedNotificationObserver load_observer(
-      NOTIFICATION_NAV_ENTRY_COMMITTED,
-      Source<NavigationController>(&web_contents->GetController()));
-  bool result = ExecuteScript(web_contents, script);
-  load_observer.Wait();
-  return result;
+  bool is_loaded = false;
+  bool result = ExecuteScriptAndExtractBool(web_contents, script, &is_loaded);
+  return result && is_loaded;
 }
 
 GURL GetFileUrlWithQuery(const base::FilePath& path,