Use content::NavigateIframeToURL in ProcessManagerBrowserTest

content/public/test/browser_test_utils.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/public/test/browser_test_utils.h"
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
@@ skipping 295 matching lines @@
 }
 
 }  // namespace
 
 bool NavigateIframeToURL(WebContents* web_contents,
                          std::string iframe_id,
                          const GURL& url) {
   // 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,
                          const std::string& query_string) {
   GURL url = net::FilePathToFileURL(path);
   if (!query_string.empty()) {
     GURL::Replacements replacements;
     replacements.SetQueryStr(query_string);
     return url.ReplaceComponents(replacements);
   }
@@ skipping 756 matching lines @@
 void FrameWatcher::WaitFrames(int frames_to_wait) {
   if (frames_to_wait <= 0)
     return;
   base::RunLoop run_loop;
   base::AutoReset<base::Closure> reset_quit(&quit_, run_loop.QuitClosure());
   base::AutoReset<int> reset_frames_to_wait(&frames_to_wait_, frames_to_wait);
   run_loop.Run();
 }
 
 }  // namespace content