Tons of timeline tweaks

chrome/browser/resources/tracing/timeline.js

Index: chrome/browser/resources/tracing/timeline.js
diff --git a/chrome/browser/resources/tracing/timeline.js b/chrome/browser/resources/tracing/timeline.js
index eecf3ab0063d5d1323fa12dfc5ee10e3b1af2691..d9e60d4946b1e936291db21e2fefed178c348a96 100644
--- a/chrome/browser/resources/tracing/timeline.js
+++ b/chrome/browser/resources/tracing/timeline.js
@@ -135,6 +135,59 @@ cr.define('tracing', function() {
   };
 
   /**
+   * Uses an embedded iframe to measure provided elements without forcing layout
+   * on the main document.
+   * @constructor
+   * @extends {Object}
+   */
+  function MeasuringStick()

[James Hawkins, 2011/11/05 17:51:37]

Move this into a separate file.

[nduca, 2011/11/09 22:52:19]

Done.

+  {

[James Hawkins, 2011/11/05 17:51:37]

Opening brace should be on the same line as the method.

[nduca, 2011/11/09 22:52:19]

Done.

+    var iframe = document.createElement('iframe');
+    iframe.style.cssText = 'width:100%;height:0;border:0;visibility:hidden';
+    document.body.appendChild(iframe);
+    this._doc = iframe.contentDocument;
+    this._window = iframe.contentWindow;
+    this._doc.body.style.cssText = 'padding:0;margin:0;overflow:hidden';
+
+    var stylesheets = document.querySelectorAll('link[rel=stylesheet]');
+    for (var i = 0; i < stylesheets.length; ++i) {
+        var stylesheet = stylesheets[i];
+        var link = this._doc.createElement('link');
+        link.rel = 'stylesheet';
+        link.href = stylesheet.href;
+        this._doc.head.appendChild(link);
+    }
+  }
+
+  MeasuringStick.prototype = {
+    __proto__: Object.prototype,
+
+    /**
+     * Converts measures like 50px to their size in pixels, if possible.
+     */
+    convertMeasureToPixels_: function(str) {

[James Hawkins, 2011/11/05 17:51:37]

No need for this method.

parseInt(str, 10);

[nduca, 2011/11/09 22:52:19]

Done.

+      var g = /(\d+)px/.exec(str);
+      if (g)
+        return parseInt(g[0]);
+      throw 'Unrecognized unit on ' + str;
+    },
+
+    /**
+     * Measures the provided element without forcing layout on the main
+     * document.
+     */
+    measure: function(element)
+    {
+      this._doc.body.appendChild(element);
+      var style = this._window.getComputedStyle(element);
+      var width = this.convertMeasureToPixels_(style.width);
+      var height = this.convertMeasureToPixels_(style.height);
+      this._doc.body.removeChild(element);
+      return { width: width, height: height };
+    }
+  };
+
+  /**
    * Renders a TimelineModel into a div element, making one
    * TimelineTrack for each subrow in each thread of the model, managing
    * overall track layout, and handling user interaction with the
@@ -167,13 +220,17 @@ cr.define('tracing', function() {
       this.dragBox_ = this.ownerDocument.createElement('div');
       this.dragBox_.className = 'timeline-drag-box';
       this.appendChild(this.dragBox_);
+      this.hideDragBox_();
 
       // The following code uses a setInterval to monitor the timeline control
       // for size changes. This is so that we can keep the canvas' bitmap size
       // correctly synchronized with its presentation size.
       // TODO(nduca): detect this in a more efficient way, e.g. iframe hack.
       this.lastSize_ = this.clientWidth + 'x' + this.clientHeight;
-      this.ownerDocument.defaultView.setInterval(function() {
+      this.checkForResizeInterval_ =
+          this.ownerDocument.defaultView.setInterval(function() {
+        if (!this.isAttachedToDocument_)
+          return;
         var curSize = this.clientWidth + 'x' + this.clientHeight;
         if (this.clientWidth && curSize != this.lastSize_) {
           this.lastSize_ = curSize;
@@ -181,18 +238,38 @@ cr.define('tracing', function() {
         }
       }.bind(this), 250);
 
-      document.addEventListener('keypress', this.onKeypress_.bind(this));
-      document.addEventListener('keydown', this.onKeydown_.bind(this));
-      document.addEventListener('mousedown', this.onMouseDown_.bind(this));
-      document.addEventListener('mousemove', this.onMouseMove_.bind(this));
-      document.addEventListener('mouseup', this.onMouseUp_.bind(this));
-      document.addEventListener('dblclick', this.onDblClick_.bind(this));
+      this.safeAddEventListener(document, 'keypress', this.onKeypress_, this);
+      this.safeAddEventListener(document, 'keydown', this.onKeydown_, this);
+      this.safeAddEventListener(document, 'mousedown', this.onMouseDown_, this);
+      this.safeAddEventListener(document, 'mousemove', this.onMouseMove_, this);
+      this.safeAddEventListener(document, 'mouseup', this.onMouseUp_, this);
+      this.safeAddEventListener(document, 'dblclick', this.onDblClick_, this);
 
       this.lastMouseViewPos_ = {x: 0, y: 0};
 
       this.selection_ = [];
     },
 
+    safeAddEventListener: function(object, event, func, target) {

[James Hawkins, 2011/11/05 17:51:37]

Document this method and the params.

In the comments, describe why using this method is necessary.

[James Hawkins, 2011/11/05 17:51:37]

This method should likely be private.

[nduca, 2011/11/09 22:52:19]

Done.

[nduca, 2011/11/09 22:52:19]

Done.

+      if (!this.boundListeners_)
+        this.boundListeners_ = [];
+      var boundFunc = func.bind(target);
+      this.boundListeners_.push({object: object,
+        event: event,
+        boundFunc: boundFunc});
+      object.addEventListener(event, boundFunc);
+    },
+
+    detach: function() {
+      for (var i = 0; i < this.boundListeners_.length; ++i) {
+        var binding = this.boundListeners_[i];
+        binding.object.removeEventListener(binding.event, binding.boundFunc);
+      }
+      this.boundListeners_ = undefined;
+      window.clearInterval(this.checkForResizeInterval_);
+      this.checkForResizeInterval_ = undefined;
+    },
+
     get model() {
       return this.model_;
     },
@@ -205,20 +282,43 @@ cr.define('tracing', function() {
       }
       this.model_ = model;
 
-      // Create tracks.
-      this.tracks_.textContent = '';
+      // Create tracks and measure their heading size

[James Hawkins, 2011/11/05 17:51:37]

nit: Period at end of sentence.

[nduca, 2011/11/09 22:52:19]

Done.

       var threads = model.getAllThreads();
-      threads.sort(tracing.TimelineThread.compare);
+      var maxHeadingWidth = 0;
+      var tracks = [];
+      var measuringStick = new MeasuringStick();
+      var headingEl = document.createElement('div');
+      headingEl.style.position = 'fixed';
+      headingEl.className = 'timeline-slice-track-title';
       for (var tI = 0; tI < threads.length; tI++) {
         var thread = threads[tI];
         var track = new TimelineThreadTrack();
         track.thread = thread;
         track.viewport = this.viewport_;
-        this.tracks_.appendChild(track);
+        tracks.push(track);
+        headingEl.textContent = track.heading;
+        var w = measuringStick.measure(headingEl).width;
+        // limit heading width to 300px

[James Hawkins, 2011/11/05 17:51:37]

Comment sentence should start with a capital letter; add period.

[nduca, 2011/11/09 22:52:19]

Done.

+        if (w > 300)
+          w = 300;
+        if (w > maxHeadingWidth)
+          maxHeadingWidth = w;
+      }
+      maxHeadingWidth += maxHeadingWidth + 4
 
+      // Attach tracks and set width

[James Hawkins, 2011/11/05 17:51:37]

Add period.

[nduca, 2011/11/09 22:52:19]

Done.

+      this.tracks_.textContent = '';
+      threads.sort(tracing.TimelineThread.compare);
+      for (var tI = 0; tI < tracks.length; tI++) {
+        var track = tracks[tI];
+        track.headingWidth = maxHeadingWidthWithUnit + 'px';
+        this.tracks_.appendChild(track);
       }
 
-      this.needsViewportReset_ = true;
+      if (this.isAttachedToDocument_)
+        this.onResize();
+      else
+        this.needsViewportReset_ = true;
     },
 
     viewportChange_: function() {
@@ -229,27 +329,42 @@ cr.define('tracing', function() {
       if (this.invalidatePending_)
         return;
       this.invalidatePending_ = true;
-      window.setTimeout(function() {
-        this.invalidatePending_ = false;
-        this.redrawAllTracks_();
-      }.bind(this), 0);
+      if (this.isAttachedToDocument_)

[James Hawkins, 2011/11/05 17:51:37]

Braces required for multi-line blocks.

[nduca, 2011/11/09 22:52:19]

Done.

+        window.setTimeout(function() {
+          this.invalidatePending_ = false;
+          this.redrawAllTracks_();
+        }.bind(this), 0);
+    },
+
+    get isAttachedToDocument_() {

[James Hawkins, 2011/11/05 17:51:37]

Document return type.

[nduca, 2011/11/09 22:52:19]

Done.

+      var cur = this;
+      while (cur.parentNode)
+        cur = cur.parentNode;
+      return cur == this.ownerDocument;
     },
 
     onResize: function() {
+      if (!this.isAttachedToDocument_)
+        throw 'Not attached to document!';
       for (var i = 0; i < this.tracks_.children.length; ++i) {
         var track = this.tracks_.children[i];
         track.onResize();
       }
+      if (this.invalidatePending_) {
+        this.invalidatePending_ = false;
+        this.redrawAllTracks_();
+      }
     },
 
     redrawAllTracks_: function() {
       if (this.needsViewportReset_ && this.clientWidth != 0) {
+        if (!this.isAttachedToDocument_)
+          throw 'Not attached to document!';
         this.needsViewportReset_ = false;
         /* update viewport */
         var rangeTimestamp = this.model_.maxTimestamp -
             this.model_.minTimestamp;
         var w = this.firstCanvas.width;
-        console.log('viewport was reset with w=', w);
         var scaleX = w / rangeTimestamp;
         var panX = -this.model_.minTimestamp;
         this.viewport_.setPanAndScale(panX, scaleX);
@@ -266,10 +381,18 @@ cr.define('tracing', function() {
       }
     },
 
+    get listenToKeys_() {
+      if (this.parentElement.parentElement.tabIndex >= 0)
+        return document.activeElement == this.parentElement.parentElement;
+      return true;
+    },
+
     onKeypress_: function(e) {
       var vp = this.viewport_;
       if (!this.firstCanvas)
         return;
+      if (!this.listenToKeys_)
+        return;
       var viewWidth = this.firstCanvas.clientWidth;
       var curMouseV, curCenterW;
       switch (e.keyCode) {
@@ -316,6 +439,8 @@ cr.define('tracing', function() {
 
     // Not all keys send a keypress.
     onKeydown_: function(e) {
+      if (!this.listenToKeys_)
+        return;
       switch (e.keyCode) {
         case 37:   // left arrow
           this.selectPrevious_(e);
@@ -326,11 +451,13 @@ cr.define('tracing', function() {
           e.preventDefault();
           break;
         case 9:    // TAB
-          if (e.shiftKey)
-            this.selectPrevious_(e);
-          else
-            this.selectNext_(e);
-          e.preventDefault();
+          if (this.parentElement.parentElement.tabIndex == -1) {
+            if (e.shiftKey)
+              this.selectPrevious_(e);
+            else
+              this.selectNext_(e);
+            e.preventDefault();
+          }
           break;
       }
     },
@@ -373,8 +500,8 @@ cr.define('tracing', function() {
       for (i = 0; i < this.selection_.length; ++i) {
         adjoining = undefined;
         this.selection_[i].slice.selected = false;
-        var track = this.selection_[i].track;
-        var slice = this.selection_[i].slice;
+        track = this.selection_[i].track;
+        slice = this.selection_[i].slice;
         if (slice) {
           if (forwardp)
             adjoining = track.pickNext(slice);
@@ -394,17 +521,23 @@ cr.define('tracing', function() {
     },
 
     get keyHelp() {
-      return 'Keyboard shortcuts:\n' +
+      var help = 'Keyboard shortcuts:\n' +
           ' w/s     : Zoom in/out    (with shift: go faster)\n' +
           ' a/d     : Pan left/right\n' +
           ' e       : Center on mouse\n' +
-          ' g/G     : Shows grid at the start/end of the selected task\n' +
-          ' <-,^TAB : Select previous event on current timeline\n' +
-          ' ->, TAB : Select next event on current timeline\n' +
-        '\n' +
-        'Dbl-click to zoom in; Shift dbl-click to zoom out\n';
+          ' g/G     : Shows grid at the start/end of the selected task\n';
 
+      if (this.parentElement.parentElement.tabIndex)
+        help += ' <-      : Select previous event on current timeline\n' +
+            ' ->      : Select next event on current timeline\n';
+      else
+        help += ' <-,^TAB : Select previous event on current timeline\n' +
+            ' ->, TAB : Select next event on current timeline\n';
 
+      help +=
+          '\n' +
+          'Dbl-click to zoom in; Shift dbl-click to zoom out\n';
+      return help;
     },
 
     get selection() {
@@ -416,20 +549,11 @@ cr.define('tracing', function() {
           this.tracks_.firstChild.firstCanvas : undefined;
     },
 
-    showDragBox_: function() {
-      this.dragBox_.hidden = false;
-    },
-
     hideDragBox_: function() {
       this.dragBox_.style.left = '-1000px';
       this.dragBox_.style.top = '-1000px';
       this.dragBox_.style.width = 0;
       this.dragBox_.style.height = 0;
-      this.dragBox_.hidden = true;
-    },
-
-    get dragBoxVisible_() {
-      return this.dragBox_.hidden == false;
     },
 
     setDragBoxPosition_: function(eDown, eCur) {
@@ -490,6 +614,8 @@ cr.define('tracing', function() {
 
       this.dragBeginEvent_ = e;
       e.preventDefault();
+      if (this.parentElement.parentElement.tabIndex)
+        this.parentElement.parentElement.focus();
     },
 
     onMouseMove_: function(e) {
@@ -504,12 +630,6 @@ cr.define('tracing', function() {
       // Remember position. Used during keyboard zooming.
       this.lastMouseViewPos_ = pos;
 
-      // Initiate the drag box if needed.
-      if (this.dragBeginEvent_ && !this.dragBoxVisible_) {
-        this.showDragBox_();
-        this.setDragBoxPosition_(e, e);
-      }
-
       // Update the drag box
       if (this.dragBeginEvent_) {
         this.setDragBoxPosition_(this.dragBeginEvent_, e);
@@ -572,7 +692,7 @@ cr.define('tracing', function() {
         scale = 1 / scale;
       this.zoomBy_(scale);
       e.preventDefault();
-    },
+    }

[James Hawkins, 2011/11/05 17:51:37]

Optional: It's generally safer to leave the trailing comma on the last entry in
the object; if the next dev to code in here doesn't remember to add it, it's a
non-obvious bug to fix.

[nduca, 2011/11/09 22:52:19]

Done.

   };
 
   /**