Bug 125709

Summary: Web Inspector: refactor TimelineDecorations into TimelineRuler
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Timothy Hatcher <timothy>
Severity: Normal CC: graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Description Flags
Patch joepeck: review+, timothy: commit-queue-

Description Timothy Hatcher 2013-12-13 13:35:38 PST
The current TimelineDecorations class needs beefed up to support new use cases in Timeline. Mainly scrollable times that isn't always starting at 0. We also need to support time scales that don't have an end time, but are specified in seconds per pixel.
Comment 1 Timothy Hatcher 2013-12-13 14:06:26 PST
Created attachment 219196 [details]
Comment 2 Joseph Pecoraro 2013-12-13 14:50:27 PST
Comment on attachment 219196 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=219196&action=review


> Source/WebInspectorUI/UserInterface/TimelineRuler.js:46
> +    this._allowsClippedLabels = false;

I would include "this._endTimePinned = false" up here with the list of other member variables that don't get `delete`d.

> Source/WebInspectorUI/UserInterface/TimelineRuler.js:165
> +        return this._secondsPerPixel;

Should this _recalculate before returning a value? Someone could call _needsLayout, but that only updates this member variable after a rAF. So if someone set endTime then checked secondsPerPixel it could be wrong.

> Source/WebInspectorUI/UserInterface/TimelineRuler.js:248
> +            var newLeftPosition = ((dividerTime - this._startTime) / duration);

Style: unnecessary extra parens

> Source/WebInspectorUI/UserInterface/TimelineRuler.js:254
> +                    removeDividerAndSelectNext();
> +                    continue;

Is it necessary to remove divider and select next? Why not just continue, and keep to reuse the current divider? Extra dividers are removed after this loop anyways (lines 284-285).

> Source/WebInspectorUI/UserInterface/TimelineRuler.js:264
> +                    removeDividerAndSelectNext();
> +                    continue;

Comment 3 Timothy Hatcher 2014-01-20 19:02:25 PST
Comment 4 Radar WebKit Bug Importer 2014-01-20 19:02:40 PST