Adds cache and buffer graphs to the properties pane.

content/browser/resources/media/new/client_renderer.js

 // Copyright 2013 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.
 
 var ClientRenderer = (function() {
   var ClientRenderer = function() {
     this.playerListElement = document.getElementById('player-list');
     this.audioStreamListElement = document.getElementById('audio-stream-list');
     this.propertiesTable = document.getElementById('property-table');
     this.logTable = document.getElementById('log');
+    this.graphElement = document.getElementById('graphs');
 
     this.selectedPlayer = null;
     this.selectedStream = null;
 
     this.selectedPlayerLogIndex = 0;
   };
 
   function removeChildren(element) {
     while (element.hasChildNodes()) {
       element.removeChild(element.lastChild);
@@ skipping 48 matching lines @@
      * Called when a property on a player is changed.
      * @param players The entire map of id -> player.
      * @param player The player that had its property changed.
      * @param key The name of the property that was changed.
      * @param value The new value of the property.
      */
     playerUpdated: function(players, player, key, value) {
       if (player === this.selectedPlayer) {
         this.drawProperties_(player.properties);
         this.drawLog_();
+        this.drawGraphs_();

[scherkus (not reviewing), 2013/09/04 23:20:53]

I noticed you don't call anything on ClientRenderer when there are network
updates

is playerUpdated() called frequently enough to draw the graph?

[Ty Overby, 2013/09/05 21:01:02]

Yeah, buffer_start, buffer_end, buffer_current get spammed *a lot*.  

       }
       if (key === 'name' || key === 'url') {
         this.redrawPlayerList_(players);
       }
     },
 
     redrawAudioStreamList_: function(streams) {
       removeChildren(this.audioStreamListElement);
 
       for (id in streams) {
         var li = document.createElement('li');
         li.appendChild(createButton(
             id, this.selectAudioStream_.bind(this, streams[id])));
         this.audioStreamListElement.appendChild(li);
       }
     },
 
     selectAudioStream_: function(audioStream) {
       this.selectedStream = audioStream;
       this.selectedPlayer = null;
       this.drawProperties_(audioStream);
       removeChildren(this.logTable.querySelector('tbody'));
+      removeChildren(this.graphElement);
     },
 
     redrawPlayerList_: function(players) {
       removeChildren(this.playerListElement);
 
       for (id in players) {
         var li = document.createElement('li');
         var player = players[id];
         var usableName = player.properties.name ||
             player.properties.url ||
             'player ' + player.id;
 
         li.appendChild(createButton(
             usableName, this.selectPlayer_.bind(this, player)));
         this.playerListElement.appendChild(li);
       }
     },
 
     selectPlayer_: function(player) {
       this.selectedPlayer = player;
       this.selectedPlayerLogIndex = 0;
       this.selectedStream = null;
       this.drawProperties_(player.properties);
 
       removeChildren(this.logTable.querySelector('tbody'));
+      removeChildren(this.graphElement);
       this.drawLog_();
+      this.drawGraphs_();
     },
 
     drawProperties_: function(propertyMap) {
       removeChildren(this.propertiesTable);
 
       for (key in propertyMap) {
         var value = propertyMap[key];
+
         var row = this.propertiesTable.insertRow(-1);
         var keyCell = row.insertCell(-1);
         var valueCell = row.insertCell(-1);
 
         keyCell.appendChild(document.createTextNode(key));
         valueCell.appendChild(document.createTextNode(value));
       }
     },
 
     appendEventToLog_: function(event) {
       var row = this.logTable.querySelector('tbody').insertRow(-1);
 
       row.insertCell(-1).appendChild(document.createTextNode(
           util.millisecondsToString(event.time)));
       row.insertCell(-1).appendChild(document.createTextNode(event.key));
       row.insertCell(-1).appendChild(document.createTextNode(event.value));
     },
 
     drawLog_: function() {
       var toDraw = this.selectedPlayer.allEvents.slice(
           this.selectedPlayerLogIndex);
       toDraw.forEach(this.appendEventToLog_.bind(this));
       this.selectedPlayerLogIndex = this.selectedPlayer.allEvents.length;
-    }
+    },

[scherkus (not reviewing), 2013/09/04 23:20:53]

nit: add blank line after this

[Ty Overby, 2013/09/05 21:01:02]

Done.

+    drawGraphs_: function() {
+      removeChildren(this.graphElement);

[scherkus (not reviewing), 2013/09/04 23:20:53]

think we can get away with recreating the graphs DOM every time?

perhaps move this call to when we need to do it, then have drawGraphs_() lazily
construct the DOM whenever the parent element of a canvas is non-null?

[Ty Overby, 2013/09/05 21:01:02]

I did it a tad differently, but to the same effect. 

+
+      var addToGraphs = (function(name, graph) {
+        var li = document.createElement('li');
+        li.appendChild(graph);
+        li.appendChild(document.createTextNode(name));
+        this.graphElement.appendChild(li);
+      }).bind(this);
+
+      if (this.selectedPlayer && this.selectedPlayer.properties.url) {

[scherkus (not reviewing), 2013/09/04 23:20:53]

it looks like this.selectedPlayer should always be non-null when drawGraphs_()
is called

can we remove the check?

also if properties.url isn't present it'd be preferable to early-return so we
don't have deeply-nested ifs

[Ty Overby, 2013/09/05 21:01:02]

Yep, check can be removed.

Done.

+        var url = this.selectedPlayer.properties.url;
+        var cache = media.cacheForUrl(url);
+
+        var player = this.selectedPlayer;
+        var props = player.properties;
+
+        if (props['buffer_start'] !== undefined &&
+            props['buffer_current'] !== undefined &&
+            props['buffer_end'] !== undefined &&
+            props['total_bytes'] !== undefined) {
+          addToGraphs('buffer',
+              this.drawBufferGraph_(props['buffer_start'],
+                                    props['buffer_current'],
+                                    props['buffer_end'],
+                                    props['total_bytes']));
+        }
+
+        if (cache) {
+          addToGraphs('cache read', cache.readCanvas);
+          addToGraphs('cache write', cache.writeCanvas);
+
+          cache.size = cache.size || +player.properties['total_bytes'];
+          cache.generateDetails();
+        }
+      }
+    },

[scherkus (not reviewing), 2013/09/04 23:20:53]

add blank line after

[Ty Overby, 2013/09/05 21:01:02]

Done.

+    drawBufferGraph_: (function() {
+        // Make a single canvas element for every buffer-graph to be drawn on
+        // and close over it with a module.

[scherkus (not reviewing), 2013/09/04 23:20:53]

any reason not to use a member variable canvas versus a closure?

right now the code is hard to piece together and the calling code looks funny
because drawBufferGraph_() not only draws, but returns the canvas to add as well

it'd be preferable to have drawBufferGraph_() stick to drawing, then make the
call to addToGraphs() separately similar to how cache is handled

[Ty Overby, 2013/09/05 21:01:02]

Yeah, I agree, I thought it was going to be cleaner this way, but it ended up
quite confusing. 

Done.

+        var canvas = document.createElement('canvas');
+        canvas.width = media.BAR_WIDTH;
+        canvas.height = media.BAR_HEIGHT;
+
+        var ctx = canvas.getContext('2d');
+        return function(start, current, end, size) {
+
+          ctx.fillStyle = '#aaa';
+          ctx.fillRect(0, 0, canvas.width, canvas.height);
+
+          var scale_factor = canvas.width / size;
+          var left = start * scale_factor;
+          var middle = current * scale_factor;
+          var right = end * scale_factor;
+
+          ctx.fillStyle = '#a0a';
+          ctx.fillRect(left, 0, middle - left, canvas.height);
+          ctx.fillStyle = '#aa0';
+          ctx.fillRect(middle, 0, right - middle, canvas.height);
+
+          return canvas;
+        }
+      })()
   };
 
   return ClientRenderer;
 })();