chrome/test/data/prerender/prerender_loader_removing_links.html - Issue 11459003: Add Start/Stop event signalling on Prerenders.

Side by Side Diff: chrome/test/data/prerender/prerender_loader_removing_links.html

Issue 11459003: Add Start/Stop event signalling on Prerenders. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: rebase Created 8 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

« chrome/test/data/prerender/prerender_loader.html ('K') | « chrome/test/data/prerender/prerender_loader.html ('k') | chrome/test/data/prerender/prerender_page.html » ('j') | chrome/test/data/prerender/prerender_page_pending.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <html>	1 <html>

2 <head>	2 <head>

3 <title>Preloader</title>	3 <title>Preloader</title>

4	4

5 <script>	5 <script>

6 function ExtractGetParameterBadlyAndInsecurely(param, defaultValue) {	6 function ExtractGetParameterBadlyAndInsecurely(param, defaultValue) {

7 var re = RegExp('[&?]' + param + '=([^&?#]*)');	7 var re = RegExp('[&?]' + param + '=([^&?#]*)');

8 var result = re.exec(document.location);	8 var result = re.exec(document.location);

9 if (result)	9 if (result)

10 return result[1];	10 return result[1];

11 return defaultValue;	11 return defaultValue;

12 }	12 }

13	13

14 var linksToInsert = ExtractGetParameterBadlyAndInsecurely('links_to_insert', 1);	14 var linksToInsert = ExtractGetParameterBadlyAndInsecurely('links_to_insert', 1);

15 var linksToRemove = ExtractGetParameterBadlyAndInsecurely('links_to_remove', 1);	15 var linksToRemove = ExtractGetParameterBadlyAndInsecurely('links_to_remove', 1);
	mmenke 2012/12/17 17:08:18 nit: While you're here, could you rename these "n 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. Show quoted text On 2012/12/17 17:08:18, Matt Menke wrote: > 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. Done.
16	16

17 function RemoveLinkElementsAndNavigate() {	17 var receivedPrerenderStartEvents = Array();

	18 var receivedPrerenderStopEvents = Array();

	19

	20 function MakePrerenderStartHandlerForLink(index) {

	21 return function() { receivedPrerenderStartEvents[index] = true; };

	22 }

	23

	24 function MakePrerenderStopHandlerForLink(index) {

	25 return function() { receivedPrerenderStopEvents[index] = true; };

	26 }

	27

	28 function RemoveLinkElements() {
	mmenke 2012/12/17 17:08:18 optional: I think the C++ part of your unit tests 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. Show quoted text On 2012/12/17 17:08:18, Matt Menke wrote: > 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. Done.
18 for (var i = 1; i <= linksToRemove; ++i) {	29 for (var i = 1; i <= linksToRemove; ++i) {

19 var link = document.getElementById('prerenderElement' + i);	30 var link = document.getElementById('prerenderElement' + i);

20 link.parentElement.removeChild(link);	31 link.parentElement.removeChild(link);

21 }	32 }

22 document.location = 'REPLACE_WITH_DESTINATION_URL';

23 }	33 }

	34
	mmenke 2012/12/17 17:08:18 nit: While I don't really have any preference abo 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. Show quoted text On 2012/12/17 17:08:18, Matt Menke wrote: > 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. Done.
24 </script>	35 </script>

25	36

26 </head>	37 </head>

27 <body>	38 <body>

28 <script>	39 <script>

29 for (var i = 1; i <= linksToInsert; ++i) {	40 for (var i = 1; i <= linksToInsert; ++i) {
	mmenke 2012/12/17 17:08:18 Is there a reason you go from 1 to linksToInsert, Is there a reason you go from 1 to linksToInsert, rather than 0 to linksToInsert - 1? gavinp 2012/12/17 20:34:02 Likely I was channeling PASCAL. Fixed. Show quoted text On 2012/12/17 17:08:18, Matt Menke wrote: > Is there a reason you go from 1 to linksToInsert, rather than 0 to linksToInsert > - 1? Likely I was channeling PASCAL. Fixed.
30 document.write(	41 document.write(

31 '<link id="prerenderElement' + i +	42 '<link id="prerenderElement' + i +

32 '" rel="prerender" href="REPLACE_WITH_PRERENDER_URL"/>');	43 '" rel="prerender" href="REPLACE_WITH_PRERENDER_URL"/>');

	44 var link = document.getElementById('prerenderElement' + i);

	45 link.addEventListener('webkitprerenderstart',

	46 MakePrerenderStartHandlerForLink(i), false);

	47 link.addEventListener('webkitprerenderstop',

	48 MakePrerenderStopHandlerForLink(i), false);
	mmenke 2012/12/17 17:08:18 optional: Maybe I just have a bias against funtio 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 Show quoted text On 2012/12/17 17:08:18, Matt Menke wrote: > 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. You may have such a bias; it's also plausible that I am unduly comfortable with lexical scope and closures. Done.
33 }	49 }

34 </script>	50 </script>

35 <a href="REPLACE_WITH_DESTINATION_URL">Link To Click</a>	51 <a id="toClick" href="REPLACE_WITH_DESTINATION_URL">Link To Click For First Prer ender</a>

36 </body>	52 </body>

37 </html>	53 </html>

OLD	NEW