Reland r251495: Re-enable prerender RemovingLink browser tests.

The tests have changed significantly since they were first disabled. Add a
WaitForStop or two for good measure, but leave them as-is for the most part.
They can be disabled again if they still flake. Merge their custom prerender
loader into the main one; it's mostly the same.

In addition, for better test coverage, add a new test which asserts on events
received when a <link rel=prerender> is added for an existing prerender after
that prerender has loaded. Significantly rework the prerender events logic
to allow the test framework to wait on an event being received in the loader.

Fix implementation of set_loader_query_and_fragment to not produce URLs with
two ?s.

Original Review URL: https://codereview.chromium.org/142013004

BUG=167340, 128841

Review URL: https://codereview.chromium.org/166273007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252156 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/prerender/prerender_browsertest.cc

@@ -1280,56 +1280,46 @@ class PrerenderBrowserTest : virtual public InProcessBrowserTest {
     return prerender_link_manager;
   }
 
-  bool DidReceivePrerenderStartEventForLinkNumber(int index) const {
-    bool received_prerender_started;
+  int GetPrerenderEventCount(int index, const std::string& type) const {
+    int event_count;
     std::string expression = base::StringPrintf(
-        "window.domAutomationController.send(Boolean("
-            "receivedPrerenderStartEvents[%d]))", index);
+        "window.domAutomationController.send("
+        "    GetPrerenderEventCount(%d, '%s'))", index, type.c_str());
 
-    CHECK(content::ExecuteScriptAndExtractBool(
-        GetActiveWebContents(),
-        expression,
-        &received_prerender_started));
-    return received_prerender_started;
+    CHECK(content::ExecuteScriptAndExtractInt(
+        GetActiveWebContents(), expression, &event_count));
+    return event_count;
   }
 
-  int GetPrerenderLoadEventCountForLinkNumber(int index) const {
-    int load_event_count;
-    std::string expression = base::StringPrintf(
-        "window.domAutomationController.send("
-            "receivedPrerenderLoadEvents[%d] || 0)", index);
+  bool DidReceivePrerenderStartEventForLinkNumber(int index) const {
+    return GetPrerenderEventCount(index, "webkitprerenderstart") > 0;
+  }
 
-    CHECK(content::ExecuteScriptAndExtractInt(
-        GetActiveWebContents(),
-        expression,
-        &load_event_count));
-    return load_event_count;
+  int GetPrerenderLoadEventCountForLinkNumber(int index) const {
+    return GetPrerenderEventCount(index, "webkitprerenderload");
   }
 
   int GetPrerenderDomContentLoadedEventCountForLinkNumber(int index) const {
-    int dom_content_loaded_event_count;
-    std::string expression = base::StringPrintf(
-        "window.domAutomationController.send("
-            "receivedPrerenderDomContentLoadedEvents[%d] || 0)", index);
-
-    CHECK(content::ExecuteScriptAndExtractInt(
-        GetActiveWebContents(),
-        expression,
-        &dom_content_loaded_event_count));
-    return dom_content_loaded_event_count;
+    return GetPrerenderEventCount(index, "webkitprerenderdomcontentloaded");
   }
 
   bool DidReceivePrerenderStopEventForLinkNumber(int index) const {
-    bool received_prerender_stopped;
+    return GetPrerenderEventCount(index, "webkitprerenderstop") > 0;
+  }
+
+  void WaitForPrerenderEventCount(int index,
+                                  const std::string& type,
+                                  int count) const {
+    int dummy;
     std::string expression = base::StringPrintf(
-        "window.domAutomationController.send(Boolean("
-            "receivedPrerenderStopEvents[%d]))", index);
+        "WaitForPrerenderEventCount(%d, '%s', %d,"
+        "    window.domAutomationController.send.bind("
+        "        window.domAutomationController, 0))",
+        index, type.c_str(), count);
 
-    CHECK(content::ExecuteScriptAndExtractBool(
-        GetActiveWebContents(),
-        expression,
-        &received_prerender_stopped));
-    return received_prerender_stopped;
+    CHECK(content::ExecuteScriptAndExtractInt(
+        GetActiveWebContents(), expression, &dummy));
+    CHECK_EQ(0, dummy);
   }
 
   bool HadPrerenderEventErrors() const {
@@ -1397,8 +1387,8 @@ class PrerenderBrowserTest : virtual public InProcessBrowserTest {
     loader_path_ = path;
   }
 
-  void set_loader_query_and_fragment(const std::string& query_and_fragment) {
-    loader_query_and_fragment_ = query_and_fragment;
+  void set_loader_query(const std::string& query) {
+    loader_query_ = query;
   }
 
   GURL GetCrossDomainTestUrl(const std::string& path) {
@@ -1494,8 +1484,8 @@ class PrerenderBrowserTest : virtual public InProcessBrowserTest {
     const net::SpawnedTestServer* src_server = test_server();
     if (https_src_server_)
       src_server = https_src_server_.get();
-    GURL loader_url = src_server->GetURL(replacement_path +
-                                         loader_query_and_fragment_);
+    GURL loader_url = src_server->GetURL(
+        replacement_path + "&" + loader_query_);
 
     GURL::Replacements loader_replacements;
     if (!loader_host_override_.empty())
@@ -1617,7 +1607,7 @@ class PrerenderBrowserTest : virtual public InProcessBrowserTest {
   bool check_load_events_;
   std::string loader_host_override_;
   std::string loader_path_;
-  std::string loader_query_and_fragment_;
+  std::string loader_query_;
   Browser* explicitly_set_browser_;
 };
 
@@ -1724,18 +1714,17 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageRemovesPending) {
   ASSERT_TRUE(IsEmptyPrerenderLinkManager());
 }
 
-// Flaky, http://crbug.com/167340.
-IN_PROC_BROWSER_TEST_F(
-    PrerenderBrowserTest, DISABLED_PrerenderPageRemovingLink) {
-  set_loader_path("files/prerender/prerender_loader_removing_links.html");
-  set_loader_query_and_fragment("?links_to_insert=1");
-  PrerenderTestURL("files/prerender/prerender_page.html",
-                   FINAL_STATUS_CANCELLED, 1);
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageRemovingLink) {
+  scoped_ptr<TestPrerender> prerender =
+      PrerenderTestURL("files/prerender/prerender_page.html",
+                       FINAL_STATUS_CANCELLED, 1);
 
   // No ChannelDestructionWatcher is needed here, since prerenders in the
   // PrerenderLinkManager should be deleted by removing the links, rather than
   // shutting down the renderer process.
   RemoveLinkElement(0);
+  prerender->WaitForStop();
+
   EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(0));
   EXPECT_FALSE(DidReceivePrerenderStopEventForLinkNumber(0));
   EXPECT_FALSE(HadPrerenderEventErrors());
@@ -1745,16 +1734,15 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_TRUE(IsEmptyPrerenderLinkManager());
 }
 
-// Flaky, http://crbug.com/167340.
 IN_PROC_BROWSER_TEST_F(
-    PrerenderBrowserTest, DISABLED_PrerenderPageRemovingLinkWithTwoLinks) {
+    PrerenderBrowserTest, PrerenderPageRemovingLinkWithTwoLinks) {
   GetPrerenderManager()->mutable_config().max_link_concurrency = 2;
   GetPrerenderManager()->mutable_config().max_link_concurrency_per_launcher = 2;
 
-  set_loader_path("files/prerender/prerender_loader_removing_links.html");
-  set_loader_query_and_fragment("?links_to_insert=2");
-  PrerenderTestURL("files/prerender/prerender_page.html",
-                   FINAL_STATUS_CANCELLED, 1);
+  set_loader_query("links_to_insert=2");
+  scoped_ptr<TestPrerender> prerender =
+      PrerenderTestURL("files/prerender/prerender_page.html",
+                       FINAL_STATUS_CANCELLED, 1);
   EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(0));
   EXPECT_FALSE(DidReceivePrerenderStopEventForLinkNumber(0));
   EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(1));
@@ -1762,6 +1750,40 @@ IN_PROC_BROWSER_TEST_F(
 
   RemoveLinkElement(0);
   RemoveLinkElement(1);
+  prerender->WaitForStop();
+
+  EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(0));
+  EXPECT_FALSE(DidReceivePrerenderStopEventForLinkNumber(0));
+  EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(1));
+  EXPECT_FALSE(DidReceivePrerenderStopEventForLinkNumber(1));
+  EXPECT_FALSE(HadPrerenderEventErrors());
+  // IsEmptyPrerenderLinkManager() is not racy because the earlier DidReceive*
+  // calls did a thread/process hop to the renderer which insured pending
+  // renderer events have arrived.
+  EXPECT_TRUE(IsEmptyPrerenderLinkManager());
+}
+
+IN_PROC_BROWSER_TEST_F(
+    PrerenderBrowserTest, PrerenderPageRemovingLinkWithTwoLinksOneLate) {
+  GetPrerenderManager()->mutable_config().max_link_concurrency = 2;
+  GetPrerenderManager()->mutable_config().max_link_concurrency_per_launcher = 2;
+
+  GURL url = test_server()->GetURL("files/prerender/prerender_page.html");
+  scoped_ptr<TestPrerender> prerender =
+      PrerenderTestURL(url, FINAL_STATUS_CANCELLED, 1);
+
+  // Add a second prerender for the same link. It reuses the prerender, so only
+  // the start event fires here.
+  AddPrerender(url, 1);
+  WaitForPrerenderEventCount(1, "webkitprerenderstart", 1);
+  EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(1));
+  EXPECT_EQ(0, GetPrerenderLoadEventCountForLinkNumber(1));
+  EXPECT_FALSE(DidReceivePrerenderStopEventForLinkNumber(1));
+
+  RemoveLinkElement(0);
+  RemoveLinkElement(1);
+  prerender->WaitForStop();
+
   EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(0));
   EXPECT_FALSE(DidReceivePrerenderStopEventForLinkNumber(0));
   EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(1));
@@ -1773,21 +1795,12 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_TRUE(IsEmptyPrerenderLinkManager());
 }
 
-#if defined(OS_WIN)
-// TODO(gavinp): Fails on XP Rel - http://crbug.com/128841
-#define MAYBE_PrerenderPageRemovingLinkWithTwoLinksRemovingOne \
-    DISABLED_PrerenderPageRemovingLinkWithTwoLinksRemovingOne
-#else
-#define MAYBE_PrerenderPageRemovingLinkWithTwoLinksRemovingOne \
-    PrerenderPageRemovingLinkWithTwoLinksRemovingOne
-#endif  // defined(OS_WIN)
 IN_PROC_BROWSER_TEST_F(
     PrerenderBrowserTest,
-    MAYBE_PrerenderPageRemovingLinkWithTwoLinksRemovingOne) {
+    PrerenderPageRemovingLinkWithTwoLinksRemovingOne) {
   GetPrerenderManager()->mutable_config().max_link_concurrency = 2;
   GetPrerenderManager()->mutable_config().max_link_concurrency_per_launcher = 2;
-  set_loader_path("files/prerender/prerender_loader_removing_links.html");
-  set_loader_query_and_fragment("?links_to_insert=2");
+  set_loader_query("links_to_insert=2");
   PrerenderTestURL("files/prerender/prerender_page.html",
                    FINAL_STATUS_USED, 1);
   EXPECT_TRUE(DidReceivePrerenderStartEventForLinkNumber(0));

chrome/test/data/prerender/prerender_events_common.js

@@ -10,66 +10,80 @@
 // Currently only errors with the ordering of Prerender events are caught.
 var hadPrerenderEventErrors = false;
 
-var receivedPrerenderStartEvents = [];
-var receivedPrerenderLoadEvents = [];
-var receivedPrerenderDomContentLoadedEvents = [];
-var receivedPrerenderStopEvents = [];
-
-function PrerenderStartHandler(index) {
-  if (receivedPrerenderStartEvents[index] ||
-      receivedPrerenderLoadEvents[index] ||
-      receivedPrerenderStopEvents[index]) {
-    hadPrerenderEventErrors = true;
-    return;
+var receivedPrerenderEvents = {
+  'webkitprerenderstart': [],
+  'webkitprerenderdomcontentloaded': [],
+  'webkitprerenderload': [],
+  'webkitprerenderstop': [],
+}
+// A list of callbacks to be called on every prerender event. Each callback
+// returns true if it should never be called again, or false to remain in the
+// list and be called on future events. These are used to implement
+// WaitForPrerenderEventCount.
+var prerenderEventCallbacks = [];
+
+function GetPrerenderEventCount(index, type) {
+  return receivedPrerenderEvents[type][index] || 0;
+}
+
+function PrerenderEventHandler(index, ev) {
+  // Check for errors.
+  if (ev.type == 'webkitprerenderstart') {
+    // No event may preceed start.
+    if (GetPrerenderEventCount(index, 'webkitprerenderstart') ||
+        GetPrerenderEventCount(index, 'webkitprerenderdomcontentloaded') ||
+        GetPrerenderEventCount(index, 'webkitprerenderload') ||
+        GetPrerenderEventCount(index, 'webkitprerenderstop')) {
+      hadPrerenderEventErrors = true;
+    }
+  } else {
+    // There may be multiple load or domcontentloaded events, but they must not
+    // come after start and must come before stop. And there may be at most one
+    // start. Note that stop may be delivered without any load events.
+    if (!GetPrerenderEventCount(index, 'webkitprerenderstart') ||
+        GetPrerenderEventCount(index, 'webkitprerenderstop')) {
+      hadPrerenderEventErrors = true;
+    }
   }
-  receivedPrerenderStartEvents[index] = true;
-}
 
-function PrerenderLoadHandler(index) {
-  if (!receivedPrerenderStartEvents[index] ||
-      receivedPrerenderStopEvents[index]) {
-    hadPrerenderEventErrors = true;
-    return;
-  }
-  if (!receivedPrerenderLoadEvents[index])
-    receivedPrerenderLoadEvents[index] = 0;
-  receivedPrerenderLoadEvents[index]++;
-}
+  // Update count.
+  receivedPrerenderEvents[ev.type][index] =
+      (receivedPrerenderEvents[ev.type][index] || 0) + 1;
 
-function PrerenderDomContentLoadedHandler(index) {
-  if (!receivedPrerenderStartEvents[index] ||
-      receivedPrerenderStopEvents[index]) {
-    hadPrerenderEventErrors = true;
-    return;
-  }
-  if (!receivedPrerenderDomContentLoadedEvents[index])
-    receivedPrerenderDomContentLoadedEvents[index] = 0;
-  receivedPrerenderDomContentLoadedEvents[index]++;
+  // Run all callbacks. Remove the ones that are done.
+  prerenderEventCallbacks = prerenderEventCallbacks.filter(function(callback) {
+    return !callback();
+  });
 }
 
-function PrerenderStopHandler(index) {
-  if (!receivedPrerenderStartEvents[index] ||
-      receivedPrerenderStopEvents[index]) {
-    hadPrerenderEventErrors = true;
-    return;
-  }
-  receivedPrerenderStopEvents[index] = true;
+// Calls |callback| when at least |count| instances of event |type| have been
+// observed for prerender |index|.
+function WaitForPrerenderEventCount(index, type, count, callback) {
+  var checkCount = function() {
+    if (GetPrerenderEventCount(index, type) >= count) {
+      callback();
+      return true;
+    }
+    return false;
+  };
+  if (!checkCount())
+    prerenderEventCallbacks.push(checkCount);
 }
 
 function AddEventHandlersToLinkElement(link, index) {
   link.addEventListener('webkitprerenderstart',
-                        PrerenderStartHandler.bind(null, index), false);
-  link.addEventListener('webkitprerenderload',
-                        PrerenderLoadHandler.bind(null, index), false);
+                        PrerenderEventHandler.bind(null, index), false);
   link.addEventListener('webkitprerenderdomcontentloaded',
-                        PrerenderDomContentLoadedHandler.bind(null, index),
-                        false);
+                        PrerenderEventHandler.bind(null, index), false);
+  link.addEventListener('webkitprerenderload',
+                        PrerenderEventHandler.bind(null, index), false);
   link.addEventListener('webkitprerenderstop',
-                        PrerenderStopHandler.bind(null, index), false);
+                        PrerenderEventHandler.bind(null, index), false);
 }
 
 function AddPrerender(url, index) {
   var link = document.createElement('link');
+  link.id = 'prerenderElement' + index;
   link.rel = 'prerender';
   link.href = url;
   AddEventHandlersToLinkElement(link, index);
@@ -77,6 +91,19 @@ function AddPrerender(url, index) {
   return link;
 }
 
+function RemoveLinkElement(index) {
+  var link = document.getElementById('prerenderElement' + index);
+  link.parentElement.removeChild(link);
+}
+
+function ExtractGetParameterBadlyAndInsecurely(param, defaultValue) {
+  var re = RegExp('[&?]' + param + '=([^&?#]*)');
+  var result = re.exec(document.location);
+  if (result)
+    return result[1];
+  return defaultValue;
+}
+
 function AddAnchor(href, target) {
   var a = document.createElement('a');
   a.href = href;

chrome/test/data/prerender/prerender_loader.html

@@ -5,7 +5,11 @@
 </head>
 <body>
 <script>
-  AddPrerender('REPLACE_WITH_PRERENDER_URL', 0);
+var numLinksToInsert =
+    ExtractGetParameterBadlyAndInsecurely('links_to_insert', 1);
+for (var i = 0; i < numLinksToInsert; ++i) {
+  AddPrerender('REPLACE_WITH_PRERENDER_URL', i);
+}
 </script>
 </body>
 </html>

chrome/test/data/prerender/prerender_loader_removing_links.html

-<html>
-<head>
-<title>Preloader</title>
-
-<script src="prerender_events_common.js"></script>
-
-<script>
-function ExtractGetParameterBadlyAndInsecurely(param, defaultValue) {
-  var re = RegExp('[&?]' + param + '=([^&?#]*)');
-  var result = re.exec(document.location);
-  if (result)
-    return result[1];
-  return defaultValue;
-}
-
-var numLinksToInsert =
-    ExtractGetParameterBadlyAndInsecurely('links_to_insert', 1);
-
-function RemoveLinkElement(index) {
-  var link = document.getElementById('prerenderElement' + index);
-  link.parentElement.removeChild(link);
-}
-</script>
-</head>
-<body>
-<script>
-for (var i = 0; i < numLinksToInsert; ++i) {
-  var link = AddPrerender('REPLACE_WITH_PRERENDER_URL', i);
-  link.id = 'prerenderElement' + i;
-}
-</script>
-</body>
-</html>

chrome/test/data/prerender/prerender_page_pending.html

@@ -14,7 +14,7 @@ var pageWasPrerendered = false;
 
 function DidPrerenderPass() {
   pageWasPrerendered = true;
-  return !receivedPrerenderStartEvents[0] && !receivedPrerenderStopEvents[0];
+  return !GetPrerenderEventCount(0, 'webkitprerenderstart');
 }
 
 // Make sure DidPrerenderPass() was called first.  Otherwise, the page was

chrome/test/data/prerender/prerender_page_removes_pending.html

@@ -17,14 +17,14 @@ function DidPrerenderPass() {
   var link = document.getElementById('thePrerender');
   if (link)
     link.parentElement.removeChild(link)
-  return !receivedPrerenderStartEvents[0] && !receivedPrerenderStopEvents[0];
+  return !GetPrerenderEventCount(0, 'webkitprerenderstart');
 }
 
 // Make sure DidPrerenderPass() was called first.  Otherwise, the page was
 // most likely reloaded instead of using the prerendered page.
 function DidDisplayPass() {
-  return pageWasPrerendered && !receivedPrerenderStartEvents[0] &&
-         !receivedPrerenderStopEvents[0];
+  return pageWasPrerendered &&
+      !GetPrerenderEventCount(0, 'webkitprerenderstart');
 }
 
 var link = document.getElementById('thePrerender');