Bug 154000 - Web Inspector: eliminate the linear and rendering frames TimelineOverview subclasses
Summary: Web Inspector: eliminate the linear and rendering frames TimelineOverview sub...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks: 153034
  Show dependency treegraph
 
Reported: 2016-02-08 12:12 PST by Matt Baker
Modified: 2016-02-15 15:57 PST (History)
8 users (show)

See Also:


Attachments
[Patch] Proposed Fix (72.52 KB, patch)
2016-02-15 13:46 PST, Matt Baker
no flags Details | Formatted Diff | Diff
[Patch] Proposed Fix (72.54 KB, patch)
2016-02-15 15:06 PST, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2016-02-08 12:12:46 PST
* SUMMARY
Eliminate the linear and rendering frames TimelineOverview subclasses. TimelineRecordingContentView maintains two separate overviews, and switches between them depending on the view mode (Timelines vs Rendering Frames). This seems overly complicated, since the two subclasses only differ in the settings they provide (selection, zoom level, ruler snapping, etc). TimelineOverview should encapsulate the ViewMode, rather than the sidebar, and maintain settings for each mode.

This change is motivated in part by https://bugs.webkit.org/show_bug.cgi?id=153034. Moving the timelines tree outline into the overview is made more awkward by the existence of two separate overview classes.
Comment 1 Radar WebKit Bug Importer 2016-02-08 12:13:56 PST
<rdar://problem/24553105>
Comment 2 Matt Baker 2016-02-15 13:46:39 PST
Created attachment 271369 [details]
[Patch] Proposed Fix
Comment 3 Timothy Hatcher 2016-02-15 14:07:36 PST
Comment on attachment 271369 [details]
[Patch] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:34
> +        this._timelinesSettings = this._createViewModeSettings(WebInspector.TimelineOverview.ViewMode.Timelines, 0.0001, 60, 0.01, 0, 15);

_timelinesViewModeSettings would read better than two plural nouns.

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:39
> +            this._renderingFramesSettings = this._createViewModeSettings(WebInspector.TimelineOverview.ViewMode.RenderingFrames, minimumDurationPerPixel, maximumDurationPerPixel, minimumDurationPerPixel, 0, 100);

_renderingFramesViewModeSettings

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:653
> +        else
> +            timelineViewMode = WebInspector.TimelineOverview.ViewMode.Timelines;

Could make this the default assignment for the let, and just keep the if without an else.
Comment 4 Matt Baker 2016-02-15 15:06:13 PST
Created attachment 271377 [details]
[Patch] Proposed Fix
Comment 5 WebKit Commit Bot 2016-02-15 15:57:26 PST
Comment on attachment 271377 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 271377

Committed r196606: <http://trac.webkit.org/changeset/196606>
Comment 6 WebKit Commit Bot 2016-02-15 15:57:30 PST
All reviewed patches have been landed.  Closing bug.