Added a warning shown if the viewed version is not the current version of...

Added a warning shown if the viewed version is not the current version of
V8 benchmark suite. This change list is only for v1/run.html. I will add
changes for v2, v3 and v4 plus the current one in trunk once this CL
has been accepted. 

Committed: http://code.google.com/p/v8/source/detail?r=2457

[Mads Ager (chromium), 2009-07-13 17:05:43]

LGTM

[Kasper Lund, 2009-07-14 08:01:42]

LGTM.

http://codereview.chromium.org/149530/diff/1/2
File benchmarks/v1/run.html (right):

http://codereview.chromium.org/149530/diff/1/2#newcode67
Line 67: background:#ffffd9;
Space after :.

http://codereview.chromium.org/149530/diff/1/2#newcode106
Line 106: function showWarning() {
showWarning -> ShowWarning. It's still a weird name, because it only shows the
warning if it's obsolete. Maybe change the name to reflect this?
ShowWarningIfObsolete?

http://codereview.chromium.org/149530/diff/1/2#newcode108
Line 108: var latesturl = "../v" + (parseInt(BenchmarkSuite.version)+1) +
"/run.html";
Spaces around +.

http://codereview.chromium.org/149530/diff/1/2#newcode108
Line 108: var latesturl = "../v" + (parseInt(BenchmarkSuite.version)+1) +
"/run.html";
Weird name. Why not latest_url?

http://codereview.chromium.org/149530/diff/1/2#newcode111
Line 111: } else {
else if (...) instead of else { if (...)

http://codereview.chromium.org/149530/diff/1/2#newcode117
Line 117: xmlhttp.open('GET', latesturl, true);
It's a bit weird that you even try this if xmlhttp is null.

http://codereview.chromium.org/149530/diff/1/2#newcode119
Line 119: if (xmlhttp.readyState==4 && xmlhttp.status==200) {
Spaces around ==.

http://codereview.chromium.org/149530/diff/1/2#newcode125
Line 125: // Ignore exception if check for newer version fails
Terminate comment with .

http://codereview.chromium.org/149530/diff/1/2#newcode141
Line 141: Warning! This is not the latest version of V8 Benchmark Suite. For the
latest version please visit <a
href="http://v8.googlecode.com/svn/data/benchmarks/current/run.html">this
page</a>.
Maybe wrap the line a bit differently? 80 characters per line. How about
changing the wording to something like: Warning! This is not the latest version
of the V8 benchmark suite. Consider running the <a>latest version</a>.

[Kasper Lund, 2009-07-14 12:05:16]

LGTM.

http://codereview.chromium.org/149530/diff/6/1003
File benchmarks/v1/run.html (right):

http://codereview.chromium.org/149530/diff/6/1003#newcode107
Line 107: // if anything goes wrong we will just catch the exception and no
if -> If

[sandholm, 2009-07-14 13:22:36]

http://codereview.chromium.org/149530/diff/1/2
File benchmarks/v1/run.html (right):

http://codereview.chromium.org/149530/diff/1/2#newcode67
Line 67: background:#ffffd9;
On 2009/07/14 08:01:42, Kasper Lund wrote:
> Space after :.

Done.

http://codereview.chromium.org/149530/diff/1/2#newcode106
Line 106: function showWarning() {
On 2009/07/14 08:01:42, Kasper Lund wrote:
> showWarning -> ShowWarning. It's still a weird name, because it only shows the
> warning if it's obsolete. Maybe change the name to reflect this?
> ShowWarningIfObsolete?

Done.

http://codereview.chromium.org/149530/diff/1/2#newcode108
Line 108: var latesturl = "../v" + (parseInt(BenchmarkSuite.version)+1) +
"/run.html";
On 2009/07/14 08:01:42, Kasper Lund wrote:
> Spaces around +.

Done.

http://codereview.chromium.org/149530/diff/1/2#newcode108
Line 108: var latesturl = "../v" + (parseInt(BenchmarkSuite.version)+1) +
"/run.html";
On 2009/07/14 08:01:42, Kasper Lund wrote:
> Weird name. Why not latest_url?

Done.

http://codereview.chromium.org/149530/diff/1/2#newcode111
Line 111: } else {
On 2009/07/14 08:01:42, Kasper Lund wrote:
> else if (...) instead of else { if (...)

Done.

http://codereview.chromium.org/149530/diff/1/2#newcode117
Line 117: xmlhttp.open('GET', latesturl, true);
On 2009/07/14 08:01:42, Kasper Lund wrote:
> It's a bit weird that you even try this if xmlhttp is null.

Well, I'll catch the null exception anyhow. But I can add an explicit null check
for robustness against future changes.

http://codereview.chromium.org/149530/diff/1/2#newcode119
Line 119: if (xmlhttp.readyState==4 && xmlhttp.status==200) {
On 2009/07/14 08:01:42, Kasper Lund wrote:
> Spaces around ==.

Done.

http://codereview.chromium.org/149530/diff/1/2#newcode141
Line 141: Warning! This is not the latest version of V8 Benchmark Suite. For the
latest version please visit <a
href="http://v8.googlecode.com/svn/data/benchmarks/current/run.html">this
page</a>.
On 2009/07/14 08:01:42, Kasper Lund wrote:
> Maybe wrap the line a bit differently? 80 characters per line. How about
> changing the wording to something like: Warning! This is not the latest
version
> of the V8 benchmark suite. Consider running the <a>latest version</a>.

Done.

[sandholm, 2009-07-14 13:24:22]

http://codereview.chromium.org/149530/diff/6/1003
File benchmarks/v1/run.html (right):

http://codereview.chromium.org/149530/diff/6/1003#newcode107
Line 107: // if anything goes wrong we will just catch the exception and no
On 2009/07/14 12:05:16, Kasper Lund wrote:
> if -> If

Done.