We should check if someone is doing startup profiling, and automatically enable the profiler button, that way they can easily capture it on new user profiles.

This is somewhat similar to Bug 1562082.

Also, ensure that it is added to the actual view, so the button is visible. Currently, just setting the pref only adds it to the "customize" menu.

(In reply to Greg Tatum [:gregtatum] from comment #1)

Also, ensure that it is added to the actual view, so the button is visible. Currently, just setting the pref only adds it to the "customize" menu.

Gijs, is there an existing way to conditionally add an item to the default toolbar set based on a pref and/or in a local build only? If I run MOZ_PROFILER_STARTUP=1 ./mach run --temp-profile --setpref devtools.performance.popup.enabled=true then the profiler icon is still inside of Customize Mode (which is both extra steps to find and also generates a lot of profile noise to get into it, drag the icon out, etc). Ideally in this case I'd like the icon to show up next to the library-button or similar by default.

(In reply to Brian Grinstead [:bgrins] from comment #2)

(In reply to Greg Tatum [:gregtatum] from comment #1)

Also, ensure that it is added to the actual view, so the button is visible. Currently, just setting the pref only adds it to the "customize" menu.

Gijs, is there an existing way to conditionally add an item to the default toolbar set based on a pref and/or in a local build only? If I run MOZ_PROFILER_STARTUP=1 ./mach run --temp-profile --setpref devtools.performance.popup.enabled=true then the profiler icon is still inside of Customize Mode (which is both extra steps to find and also generates a lot of profile noise to get into it, drag the icon out, etc). Ideally in this case I'd like the icon to show up next to the library-button or similar by default.

The defaults are currently fixed per-build. They vary based on compile-time defines (specifically, devedition builds add a devtools button) but are otherwise fixed. Off-hand, I suspect changing them based on a pref would be tricky.

I don't really understand the context for this bug because I don't know what the "profiler button" is, or what the longer-term plan for it is, and if the pref is temporary or supposed to be used with a mach command or something else.

I think you have at least these options:

• You could change the defaults based on a build-time constant for local builds, but this means that if you build with specific branding you might not get the button.
• You can at runtime just add the icon using CustomizableUI.addWidgetToArea("some-button", "nav-bar", indexIntoNavbarPlacements), where the index can be determined by finding library-button in the rv of CUI.getWidgetIdsInArea("nav-bar"). Note that if you do this, the defaults are not changed so (a) if the user resets the toolbar, it will disappear and (b) this will make it impossible for users to indicate they don't want this button appearing every startup.

There may be other more appropriate options but without more context it's hard to say what they are and/or if/which of the above is best. Does that help? If not, can you expand on what the bigger picture is?

(In reply to :Gijs (he/him) from comment #3)

(In reply to Brian Grinstead [:bgrins] from comment #2)

(In reply to Greg Tatum [:gregtatum] from comment #1)

Also, ensure that it is added to the actual view, so the button is visible. Currently, just setting the pref only adds it to the "customize" menu.

Gijs, is there an existing way to conditionally add an item to the default toolbar set based on a pref and/or in a local build only? If I run MOZ_PROFILER_STARTUP=1 ./mach run --temp-profile --setpref devtools.performance.popup.enabled=true then the profiler icon is still inside of Customize Mode (which is both extra steps to find and also generates a lot of profile noise to get into it, drag the icon out, etc). Ideally in this case I'd like the icon to show up next to the library-button or similar by default.
There may be other more appropriate options but without more context it's hard to say what they are and/or if/which of the above is best. Does that help? If not, can you expand on what the bigger picture is?

Greg's probably a better person to give the bigger picture, so I'll forward to him. But, this is about adding a button similar to what the Gecko Profiler addon does (in order to start/stop/capture profiles) but without having to install an addon. I was thinking this might be something we want to unconditionally enable in local builds to reduce friction for capturing profiles in development. But maybe it should still be guarded by a pref (since having it in the toolbar by default may mess up mochitests for toolbar/customize mode). Regardless, when the button is enabled it should appear in the toolbar by default IMO.

Note that there's some overlap between this bug and Bug 1570357 - it's possible they should be duped, depending on how solve this.

Gjis: I think this is the job of the profiler popup to handle this use case, so I wouldn't worry about this .

bgrins: would it be enough to show the profile popup if the MOZ_PROFILER_STARTUP=1 is enabled? Otherwise I would probably just create a separate environment variable, e.g. MOZ_PROFILER_BUTTON=1. That pref is more to say "has the popup been turned on by the user?", and shouldn't control its placement. By default when enabled by the UI, it gets added to the main area.

(In reply to Greg Tatum [:gregtatum] from comment #5)

Gjis: I think this is the job of the profiler popup to handle this use case, so I wouldn't worry about this .

bgrins: would it be enough to show the profile popup if the MOZ_PROFILER_STARTUP=1 is enabled? Otherwise I would probably just create a separate environment variable, e.g. MOZ_PROFILER_BUTTON=1. That pref is more to say "has the popup been turned on by the user?", and shouldn't control its placement. By default when enabled by the UI, it gets added to the main area.

Oh, I see now if I go and click in the UI to enable it then it gets added to the toolbar - I didn't realize that. Showing it based on MOZ_PROFILER_STARTUP would solve my use case, yes. If it's not a pain to do, it'd be nice if that also flipped the pref for me so I didn't also have to pass --setpref devtools.performance.popup.enabled=true.

FWIW, it would seem sensible to add the button by default on nightly. We could potentially do something like:

if (AppConstants.NIGHTLY_BUILD && !Cu.isInAutomation) {
// add button into the defaults
}

which wouldn't change tests but would make sure people can easily access the button on nightly / local builds.

If the button is in the defaults and the button does not exist, it should just get ignored in terms of "restore to defaults" etc.

You could go the "manually call addWidgetToArea" route for other builds if you detect the pref is flipped, though in that case it should flip a separate pref to ensure we only do this once (so that if the user manually removes the button in customize mode, we don't keep adding it back.)

FWIW, it would seem sensible to add the button by default on nightly

We've discussed this in our team. It'd be worth getting consensus to do this by default.

(In reply to Greg Tatum [:gregtatum] from comment #8)

FWIW, it would seem sensible to add the button by default on nightly

We've discussed this in our team. It'd be worth getting consensus to do this by default.

I can r+ a patch to do this, the CUI part is very simple - you'd want to add

if (AppConstants.NIGHTLY_BUILD && !Cu.isInAutomation) {
  navbarPlacements.splice(7, 0, "identifier-of-profiler-button");
}

after https://searchfox.org/mozilla-central/rev/35cc00a25c4471993fdaa5761952bd3afd4f1731/browser/components/customizableui/CustomizableUI.jsm#234-236

However, that wouldn't add the button to existing profiles. The easiest way to do that is if the button is in the builtin list in CustomizableWidgets (there's some machinery to auto-add things) - it looks to me like it's not; where does it live? If it's somehow painful to move it we can do something custom, but that might be more difficult...

There's also the question of, if this is currently behind a hidden pref, what (else) that pref controls and if we're happy to turn that on for everyone - I don't really have an opinion on that part. :-)

I can r+ a patch to do this, the CUI part is very simple - you'd want to add

Thanks! I think we may want to do this when we release our new profiler recording UI on Nightly. It would be a good addition I think. I'll reach out once I think we are ready for it.

where does it live?

I took some time to limit the performance impact of this button, and it hooks into the DevToolsStartup process for its initialization: https://searchfox.org/mozilla-central/rev/2f29d53865cb895bf16c91336cc575aecd996a17/devtools/startup/DevToolsStartup.jsm

The UI for the popup is shared with the DevTools panel, but the Gecko Profiler interface is not powered by the remote debugging protocol.
https://searchfox.org/mozilla-central/rev/2f29d53865cb895bf16c91336cc575aecd996a17/devtools/client/performance-new/popup/README.md

There's also the question of, if this is currently behind a hidden pref, what (else) that pref controls and if we're happy to turn that on for everyone - I don't really have an opinion on that part. :-)

It enables the shortcuts for the profiler. Also we will probably need to finish up some improvements to the popup before we'd be ready to turn it on by default. I'm currently working on improving the test coverage for this area. I have a meta bug for this work, but I haven't sorted it into "blocking" things yet. This also opens up the profiler front-end to more users, which we have a few blockers there that we are currently working on.

Greg, is this still valid bug? It feels more related to the old DevTools Performance panel (now replaced by the new Firefox profiler) and devtools.performance.popup.enabled doesn't exist anymore.
Should we close it?

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #11)

Greg, is this still valid bug?

Yes, it is still valid. MOZ_PROFILER_STARTUP controls the gecko profiler.