Bug 125878 - Web Inspector: add the basics of TimelineOverview
Summary: Web Inspector: add the basics of TimelineOverview
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-12-17 15:46 PST by Timothy Hatcher
Modified: 2014-01-20 19:03 PST (History)
4 users (show)

See Also:


Attachments
Patch (32.76 KB, patch)
2013-12-17 15:53 PST, Timothy Hatcher
joepeck: review+
timothy: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2013-12-17 15:46:31 PST
Add a new TimelineOverview class that has a time ruler and tracks the current recording time. This class will eventually manage the timeline overview graphs and time window selection.
Comment 1 Timothy Hatcher 2013-12-17 15:53:52 PST
Created attachment 219458 [details]
Patch
Comment 2 Joseph Pecoraro 2013-12-17 16:32:00 PST
Comment on attachment 219458 [details]
Patch

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);
Comment 3 Timothy Hatcher 2014-01-20 19:02:41 PST
https://trac.webkit.org/changeset/162408
Comment 4 Radar WebKit Bug Importer 2014-01-20 19:03:14 PST
<rdar://problem/15866319>