Modifications to performance monitor UI to address real webui.

chrome/browser/resources/performance_monitor/chart.js

Index: chrome/browser/resources/performance_monitor/chart.js
diff --git a/chrome/browser/resources/performance_monitor/chart.js b/chrome/browser/resources/performance_monitor/chart.js
index 1f0f4ef3804c6d9604596d59f8089d53ab747b3a..3c2f9cf73da3e81514c1c8a081579e03cf98e1c4 100644
--- a/chrome/browser/resources/performance_monitor/chart.js
+++ b/chrome/browser/resources/performance_monitor/chart.js
@@ -4,10 +4,7 @@
 
 'use strict';
 
-/* convert to cr.define('PerformanceMonitor', function() {...
- * when integrating into webui */
-
-var Installer = function() {
+cr.define('performance_monitor', function() {
   /**
    * Enum for time ranges, giving a descriptive name, time span prior to |now|,
    * data point resolution, and time-label frequency and format for each.
@@ -43,6 +40,12 @@ var Installer = function() {
         resolution: 1000 * 3600 * 3, labelEvery: 112, format: 'M/yy'},
   };
 
+  /*
+   * Offset, in mS, by which to subtract to convert GMT to local time

[Evan Stade, 2012/08/03 17:50:59]

isn't milliseconds abbreviated to ms?

[clintstaley, 2012/08/06 21:06:47]

I've seen mS, and even msec, but happy to adjust to ms.

+   * @type {!number}
+   */
+  var timezoneOffset = new Date().getTimezoneOffset() * 60000;
+
   /** @constructor */
   function PerformanceMonitor() {
     this.__proto__ = PerformanceMonitor.prototype;
@@ -51,79 +54,100 @@ var Installer = function() {
      * If a div list is not empty, the associated data will be non-null, or
      * null but about to be filled by webui response. Thus, any metric with
      * non-empty div list but null data is awaiting a data response from the
-     * webui.
+     * webui. The webui uses numbers to uniquely identify metric and event

[Dan Beam, 2012/08/03 18:40:36]

should these say "webui handler" instead of webui?

[clintstaley, 2012/08/06 21:06:47]

Done.

+     * types, so we use the same numbers (in string form) for the map key,
+     * repeating the id number in the |id| field, as a number.
      * @type {Object.<string, {

[Dan Beam, 2012/08/03 18:40:36]

I think wrapping for these long @type expressions is usually like this:

 * @type {!{blah: type, blee: type, blar: type, blag: type,
 *          bluh: type, blug: type, blue: type}}

[clintstaley, 2012/08/06 21:06:47]

I've gotten different responses on this from different reviewers.  This
indentation, for instance, is the result of prior review.  And, given the types
in question,I don't think I can get the : lineup implied in your diagram with
more than one type per line.

-     *                   divs: !Array.<HTMLDivElement>,
-     *                   yAxis: !{max: !number, color: !string},
-     *                   data: ?Array.<{time: !number, value: !number}>,
+     *                   id: !number,
      *                   description: !string,
-     *                   units: !string
+     *                   units: !string,
+     *                   yAxis: !{max: !number, color: !string},
+     *                   divs: !Array.<HTMLDivElement>,
+     *                   data: ?Array.<{time: !number, value: !number}>
      *                 }>}
      * @private
      */
-    this.metricMap_ = {
-      jankiness: {
-        divs: [],
-        yAxis: {max: 100, color: 'rgb(255, 128, 128)'},
-        data: null,
-        description: 'Jankiness',
-        units: 'milliJanks'
-      },
-      oddness: {
-        divs: [],
-        yAxis: {max: 20, color: 'rgb(0, 192, 0)'},
-        data: null,
-        description: 'Oddness',
-        units: 'kOdds'
-      }
-    };
+    this.metricMap_ = {};
 
     /*
      * Similar data for events, though no yAxis info is needed since events
      * are simply labelled markers at X locations. Rules regarding null data
      * with non-empty div list apply here as for metricMap_ above.
-     * @type {Object.<string, {
-     *   divs: !Array.<HTMLDivElement>,
+     * @type {Object.<number, {
+     *   id: !number,
      *   description: !string,
      *   color: !string,
+     *   divs: !Array.<HTMLDivElement>,
      *   data: ?Array.<{time: !number, longDescription: !string}>
      * }>}
      * @private
      */
-    this.eventMap_ = {
-      wampusAttacks: {
-        divs: [],
-        description: 'Wampus Attack',
-        color: 'rgb(0, 0, 255)',
-        data: null
-      },
-      solarEclipses: {
-        divs: [],
-        description: 'Solar Eclipse',
-        color: 'rgb(255, 0, 255)',
-        data: null
-      }
-    };
+    this.eventMap_ = {};
 
     /**
-     * Time periods in which the browser was active and collecting metrics.
+     * Time periods in which the browser was active and collecting metrics
      * and events.
      * @type {!Array.<{start: !number, end: !number}>}
      * @private
      */
     this.intervals_ = [];
 
-    this.setupCheckboxes_(
-        '#chooseMetrics', this.metricMap_, this.addMetric, this.dropMetric);
-    this.setupCheckboxes_(
-        '#chooseEvents', this.eventMap_, this.addEventType, this.dropEventType);
     this.setupTimeRangeChooser_();
+    chrome.send('getAllEventTypes');
+    chrome.send('getAllMetricTypes');
     this.setupMainChart_();
-    TimeRange_.day.element.click().call(this);
+    TimeRange_.day.element.click();
   }
 
   PerformanceMonitor.prototype = {
     /**
+     * Receive a list of all metrics, and populate |this.metricMap_| to
+     * reflect said list. Reconfigure the checkbox set for metric selection.
+     * @param {Array.<{metricType: !String, shortDescription: !String}>}
+     *   allMetrics All metrics from which to select.

[Dan Beam, 2012/08/03 18:40:36]

nit: 4 \s instead of 2 \s before allMetrics

[clintstaley, 2012/08/06 21:06:47]

Done.

+     */
+    getAllMetricTypesCallback: function(allMetrics) {
+      for (var i = 0; i < allMetrics.length; i++) {
+        var metric = allMetrics[i];
+
+        this.metricMap_[metric.metricType] = {
+          id: metric.metricType,
+          description: metric.shortDescription,
+          units: metric.units,
+          yAxis: {min: 0, max: metric.maxValue, color: 'rgb(0, 0, 255'},
+          divs: [],
+          data: null
+        };
+      }
+
+      this.setupCheckboxes_($('#chooseMetrics')[0],
+          this.metricMap_, this.addMetric, this.dropMetric);
+    },
+
+    /**
+     * Receive a list of all event types, and populate |this.eventMap_| to
+     * reflect said list. Reconfigure the checkbox set for event selection.
+     * @param {Array.<{eventType: !String, shortDescription: !String}>}
+     *   allEvents All events from which to select.
+     */
+    getAllEventTypesCallback: function(allEvents) {
+      for (var i = 0; i < allEvents.length; i++) {
+        var eventInfo = allEvents[i];
+
+        this.eventMap_[eventInfo.eventType] = {
+          id: eventInfo.eventType,
+          color: 'rgb(255, 0, 0)',
+          data: null,
+          description: eventInfo.shortDescription,
+          divs: []
+        };
+      }
+
+      this.setupCheckboxes_($('#chooseEvents')[0],
+          this.eventMap_, this.addEventType, this.dropEventType);
+    },
+
+    /**
      * Set up the radio button set to choose time range. Use div#radioTemplate
      * as a template.
      * @private
@@ -154,13 +178,13 @@ var Installer = function() {
 
     /**
      * Generalized function for setting up checkbox blocks for either events
-     * or metrics. Take a div ID |divId| into which to place the checkboxes,
+     * or metrics. Take a div |div| into which to place the checkboxes,
      * and a map |optionMap| with values that each include a property
      * |description|. Set up one checkbox for each entry in |optionMap|
      * labelled with that description. Arrange callbacks to function |check|
      * or |uncheck|, passing them the key of the checked or unchecked option,
      * when the relevant checkbox state changes.
-     * @param {!string} divId Id of division into which to put checkboxes
+     * @param {!HTMLDivElement} div div into which to put checkboxes

[Dan Beam, 2012/08/03 18:40:36]

Generally we capitalize after the parameter name and add a period at the end
(implying the description should be a full sentence).

  @param {!HTMLDivElement} div A <div> into which to put checkboxes.

(something like this)

[clintstaley, 2012/08/06 21:06:47]

Done, here and elsewhere.  Of course, it's a sentence fragment either way, but
it is certainly more readable as a noun phrase with the capital to separate it
off, which I assume is the main idea (?).

      * @param {!Object} optionMap map of metric/event entries
      * @param {!function(this:Controller, Object)} check
      *     function to select an entry (metric or event)
@@ -168,23 +192,19 @@ var Installer = function() {
      *     function to deselect an entry (metric or event)
      * @private
      */
-    setupCheckboxes_: function(divId, optionMap, check, uncheck) {
+    setupCheckboxes_: function(div, optionMap, check, uncheck) {
       var checkboxTemplate = $('#checkboxTemplate')[0];
-      var chooseMetricsDiv = $(divId)[0];
 
       for (var option in optionMap) {
         var checkbox = checkboxTemplate.cloneNode(true);
         checkbox.querySelector('span').innerText = 'Show ' +
             optionMap[option].description;
-        chooseMetricsDiv.appendChild(checkbox);
+        div.appendChild(checkbox);
 
         var input = checkbox.querySelector('input');
-        input.option = option;
+        input.option = optionMap[option].id;
         input.addEventListener('change', function(e) {
-          if (e.target.checked)
-            check.call(this, e.target.option);
-          else
-            uncheck.call(this, e.target.option);
+          (e.target.checked ? check : uncheck).call(this, e.target.option);
         }.bind(this));
       }
     },
@@ -213,36 +233,15 @@ var Installer = function() {
       this.end = Math.floor(Date.now() / range.resolution) *
           range.resolution;
 
-      // Take the GMT value of this.end ("now") and subtract from it the
-      // number of minutes by which we lag GMT in the present timezone,
-      // X mS/minute. This will show time in the present timezone.
-      this.end -= new Date().getTimezoneOffset() * 60000;
       this.start = this.end - range.timeSpan;
       this.requestIntervals();
     },
 
     /**
-     * Return mock interval set for testing.
-     * @return {!Array.<{start: !number, end: !number}>} intervals
-     */
-    getMockIntervals: function() {
-      var interval = this.end - this.start;
-
-      return [
-        {start: this.start + interval * .1,
-         end: this.start + interval * .2},
-        {start: this.start + interval * .7,
-         end: this.start + interval}
-      ];
-    },
-
-    /**
      * Request activity intervals in the current time range.
      */
     requestIntervals: function() {
-      this.onReceiveIntervals(this.getMockIntervals());
-      // Replace with: chrome.send('getIntervals', this.start, this.end,
-      // this.onReceiveIntervals);
+      chrome.send('getActiveIntervals', [this.start, this.end]);
     },
 
     /**
@@ -251,7 +250,7 @@ var Installer = function() {
      * all metrics and event types that are currently selected.
      * @param {!Array.<{start: !number, end: !number}>} intervals new intervals
      */
-    onReceiveIntervals: function(intervals) {
+    getActiveIntervalsCallback: function(intervals) {
       this.intervals_ = intervals;
 
       for (var metric in this.metricMap_) {
@@ -263,7 +262,7 @@ var Installer = function() {
       for (var eventType in this.eventMap_) {
         var eventValue = this.eventMap_[eventType];
         if (eventValue.divs.length > 0)
-          this.refreshEventType(eventType);
+          this.refreshEventType(eventValue.id);
       }
     },
 
@@ -278,7 +277,7 @@ var Installer = function() {
 
     /**
      * Remove a metric from the chart(s).
-     * @param {!string} metric Metric to stop displaying
+     * @param {!string} metric Metric to stop displaying.
      */
     dropMetric: function(metric) {
       var metricValue = this.metricMap_[metric];
@@ -289,61 +288,35 @@ var Installer = function() {
     },
 
     /**
-     * Return mock metric data points for testing. Give values ranging from
-     * offset to max-offset. (This lets us avoid direct overlap of
-     * different mock data sets in an ugly way that will die in the next
-     * version anyway.)
-     * @param {!number} max Max data value to return, less offset
-     * @param {!number} offset Adjustment factor
-     * @return {!Array.<{time: !number, value: !number}>}
-     */
-    getMockDataPoints: function(max, offset) {
-      var dataPoints = [];
-
-      for (var i = 0; i < this.intervals_.length; i++) {
-        // Rise from low offset to high max-offset in 100 point steps.
-        for (var time = this.intervals_[i].start;
-             time <= this.intervals_[i].end; time += this.range.resolution) {
-          var numPoints = time / this.range.resolution;
-          dataPoints.push({time: time, value: offset + (numPoints % 100) *
-              (max - 2 * offset) / 100});
-        }
-      }
-      return dataPoints;
-    },
-
-    /**
      * Request new metric data, assuming the metric table and chart already
      * exist.
-     * @param {!string} metric Metric for which to get data
+     * @param {!string} metric Metric for which to get data.
      */
     refreshMetric: function(metric) {
       var metricValue = this.metricMap_[metric];
 
       metricValue.data = null;  // Mark metric as awaiting response.
-      this.onReceiveMetric(metric,
-          this.getMockDataPoints(metricValue.yAxis.max, 5));
-      // Replace with:
-      // chrome.send("getMetric", this.range.start, this.range.end,
-      //  this.range.resolution, this.onReceiveMetric);
+      chrome.send('getMetric', [metricValue.id,
+          this.start, this.end, this.range.resolution]);
     },
 
     /**
-     * Receive new datapoints for |metric|, convert the data to Flot-usable
+     * Receive new datapoints for a metric, convert the data to Flot-usable
      * form, and redraw all affected charts.
-     * @param {!string} metric Metric to which |points| applies
-     * @param {!Array.<{time: !number, value: !number}>} points
-     *     new data points
+     * @param {!{
+     *   type: !number,
+     *   points: !Array.<{time: !number, value: !number}>
+     * }} result Object giving metric ID, and time/value point pairs for
+     *     that id.
      */
-    onReceiveMetric: function(metric, points) {
-      var metricValue = this.metricMap_[metric];
-
+    getMetricCallback: function(result) {
+      var metricValue = this.metricMap_[result.metricType];
       // Might have been dropped while waiting for data.
       if (metricValue.divs.length == 0)
         return;
 
       var series = [];
-      metricValue.data = [series];
+      metricValue.data = [series];  // Data ends with current open series.
 
       // Traverse the points, and the intervals, in parallel. Both are in
       // ascending time order. Create a sequence of data "series" (per Flot)
@@ -351,28 +324,28 @@ var Installer = function() {
       var interval = this.intervals_[0];
       var intervalIndex = 0;
       var pointIndex = 0;
-      while (pointIndex < points.length &&
+      while (pointIndex < result.points.length &&
           intervalIndex < this.intervals_.length) {
-        var point = points[pointIndex++];
+        var point = result.points[pointIndex++];
         while (intervalIndex < this.intervals_.length &&
             point.time > interval.end) {
-          interval = this.intervals_[++intervalIndex]; // Jump to new interval.
+          interval = this.intervals_[++intervalIndex];  // Jump to new interval.
           if (series.length > 0) {
-            series = [];                   // Start a new series.
-            metricValue.data.push(series); // Put it on the end of the data.
+            series = [];  // Start a new series.
+            metricValue.data.push(series);  // Put it on the end of the data.
           }
         }
         if (intervalIndex < this.intervals_.length &&
-            point.time > interval.start)
-          series.push([point.time, point.value]);
+            point.time > interval.start) {
+          series.push([point.time - timezoneOffset, point.value]);
+        }
       }
-
       metricValue.divs.forEach(this.drawChart, this);
     },
 
     /**
      * Add a new event to the chart(s).
-     * @param {!string} eventType type of event to start displaying
+     * @param {!string} eventType type of event to start displaying.
      */
     addEventType: function(eventType) {
       // Events show on all charts.
@@ -393,59 +366,38 @@ var Installer = function() {
     },
 
     /**
-     * Return mock event points for testing.
-     * @param {!string} eventType type of event to generate mock data for
-     * @return {!Array.<{time: !number, longDescription: !string}>}
-     */
-    getMockEventValues: function(eventType) {
-      var mockValues = [];
-      for (var i = 0; i < this.intervals_.length; i++) {
-        var interval = this.intervals_[i];
-
-        mockValues.push({
-            time: interval.start,
-            longDescription: eventType + ' at ' +
-            new Date(interval.start) + ' blah, blah blah'});
-        mockValues.push({
-            time: (interval.start + interval.end) / 2,
-            longDescription: eventType + ' at ' +
-            new Date((interval.start + interval.end) / 2) + ' blah, blah'});
-        mockValues.push({
-            time: interval.end,
-            longDescription: eventType + ' at ' + new Date(interval.end) +
-            ' blah, blah blah'});
-      }
-      return mockValues;
-    },
-
-    /**
      * Request new data for |eventType|, for times in the current range.
      * @param {!string} eventType type of event to get new data for
      */
     refreshEventType: function(eventType) {
       // Mark eventType as awaiting response.
       this.eventMap_[eventType].data = null;
-      this.onReceiveEventType(eventType, this.getMockEventValues(eventType));
-      // Replace with:
-      // chrome.send("getEvents", eventType, this.range.start, this.range.end);
+
+      chrome.send('getEvents', [eventType, this.start, this.end]);
     },
 
     /**
-     * Receive new data for |eventType|. If the event has been deselected while
-     * awaiting webui response, do nothing. Otherwise, save the data directly,
-     * since events are handled differently than metrics when drawing
+     * Receive new events for |eventType|. If |eventType| has been deselected
+     * while awaiting webui response, do nothing. Otherwise, save the data
+     * directly, since events are handled differently than metrics when drawing
      * (no "series"), and redraw all the affected charts.
-     * @param {!string} eventType type of event the new data applies to
-     * @param {!Array.<{time: !number, longDescription: !string}>} values
-     *     new event values
+     * @param {!{
+     *   type: !number,
+     *   points: !Array.<{time: !number, longDescription: string}>
+     * }} result Object giving eventType ID, and time/description pairs for
+     *     each event of that type in the range requested.
      */
-    onReceiveEventType: function(eventType, values) {
-      var eventValue = this.eventMap_[eventType];
+    getEventsCallback: function(result) {
+      var eventValue = this.eventMap_[result.eventType];
 
       if (eventValue.divs.length == 0)
         return;
 
-      eventValue.data = values;
+      result.points.forEach(function(elememt) {
+        element.time -= timezoneOffset;
+      });
+
+      eventValue.data = result.points;
       eventValue.divs.forEach(this.drawChart, this);
     },
 
@@ -524,11 +476,15 @@ var Installer = function() {
         var eventValue = eventValues[i];
         for (var d = 0; d < eventValue.data.length; d++) {
           var point = eventValue.data[d];
-          markings.push({
-            color: eventValue.color,
-            description: eventValue.description,
-            xaxis: {from: point.time, to: point.time}
-          });
+          if (point.time >= this.start && point.time <= this.end) {
+            markings.push({
+              color: eventValue.color,
+              description: eventValue.description,
+              xaxis: {from: point.time, to: point.time}
+            });
+          } else {
+            console.log('Event out of time range at: ' + point.time);
+          }
         }
       }
 
@@ -561,16 +517,26 @@ var Installer = function() {
         }
       });
 
+      // Ensure at least one y axis, as a reference for event placement.
+      if (yAxes.length == 0)
+        yAxes.push({max: 1.0});
+
       var markings = this.getEventMarks_(chartData.events);
       var chart = this.charts[0];
       var plot = $.plot(chart, seriesSeq, {
         yaxes: yAxes,
-        xaxis: {mode: 'time'},
+        xaxis: {
+          mode: 'time',
+          min: this.start - timezoneOffset,
+          max: this.end - timezoneOffset
+        },
+        points: {show: true, radius: 1},
+        lines: {show: true},
         grid: {markings: markings}
       });
 
       // For each event in |markings|, create also a label div, with left
-      // edge colinear with the event vertical-line. Top of label is
+      // edge colinear with the event vertical line. Top of label is
       // presently a hack-in, putting labels in three tiers of 25px height
       // each to avoid overlap. Will need something better.
       var labelTemplate = $('#labelTemplate')[0];
@@ -582,6 +548,7 @@ var Installer = function() {
         labelDiv.innerText = mark.description;
         labelDiv.style.left = point.left + 'px';
         labelDiv.style.top = (point.top + 25 * (i % 3)) + 'px';
+
         chart.appendChild(labelDiv);
       }
     }
@@ -589,6 +556,6 @@ var Installer = function() {
   return {
     PerformanceMonitor: PerformanceMonitor
   };
-}();
+});
 
-var performanceMonitor = new Installer.PerformanceMonitor();
+var PerformanceMonitor = new performance_monitor.PerformanceMonitor();