Bug 140737 - Web Inspector: Improve the UI of the type profiler popover
Summary: Web Inspector: Improve the UI of the type profiler popover
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: Joseph Pecoraro
URL:
Keywords: InRadar
: 143053 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-01-21 12:33 PST by Timothy Hatcher
Modified: 2022-03-01 02:47 PST (History)
9 users (show)

See Also:


Attachments
[PATCH] Take 1 (62.80 KB, patch)
2015-05-07 13:35 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff
[IMAGE] Before (166.58 KB, image/png)
2015-05-07 13:36 PDT, Joseph Pecoraro
no flags Details
[IMAGE] After (159.88 KB, image/png)
2015-05-07 13:36 PDT, Joseph Pecoraro
no flags Details
[IMAGE] Console (live object) (79.66 KB, image/png)
2015-05-07 13:37 PDT, Joseph Pecoraro
no flags Details
[PATCH] For Landing (61.71 KB, patch)
2015-05-20 19:03 PDT, Joseph Pecoraro
commit-queue: 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 2015-01-21 12:33:26 PST
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.
Comment 1 Radar WebKit Bug Importer 2015-01-21 12:33:39 PST
<rdar://problem/19552352>
Comment 2 Joseph Pecoraro 2015-05-06 15:20:23 PDT
I'll look into this.
Comment 3 Saam Barati 2015-05-06 17:22:07 PDT
(In reply to comment #2)
> I'll look into this.

Thanks Joe!
This is long overdue.
Comment 4 Joseph Pecoraro 2015-05-07 13:35:14 PDT
Created attachment 252616 [details]
[PATCH] Take 1
Comment 5 Joseph Pecoraro 2015-05-07 13:36:10 PDT
Created attachment 252617 [details]
[IMAGE] Before
Comment 6 Joseph Pecoraro 2015-05-07 13:36:32 PDT
Created attachment 252618 [details]
[IMAGE] After
Comment 7 Joseph Pecoraro 2015-05-07 13:37:03 PDT
Created attachment 252619 [details]
[IMAGE] Console (live object)
Comment 8 Timothy Hatcher 2015-05-08 10:52:41 PDT
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?
Comment 9 Saam Barati 2015-05-13 16:43:24 PDT
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?
Comment 10 Joseph Pecoraro 2015-05-13 17:16:18 PDT
(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?
Comment 11 Saam Barati 2015-05-13 17:20:14 PDT
(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.
Comment 12 Timothy Hatcher 2015-05-13 17:53:38 PDT
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.
Comment 13 Joseph Pecoraro 2015-05-20 19:03:59 PDT
Created attachment 253493 [details]
[PATCH] For Landing

Addressed review comments. Yes, there was left-over debugging code in here.
Comment 14 Joseph Pecoraro 2015-05-20 19:06:04 PDT
*** Bug 143053 has been marked as a duplicate of this bug. ***
Comment 15 WebKit Commit Bot 2015-05-20 19:07:56 PDT
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
Comment 16 Joseph Pecoraro 2015-05-21 16:34:15 PDT
https://bugs.webkit.org/show_bug.cgi?id=140737