Adds cache and buffer graphs to the properties pane.

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

Index: content/browser/resources/media/new/client_renderer.js
diff --git a/content/browser/resources/media/new/client_renderer.js b/content/browser/resources/media/new/client_renderer.js
index 02f47e163cf53dc1c5e3ea24066f03a76f1c0a3f..6ffd1f3c0880d06bcd46baaa0c15bf78702a27ef 100644
--- a/content/browser/resources/media/new/client_renderer.js
+++ b/content/browser/resources/media/new/client_renderer.js
@@ -8,6 +8,7 @@ var ClientRenderer = (function() {
     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;
@@ -76,6 +77,7 @@ var ClientRenderer = (function() {
       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);
@@ -98,6 +100,7 @@ var ClientRenderer = (function() {
       this.selectedPlayer = null;
       this.drawProperties_(audioStream);
       removeChildren(this.logTable.querySelector('tbody'));
+      removeChildren(this.graphElement);
     },
 
     redrawPlayerList_: function(players) {
@@ -123,7 +126,9 @@ var ClientRenderer = (function() {
       this.drawProperties_(player.properties);
 
       removeChildren(this.logTable.querySelector('tbody'));
+      removeChildren(this.graphElement);
       this.drawLog_();
+      this.drawGraphs_();
     },
 
     drawProperties_: function(propertyMap) {
@@ -131,6 +136,7 @@ var ClientRenderer = (function() {
 
       for (key in propertyMap) {
         var value = propertyMap[key];
+
         var row = this.propertiesTable.insertRow(-1);
         var keyCell = row.insertCell(-1);
         var valueCell = row.insertCell(-1);
@@ -154,7 +160,70 @@ var ClientRenderer = (function() {
           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;