Bug 140737 – Web Inspector: Improve the UI of the type profiler popover

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

[Timothy Hatcher]

The type profiler currently has a simple text UI in the popover. We should do something more like the object tree and clean things up.

[Radar WebKit Bug Importer]

<rdar://problem/19552352>

[Joseph Pecoraro]

I'll look into this.

[Saam Barati]

(In reply to comment #2)
> I'll look into this.

Thanks Joe!
This is long overdue.

[Joseph Pecoraro]

Created attachment 252616 [details]
[PATCH] Take 1

[Joseph Pecoraro]

Created attachment 252617 [details]
[IMAGE] Before

[Joseph Pecoraro]

Created attachment 252618 [details]
[IMAGE] After

[Joseph Pecoraro]

Created attachment 252619 [details]
[IMAGE] Console (live object)

[Timothy Hatcher]

Comment on attachment 252616 [details]
[PATCH] Take 1

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

> Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:750
> -        scripts[0].requestContent().then(scriptContentAvailable.bind(this));
> +        scripts[0].requestContent().then(scriptContentAvailable.bind(this)).catch(function() { console.error("scriptContentAvailable", arguments); });

Unrelated?

[Saam Barati]

Comment on attachment 252616 [details]
[PATCH] Take 1

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

> Source/WebInspectorUI/UserInterface/Models/TypeDescription.js:49
> +            payload.typeSet = WebInspector.TypeSet.fromPayload(payload.typeSet);

Out of curiosity, why change the payload itself instead of creating new variables?

[Joseph Pecoraro]

(In reply to comment #9)
> Comment on attachment 252616 [details]
> [PATCH] Take 1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=252616&action=review
> 
> > Source/WebInspectorUI/UserInterface/Models/TypeDescription.js:49
> > +            payload.typeSet = WebInspector.TypeSet.fromPayload(payload.typeSet);
> 
> Out of curiosity, why change the payload itself instead of creating new
> variables?

Heh, no particular reason. I just figured I'd update it in place.

Is there an advantage to one way or the other?

[Saam Barati]

(In reply to comment #10)
> (In reply to comment #9)
> > Comment on attachment 252616 [details]
> > [PATCH] Take 1
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=252616&action=review
> > 
> > > Source/WebInspectorUI/UserInterface/Models/TypeDescription.js:49
> > > +            payload.typeSet = WebInspector.TypeSet.fromPayload(payload.typeSet);
> > 
> > Out of curiosity, why change the payload itself instead of creating new
> > variables?
> 
> Heh, no particular reason. I just figured I'd update it in place.
> 
> Is there an advantage to one way or the other?

I was just thinking that it might be confusing in case someone used
the same payload to call fromPayload twice. It just seems like it should
be a pure function, but the second call would fail. It's not a big
deal one way or the other.

[Timothy Hatcher]

Comment on attachment 252616 [details]
[PATCH] Take 1

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

>>>> Source/WebInspectorUI/UserInterface/Models/TypeDescription.js:49
>>>> +            payload.typeSet = WebInspector.TypeSet.fromPayload(payload.typeSet);
>>> 
>>> Out of curiosity, why change the payload itself instead of creating new variables?
>> 
>> Heh, no particular reason. I just figured I'd update it in place.
>> 
>> Is there an advantage to one way or the other?
> 
> I was just thinking that it might be confusing in case someone used
> the same payload to call fromPayload twice. It just seems like it should
> be a pure function, but the second call would fail. It's not a big
> deal one way or the other.

I agree with Saam. I think separate locals would be better for this.

[Joseph Pecoraro]

Created attachment 253493 [details]
[PATCH] For Landing

Addressed review comments. Yes, there was left-over debugging code in here.

[Joseph Pecoraro]

*** Bug 143053 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 253493 [details]
[PATCH] For Landing

Rejecting attachment 253493 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 253493, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
kenView.js
patching file Source/WebInspectorUI/UserInterface/Views/TypeTreeElement.css
patching file Source/WebInspectorUI/UserInterface/Views/TypeTreeElement.js
rm 'Source/WebInspectorUI/UserInterface/Views/PropertiesSection.js'
patching file Source/WebInspectorUI/UserInterface/Views/TypeTreeView.css
patching file Source/WebInspectorUI/UserInterface/Views/TypeTreeView.js

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/4707431519092736

[Joseph Pecoraro]

https://bugs.webkit.org/show_bug.cgi?id=140737