Bug 111298 – Web Inspector: allow each profiler panel to have own landing page

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Alexei Filippov]

Refactor profiler landing page to allow different profiler panels to have own landing pages.

[Alexei Filippov]

Created attachment 191185 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 191185 [details]
Patch

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

> Source/WebCore/inspector/front-end/ProfileLauncherView.js:31
> +

Revert this?

> Source/WebCore/inspector/front-end/ProfileLauncherView.js:207
> +    if (WebInspector.experimentsSettings.liveNativeMemoryChart.isEnabled()) {

I think we should instead hide the launcher along with corresponding panel when the profile type is disabled.

> Source/WebCore/inspector/front-end/ProfilesPanel.js:1347
> +    WebInspector.SidebarTreeElement.call(this, "profile-launcher-view-tree-item", WebInspector.UIString("New Profile"), "", null, false);

Why did this change? Are you going to make it a button?

[Alexei Filippov]

Comment on attachment 191185 [details]
Patch

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

>> Source/WebCore/inspector/front-end/ProfileLauncherView.js:31
>> +
> 
> Revert this?

ok

>> Source/WebCore/inspector/front-end/ProfileLauncherView.js:207
>> +    if (WebInspector.experimentsSettings.liveNativeMemoryChart.isEnabled()) {
> 
> I think we should instead hide the launcher along with corresponding panel when the profile type is disabled.

If it is disabled the panel won't be added to the list of available panels.

>> Source/WebCore/inspector/front-end/ProfilesPanel.js:1347
>> +    WebInspector.SidebarTreeElement.call(this, "profile-launcher-view-tree-item", WebInspector.UIString("New Profile"), "", null, false);
> 
> Why did this change? Are you going to make it a button?

No. I think this name is better describes the purpose of the landing page. Why it makes you think of a button? It does have a verb in the name.

[Alexei Filippov]

Created attachment 191260 [details]
Patch

[Alexei Filippov]

(In reply to comment #3)
> No. I think this name is better describes the purpose of the landing page. Why it makes you think of a button? It does have a verb in the name.
It does *not* have a verb in the name.

[Yury Semikhatsky]

(In reply to comment #5)
> (In reply to comment #3)
> > No. I think this name is better describes the purpose of the landing page. Why it makes you think of a button? It does have a verb in the name.
> It does *not* have a verb in the name.

Can you attach a screenshot? As far as I understand it changes the title of profiles sidebar to "New Profile" which doesn't make sense to me unless you turn the title into a button that does the same as the  "o" button in the status bar.

[Alexei Filippov]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > No. I think this name is better describes the purpose of the landing page. Why it makes you think of a button? It does have a verb in the name.
> > It does *not* have a verb in the name.
> 
> Can you attach a screenshot? As far as I understand it changes the title of profiles sidebar to "New Profile" which doesn't make sense to me unless you turn the title into a button that does the same as the  "o" button in the status bar.

It is more like a "New tab" button in chromium.
Anyway I reverted this change as it is not really related to the main patch.

[Alexei Filippov]

Created attachment 191432 [details]
Patch

[WebKit Review Bot]

Comment on attachment 191432 [details]
Patch

Clearing flags on attachment: 191432

Committed r144754: <http://trac.webkit.org/changeset/144754>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.