DevTools: Support popover on timeline overview.

Source/devtools/front_end/components_lazy/FilmStripView.js

Index: Source/devtools/front_end/components_lazy/FilmStripView.js
diff --git a/Source/devtools/front_end/components_lazy/FilmStripView.js b/Source/devtools/front_end/components_lazy/FilmStripView.js
index 46131e4820664dad19458d026bf9055906f2e1d4..6926478841a4d659e2ccc02576a73dce946c6666 100644
--- a/Source/devtools/front_end/components_lazy/FilmStripView.js
+++ b/Source/devtools/front_end/components_lazy/FilmStripView.js
@@ -11,8 +11,9 @@ WebInspector.FilmStripView = function()
     WebInspector.HBox.call(this, true);
     this.registerRequiredCSS("components_lazy/filmStripView.css");
     this.contentElement.classList.add("film-strip-view");
-    this._mode = WebInspector.FilmStripView.Modes.TimeBased;
+    this._framesContainer = this.contentElement.createChild("div", "frames-container");
     this.reset();
+    this.setMode(WebInspector.FilmStripView.Modes.TimeBased);
 }
 
 WebInspector.FilmStripView.Events = {
@@ -33,6 +34,7 @@ WebInspector.FilmStripView.prototype = {
     setMode: function(mode)
     {
         this._mode = mode;
+        this.contentElement.classList.toggle("time-based", mode === WebInspector.FilmStripView.Modes.TimeBased);
         this.update();
     },
 
@@ -54,55 +56,34 @@ WebInspector.FilmStripView.prototype = {
         this.update();
     },
 
-    update: function()
+    /**
+     * @param {number} index
+     * @return {!Element}
+     */
+    frameElementByIndex: function(index)
     {
-        if (!this._model)
-            return;
-        var frames = this._model.frames();
-        if (!frames.length)
-            return;
-        this.contentElement.removeChildren();
-        this._label.remove();
-        var zeroTime = this._zeroTime;
-
-        /**
-         * @param {!WebInspector.FilmStripModel.Frame} frame
-         * @param {boolean=} skipTimeLabel
-         * @return {!Element}
-         * @this {WebInspector.FilmStripView}
-         */
-        function createElementForFrame(frame, skipTimeLabel)
-        {
+        var element = this._elementsCache[index];
+        if (!element) {
+            var frame = this._model.frames()[index];

[caseq, 2015/06/16 14:14:49]

Is this method guaranteed to always be called after setModel()?

[alph, 2015/06/17 09:17:08]

Yes. I can cover it with assert if you want.

             var time = frame.timestamp;
-            var element = createElementWithClass("div", "frame");
+            element = createElementWithClass("div", "frame");
             element.createChild("div", "thumbnail").createChild("img").src = "data:image/jpg;base64," + frame.imageData;
-            if (!skipTimeLabel)
-                element.createChild("div", "time").textContent = Number.millisToString(time - zeroTime);
+            element.createChild("div", "time").textContent = Number.millisToString(time - this._zeroTime);
             element.addEventListener("mousedown", this._onMouseEvent.bind(this, WebInspector.FilmStripView.Events.FrameSelected, time), false);
             element.addEventListener("mouseenter", this._onMouseEvent.bind(this, WebInspector.FilmStripView.Events.FrameEnter, time), false);
             element.addEventListener("mouseout", this._onMouseEvent.bind(this, WebInspector.FilmStripView.Events.FrameExit, time), false);
             element.addEventListener("dblclick", this._onDoubleClick.bind(this, frame), false);
-            this.contentElement.appendChild(element);
-            return element;
-        }
-
-        if (this._mode === WebInspector.FilmStripView.Modes.FrameBased) {
-            for (var frame of frames)
-                createElementForFrame.call(this, frame);
-            return;
-        }
-
-        /**
-         * @return {!Element}
-         * @this {WebInspector.FilmStripView}
-         */
-        function createEmptyElement()
-        {
-            var element = createElementWithClass("div", "frame");
-            this.contentElement.appendChild(element);
-            return element;
+            this._elementsCache[index] = element;
         }
+        return element;
+    },
 
+    /**
+     * @param {number} time
+     * @return {number}
+     */
+    frameIndexByTime: function(time)
+    {
         /**
          * @param {number} time
          * @param {!WebInspector.FilmStripModel.Frame} frame
@@ -112,21 +93,38 @@ WebInspector.FilmStripView.prototype = {
         {
             return time - frame.timestamp;
         }
+        // Using the first frame to fill the interval between recording start
+        // and a moment the frame is taken.
+        return Math.max(this._model.frames().upperBound(time, comparator) - 1, 0);

[caseq, 2015/06/16 14:14:49]

ditto.

[alph, 2015/06/17 09:17:08]

ditto.

+    },
 
-        var width = this.contentElement.clientWidth;
-        var scale = this._spanTime / width;
+    update: function()
+    {
+        if (!this._model)
+            return;
+        var frames = this._model.frames();
+        if (!frames.length)
+            return;
+        this._framesContainer.removeChildren();
+        this._label.remove();

[caseq, 2015/06/16 14:14:49]

nit: please remove label into something more descriptive.

[alph, 2015/06/17 09:17:08]

I actually didn't introduce this name, but ok renamed it to _statusLabel 

 
-        // Calculate frame width basing on the first frame.
-        var tempElement = createElementWithClass("div", "frame");
-        tempElement.createChild("div", "thumbnail").createChild("img").src = "data:image/jpg;base64," + frames[0].imageData;
-        var frameWidth = Math.ceil(WebInspector.measurePreferredSize(tempElement, this.contentElement).width);
+        if (this._mode === WebInspector.FilmStripView.Modes.FrameBased) {
+            for (var i = 0; i < frames.length; ++i)
+                this._framesContainer.appendChild(this.frameElementByIndex(i).cloneNode(true));

[caseq, 2015/06/16 14:14:49]

So what's the purpose of the cache if we're cloning all the nodes?

[alph, 2015/06/17 09:17:08]

Popover uses cached versions directly.

+            return;
+        }
+
+        var width = this._framesContainer.clientWidth;
+        var scale = this._spanTime / width;
+        var element0 = this.frameElementByIndex(0);  // Calculate frame width basing on the first frame.
+        var frameWidth = Math.ceil(WebInspector.measurePreferredSize(element0, this.contentElement).width);
         if (!frameWidth)
             return;
 
         for (var pos = frameWidth; pos < width; pos += frameWidth) {
-            var time = pos * scale + zeroTime;
-            var index = frames.upperBound(time, comparator) - 1;
-            var element = index >= 0 ? createElementForFrame.call(this, frames[index], true) : createEmptyElement.call(this);
+            var time = pos * scale + this._zeroTime;
+            var index = this.frameIndexByTime(time);
+            var element = this._framesContainer.appendChild(this.frameElementByIndex(index).cloneNode(true));

[caseq, 2015/06/16 14:14:49]

ditto.

[alph, 2015/06/17 09:17:08]

ditto.

             element.style.width = frameWidth + "px";
         }
     },
@@ -161,8 +159,9 @@ WebInspector.FilmStripView.prototype = {
     reset: function()
     {
         this._zeroTime = 0;
-        this.contentElement.removeChildren();
-        this._label = this.contentElement.createChild("div", "label");
+        this._elementsCache = [];
+        this._framesContainer.removeChildren();
+        this._label = this._framesContainer.createChild("div", "label");
         this._label.textContent = WebInspector.UIString("No frames recorded. Reload page to start recording.");
     },