LGTM, with suggestions.
http://codereview.chromium.org/155496/diff/1/2 File benchmarks/run.html (right): http://codereview.chromium.org/155496/diff/1/2#newcode58 Line 58: function ShowWarningIfObsolete() { It might not be worth it to do this check if the banchmark is not received over http (i.e., check location.protocol is "http:" or "https:"). Or even if it is read from a location that isn't standard. I.e., something like: if (!/^https?:.*\/v\d+\/run.html/i.test(location.href)) { return; } You can even check that the digits in the href matches BenchmarkSuite.version, or that it comes from the original v8.googlecode.com location. http://codereview.chromium.org/155496/diff/1/2#newcode65 Line 65: if (window.XMLHttpRequest) { I recommend using if (typeof XMLHttpRequest != "undefined") { instead. It's dangerous to rely on boolean-conversion of host objects. It's probably not important though (it reputedly works in IE, and that's the most common cause of this kind of problems). http://codereview.chromium.org/155496/diff/1/2#newcode66 Line 66: xmlhttp = new window.XMLHttpRequest(); You can drop the "window." in front. http://codereview.chromium.org/155496/diff/1/2#newcode67 Line 67: } else if (window.ActiveXObject) { Drop this test completely. If it fails, there is no case to fall through to, so you might as well just try to create the object and fail if you can't. http://codereview.chromium.org/155496/diff/1/2#newcode70 Line 70: xmlhttp.open('GET', next_version_url, true); The third parameter makes the request asynchroneous. That means that the file might be received while the benchmarks are being run, and might affect the results. It would be better to make it synchroneous, so it is done before the benchmarks starts running. http://codereview.chromium.org/155496 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
