Add Start/Stop event signalling on Prerenders.

chrome/test/data/prerender/prerender_loader_removing_links.html

Index: chrome/test/data/prerender/prerender_loader_removing_links.html
diff --git a/chrome/test/data/prerender/prerender_loader_removing_links.html b/chrome/test/data/prerender/prerender_loader_removing_links.html
index 08d4037f228088f3be6686ffe8823dfc42892605..fdc62ab09f7c71a1fe4b64c36e80db9e23fa90f1 100644
--- a/chrome/test/data/prerender/prerender_loader_removing_links.html
+++ b/chrome/test/data/prerender/prerender_loader_removing_links.html
@@ -14,13 +14,24 @@ function ExtractGetParameterBadlyAndInsecurely(param, defaultValue) {
 var linksToInsert = ExtractGetParameterBadlyAndInsecurely('links_to_insert', 1);
 var linksToRemove = ExtractGetParameterBadlyAndInsecurely('links_to_remove', 1);

[mmenke, 2012/12/17 17:08:18]

nit:  While you're here, could you rename these "numLinksToRemove" /
"numLinkToInsert"?  AS it is, have to read the code to figure out they're
counts, and not URLs.

[gavinp, 2012/12/17 20:34:02]

Done.

 
-function RemoveLinkElementsAndNavigate() {
+var receivedPrerenderStartEvents = Array();
+var receivedPrerenderStopEvents = Array();
+
+function MakePrerenderStartHandlerForLink(index) {
+  return function() { receivedPrerenderStartEvents[index] = true; };
+}
+
+function MakePrerenderStopHandlerForLink(index) {
+  return function() { receivedPrerenderStopEvents[index] = true; };
+}
+
+function RemoveLinkElements() {

[mmenke, 2012/12/17 17:08:18]

optional:  I think the C++ part of your unit tests would be more legible if you
renamed this to "RemoveLinkElement(var index)", and remove linksToRemove.

[gavinp, 2012/12/17 20:34:02]

Done.

   for (var i = 1; i <= linksToRemove; ++i) {
     var link = document.getElementById('prerenderElement' + i);
     link.parentElement.removeChild(link);
   }
-  document.location = 'REPLACE_WITH_DESTINATION_URL';
 }
+

[mmenke, 2012/12/17 17:08:18]

nit:  While I don't really have any preference about whether we have spaces
before / after script tags or not, it looks like this is the only case we have
one, just before the clsoe tag at least in this CL.

[gavinp, 2012/12/17 20:34:02]

Done.

 </script>
 
 </head>
@@ -30,8 +41,13 @@ for (var i = 1; i <= linksToInsert; ++i) {
   document.write(
       '<link id="prerenderElement' + i +
       '" rel="prerender" href="REPLACE_WITH_PRERENDER_URL"/>');
+  var link = document.getElementById('prerenderElement' + i);
+  link.addEventListener('webkitprerenderstart',
+		        MakePrerenderStartHandlerForLink(i), false);
+  link.addEventListener('webkitprerenderstop',
+		        MakePrerenderStopHandlerForLink(i), false);

[mmenke, 2012/12/17 17:08:18]

optional:  Maybe I just have a bias against funtions that return functions, and
how Javascript handles scoping, but I think this would be a little cleaner as:

PrerenderStopHandleForLink.bind(null, i);

With PrerenderStopHandleForLink being the inner function in
MakePrerenderStopHandleForLink.

[gavinp, 2012/12/17 20:34:02]

You may have such a bias; it's also plausible that I am unduly comfortable with
lexical scope and closures.

Done.

 }
 </script>
-<a href="REPLACE_WITH_DESTINATION_URL">Link To Click</a>
+<a id="toClick" href="REPLACE_WITH_DESTINATION_URL">Link To Click For First Prerender</a>
 </body>
 </html>