Support Chrome Endure graphs in perf dashboard.

dashboard/ui/endure_js/common.js

+/*
+  Copyright (c) 2012 The Chromium Authors. All rights reserved.
+  Use of this source code is governed by a BSD-style license that can be
+  found in the LICENSE file.
+*/
+
+/**
+ * @fileoverview Common methods for performance-plotting Javascript.

[alias of yukishiino, 2013/01/31 10:57:11]

I'd recommend a different file name for this file.
'common' doesn't make much sense, and the content code is not that common (URL
and fetch specific).

I'd recommend url_util(s).js or anything better than 'common'.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Because it's a fork of dashboard/ui/js/common.js, I think keeping it is better.

+ */
+
+/**
+ * Fetches a URL asynchronously and invokes a callback when complete.
+ *
+ * @param {string} url URL to fetch.
+ * @param {Function(string, string)} callback The function to invoke when the

[alias of yukishiino, 2013/01/31 10:57:11]

{Function(string, ?string)}
since |error| can be null.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Done.

+ *     results of the URL fetch are complete.  The function should accept two
+ *     strings representing the URL data, and any errors, respectively.
+ */
+function Fetch(url, callback) {
+  var r = new XMLHttpRequest();
+  r.open('GET', url, true);
+  r.setRequestHeader('pragma', 'no-cache');
+  r.setRequestHeader('cache-control', 'no-cache');
+
+  r.onreadystatechange = function() {
+    if (r.readyState == 4) {
+      var text = r.responseText;
+      var error;

[alias of yukishiino, 2013/01/31 10:57:11]

Better to initialize |error| with null, otherwise error becomes undefined, which
is slightly different from null.
null == undefined, but null !== undefined.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Done.

+      if (r.status != 200)
+        error = url + ': ' + r.status + ': ' + r.statusText;
+      else if (!text)
+        error = url + ': null response';
+      callback(text, error);
+    }
+  }
+
+  r.send(null);
+}
+
+/**
+ * Parses the parameters of the current page's URL.
+ *
+ * @return {Object} An object with properties given by the parameters specified

[alias of yukishiino, 2013/01/31 10:57:11]

{Object.<string, (string|number)>}

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Done.

+ *     in the URL's query string.
+ */
+function ParseParams() {
+  var result = new Object();

[alias of yukishiino, 2013/01/31 10:57:11]

var result = {};
is the same in effect, and more often used especially for a dict.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Done.

+
+  var query = window.location.search.substring(1)

[alias of yukishiino, 2013/01/31 10:57:11]

missing semi-colon.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Done.

+  if (query.charAt(query.length - 1) == '/')
+    query = query.substring(0, query.length - 1)  // Strip trailing slash.

[alias of yukishiino, 2013/01/31 10:57:11]

if (query.slice(-1) == '/') {
  query = query.slice(0, -1);
}

1. string.slice() supports a negative index.
2. Always use braces {} in JavaScript.
3. Put a semi colon at the end of statement.

By the way, why do you need to strip the last slash?
In general, it's possible that a CGI parameter has a value ending with a slash. 
query is not URL path.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Done.
I'm not sure.  What do you think, Dennis?

+  var s = query.split('&');
+
+  for (i = 0; i < s.length; ++i) {
+    var v = s[i].split('=');

[alias of yukishiino, 2013/01/31 10:57:11]

This code does not handle '=' in the value correctly.
'a=b=c'.split('=') produces ['a', 'b', 'c'].

In general, values may include '=' in itself.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Hmm, I thought to fix it, but stopped it not to have more difference from the
original dashboard/ui/js/common.js.

+    var key = v[0];
+    var value = unescape(v[1]);

[alias of yukishiino, 2013/01/31 10:57:11]

escape/unescape are obsolete.
Use encodeURIComponent/decodeURIComponent instead.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Stopped to fix it in the same reason above.

+    result[key] = value;
+  }
+
+  if ('history' in result) {
+    result['history'] = parseInt(result['history']);
+    result['history'] = Math.max(result['history'], 2);
+  }
+  if ('rev' in result) {
+    result['rev'] = parseInt(result['rev']);
+    result['rev'] = Math.max(result['rev'], -1);

[alias of yukishiino, 2013/01/31 10:57:11]

Why -1?  Can rev be negative?

By the way, if 'rev=abc', then parseInt('abc') => NaN, and Math.max(NaN, -1) =>
NaN.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

I'm not sure.  What do you think, Dennis?

[dennis_jeffrey, 2013/02/05 02:48:28]

I don't think "rev" should ever be negative from the perspective of the
endurance tests, so I'd say that we can go ahead and remove this statement. 
Also, "rev" should always be something that can parse into an int.  If it isn't,
Yuki indicates that trying to parse it into an int will be NaN, and I think
that's fine here.

[Dai Mikurube (NOT FULLTIME), 2013/02/05 06:31:49]

Removed the statement.

+  }
+
+  return result;
+}
+
+/**
+ * Creates the URL constructed from the current pathname and the given params.
+ *
+ * @param {Object} An object containing parameters for a URL query string.

[alias of yukishiino, 2013/01/31 10:57:11]

{Object.<string, (string|number)>}

[alias of yukishiino, 2013/01/31 10:57:11]

Missing parameter name.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Done.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

Done.

+ * @return {string} The URL constructed from the given params.
+ */
+function MakeURL(params) {
+  var url = window.location.pathname;
+  var sep = '?';
+  for (p in params) {
+    if (!p)

[alias of yukishiino, 2013/01/31 10:57:11]

Shouldn't reach here.
For "for (p in params)", p must not be null or undefined.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

We may want to remove it.  What do you think, Dennis?

[dennis_jeffrey, 2013/02/05 02:48:28]

Yes, if it's true that p will never be null or undefined in this kind of loop,
then it's safe to remove this check.  It does make sense that if we loop through
an object's properties, there should never be a null or undefined property
(though I guess its corresponding value may be null or undefined).

[Dai Mikurube (NOT FULLTIME), 2013/02/05 06:31:49]

Applied the change below.

+      continue;
+    url += sep + p + '=' + params[p];

[alias of yukishiino, 2013/01/31 10:57:11]

You'd better use encodeURIComponent() for values.

By the way, I usually do

  var key_values = [];
  for (key in params) {
    // better to sanitize key here.
    key_values.push(key + '=' + encodeURIComponent(params[key]));
  }
  return path + '?' + key_values.join('&');

If we had very many keys, array + join is more efficient than string
concatenation, and no need to care about '?' and '&'.

[Dai Mikurube (NOT FULLTIME), 2013/02/01 06:14:13]

What do you think, Dennis?

[dennis_jeffrey, 2013/02/05 02:48:28]

I think this sounds good, as long as we make sure to decode the values on the
other end where they're consumed.

[Dai Mikurube (NOT FULLTIME), 2013/02/05 06:31:49]

Replaced with the code by Yuki.

+    sep = '&';
+  }
+  return url;
+}