Bug 127063 - Web Inspector: Implement the discrete Script and Layout timeline views
Summary: Web Inspector: Implement the discrete Script and Layout timeline views
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: 2014-01-15 13:01 PST by Timothy Hatcher
Modified: 2014-01-20 19:04 PST (History)
4 users (show)

See Also:


Attachments
Patch (54.48 KB, patch)
2014-01-15 13:13 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 2014-01-15 13:01:56 PST
Bring up the Script and Layout timeline views.
Comment 1 Radar WebKit Bug Importer 2014-01-15 13:02:25 PST
<rdar://problem/15827848>
Comment 2 Timothy Hatcher 2014-01-15 13:13:24 PST
Created attachment 221302 [details]
Patch
Comment 3 Joseph Pecoraro 2014-01-17 12:49:07 PST
Comment on attachment 221302 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/LayoutTimelineView.js:120
> +        this._layoutTimeline.addEventListener(WebInspector.Timeline.Event.RecordAdded, this._layoutTimelineRecordAdded, this);

Hmm, I wonder if we should create "RecordsAdded" batched event. Is this likely to get called a lot of times?

> Source/WebInspectorUI/UserInterface/ScriptTimelineDataGridNode.js:80
> +            } else {
> +                console.error("Unknown SourceCode subclass.");
> +            }

Style: braces

> Source/WebInspectorUI/UserInterface/TimelineRecordTreeElement.js:37
> +        // FIXME: This needs to live update the subtitle in response to the WebInspector.SourceCodeLocation.Event.DisplayLocationChanged event.

Doh I see this FIXME was in the old code. I can see this being kind of tricky. Basically we want to update this.subtitleElement, but we first have to call the GeneralTreeElement super constructor, get the element created, then hook into it. It is a little tricky.

> Source/WebInspectorUI/UserInterface/TimelineRecordTreeElement.js:41
> +        if (showFullLocationSubtitle) {
> +            subtitle = this._sourceCodeLocation.displayLocationString(WebInspector.SourceCodeLocation.ColumnStyle.OnlyIfLarge);
> +        } else {

Style: braces

> Source/WebInspectorUI/UserInterface/TimelineRecordTreeElement.js:74
> +            console.error("Unknown TimelineRecord eventType: " + timelineRecord.eventType, timelineRecord);

Nit: Unknown Layout TimelineRecord eventType

> Source/WebInspectorUI/UserInterface/TimelineRecordTreeElement.js:108
> +            console.error("Unknown TimelineRecord eventType: " + timelineRecord.eventType, timelineRecord);

Nit: Unknown Script TimelineRecord eventType

> Source/WebInspectorUI/UserInterface/TimelineSidebarPanel.js:135
> +        // TimelineRecordTreeElement to stay selected when the Resource is represents is showing.

Typo: "Resource is represents" => "Resource it represents"
Comment 4 Timothy Hatcher 2014-01-20 19:04:37 PST
https://trac.webkit.org/changeset/162416