Bug 147381 – Web Inspector: Convert timeline view classes to ES6 classes

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

[Brian Burg]

,

[Radar WebKit Bug Importer]

<rdar://problem/22040696>

[Brian Burg]

Created attachment 257769 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 257769 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:27
> +WebInspector.TimelineRecordingContentView = class TimelineRecordingContentView extends WebInspector.ContentView

Can you do this and extend WebInspector.ContentView even though that is not an ES6 class yet?!

[Brian Burg]

(In reply to comment #3)
> Comment on attachment 257769 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257769&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:27
> > +WebInspector.TimelineRecordingContentView = class TimelineRecordingContentView extends WebInspector.ContentView
> 
> Can you do this and extend WebInspector.ContentView even though that is not
> an ES6 class yet?!

I was skeptical, but everything seemed to work. Maybe you should apply the patch and sanity check it.

[Brian Burg]

(In reply to comment #3)
> Comment on attachment 257769 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257769&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:27
> > +WebInspector.TimelineRecordingContentView = class TimelineRecordingContentView extends WebInspector.ContentView
> 
> Can you do this and extend WebInspector.ContentView even though that is not
> an ES6 class yet?!

Here's a console interaction to demo this:

> window.A = function() { console.log("A ctor"); }; window.B = class B extends A { }
< class B = $1
> new B
[Log] A ctor
< B {} = $2

[Joseph Pecoraro]

Comment on attachment 257769 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/LayoutTimelineView.js:34
> -    this.navigationSidebarTreeOutline.element.classList.add(WebInspector.NavigationSidebarPanel.HideDisclosureButtonsStyleClassName);
> -    this.navigationSidebarTreeOutline.element.classList.add(WebInspector.LayoutTimelineView.TreeOutlineStyleClassName);
> +        console.assert(timeline.type === WebInspector.TimelineRecord.Type.Layout, timeline);
>  
> -    var columns = {eventType: {}, location: {}, width: {}, height: {}, startTime: {}, totalTime: {}};
> +        this.navigationSidebarTreeOutline.element.classList.add(WebInspector.NavigationSidebarPanel.HideDisclosureButtonsStyleClassName);

I think there is a way to create a git diff ignoring whitespace only changes. It would likely make this much easier to read / see the actual changes!

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js:-43
> -WebInspector.TimelineRecordBar.UnfinishedStyleClassName = "unfinished";

I feel like if the style class is used in multiple places it could stay a variable. But I don't mind inline a few of these.

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:547
>          const timelineHeight = 36;
>          const renderingFramesTimelineHeight = 108;

This is interesting. Until Saam's super recent changes, we couldn't use "const" in classes. Now, we can! There are a couple instances of "const " in these files, but we should be good now.

[Brian Burg]

Committed r187689: <http://trac.webkit.org/changeset/187689>