Bug 154000

Summary: Web Inspector: eliminate the linear and rendering frames TimelineOverview subclasses
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Severity: Normal CC: bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 153034    
Description Flags
[Patch] Proposed Fix
[Patch] Proposed Fix none

Description Matt Baker 2016-02-08 12:12:46 PST
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
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);


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