833495 - Should have a pref to enable optional chrome gecko profiling

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Should have a pref that defaults to false, but when true allows the user to choose whether they want to show chrome stuff in the profiler.

Basically the same way that you can flip the pref to get the option to run the scratchpad in the chrome environment.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

From IRC:

past: the profiler could just respect devtools.chrome.enabled like the rest of our tools

Comment 2

[Rob Campbell [:rc] (:robcee)]

I think I'd prefer to have a piece of UI to control those. I think devtools.chrome.enabled is too greedy, i.e., we'll show too much when a user doesn't necessarily want to see it.

We show results from chrome js now. I think we should change this to allow the profiler to show data from gecko platform instead -- disable JS-only flag.

Comment 3

[Anton Kovalyov (:anton)]

Attachment: Add an option to show Gecko platform data in the Profiler

This patch depends on a patch from bug 828038, make sure to apply that one first. I added a gear button just like in the debugger. Right now it has only one option (this one) but in future we will probably add more.

Comment 4

[Anton Kovalyov (:anton)]

Attachment: Add an option to show Gecko platform data in the Profiler

Added this nice blue glow just like in the debugger.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

We will be moving the debugger options to the options panel in bug 879408, so maybe you could be a little avant-garde and put profiler options there right away :-)

Comment 6

[Anton Kovalyov (:anton)]

(In reply to Panos Astithas [:past] from comment #5)
> We will be moving the debugger options to the options panel in bug 879408,
> so maybe you could be a little avant-garde and put profiler options there
> right away :-)

Makes sense, will do.

Comment 7

[Anton Kovalyov (:anton)]

Attachment: Add an option to show Gecko platform data in the Profiler

Moved the option into a shared Options tab per Panos' suggestion.

Comment 8

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 770376 [details] [diff] [review]
Add an option to show Gecko platform data in the Profiler

Review of attachment 770376 [details] [diff] [review]:
-----------------------------------------------------------------

looks fine.

::: browser/app/profile/firefox.js
@@ +1092,5 @@
>  // Enable the Profiler
>  pref("devtools.profiler.enabled", true);
>  
> +// The default Profiler UI settings
> +pref("devtools.profiler.ui.show-platform-data", false);

not the same as "chrome-enabled".

::: browser/devtools/profiler/cleopatra/js/devtools.js
@@ +14,5 @@
> +
> +  query.split("&").forEach(function (pair) {
> +    pair = pair.split("=");
> +    gParsedQS[decodeURIComponent(pair[0])] = decodeURIComponent(pair[1]);
> +  });

cute. But I wouldn't call it beautiful.

Comment 9

[Anton Kovalyov (:anton)]

(In reply to Rob Campbell [:rc] (:robcee) from comment #8)
> Comment on attachment 770376 [details] [diff] [review]
> Add an option to show Gecko platform data in the Profiler
> 
> Review of attachment 770376 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> looks fine.
> 
> ::: browser/app/profile/firefox.js
> @@ +1092,5 @@
> >  // Enable the Profiler
> >  pref("devtools.profiler.enabled", true);
> >  
> > +// The default Profiler UI settings
> > +pref("devtools.profiler.ui.show-platform-data", false);
> 
> not the same as "chrome-enabled".

That's intentional. We already display chrome JavaScript data so using that flag would be misleading.

Comment 10

[Anton Kovalyov (:anton)]

https://hg.mozilla.org/integration/fx-team/rev/7a3cbd26d3d0

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7a3cbd26d3d0