Bug 125709 – Web Inspector: refactor TimelineDecorations into TimelineRuler

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Timothy Hatcher]

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.

[Timothy Hatcher]

Created attachment 219196 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 219196 [details]
Patch

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

r=me

> 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;

Ditto.

[Timothy Hatcher]

https://trac.webkit.org/changeset/162407

[Radar WebKit Bug Importer]

<rdar://problem/15866316>