[webkit-reviews] review granted: [Bug 125878] Web Inspector: add the basics of TimelineOverview : [Attachment 219458] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Dec 17 16:33:54 PST 2013
Joseph Pecoraro <joepeck at webkit.org> has granted Timothy Hatcher
<timothy at apple.com>'s request for review:
Bug 125878: Web Inspector: add the basics of TimelineOverview
https://bugs.webkit.org/show_bug.cgi?id=125878
Attachment 219458: Patch
https://bugs.webkit.org/attachment.cgi?id=219458&action=review
------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=219458&action=review
r=me
> Source/WebInspectorUI/UserInterface/TimelineContentView.js:189
> + var timespanSinceLastUpdate = (timestamp -
this._lastUpdateTimestamp) / 1000 || 0;
console.assert(timestamp > this._lastUpdateTimestamp)?
> Source/WebInspectorUI/UserInterface/TimelineOverview.js:44
> + this.zeroTime = 0;
> + this.secondsPerPixel = 0.0025;
Is this suppose to be this.startTime = 0? There is no access to this.zeroTime
and there is no getter/setter for it.
> Source/WebInspectorUI/UserInterface/TimelineRecording.js:99
> + if (isNaN(this._startTime))
> + this._startTime = record.startTime;
I think this is supposed to be:
isNaN(record.startTime)
> Source/WebInspectorUI/UserInterface/TimelineRuler.js:378
> + this._markerElementMap.forEach(function(markerElement, marker) {
> + var newLeftPosition = (marker.time - this._startTime) /
duration;
> +
> + this._updateLeftPositionOfElement(markerElement,
newLeftPosition, visibleWidth);
> +
> + if (!markerElement.parentNode)
> + this._markersElement.appendChild(markerElement);
> + }.bind(this));
No need for the .bind(). You can use Map.prototype.forEach's second optional
"thisObject" parameter.
this._markerElementMap.forEach(function() {
...
}, this);
More information about the webkit-reviews
mailing list