Most devtools have shortcuts.

I propose we have one for the profiler popup icon, which would be useful when working on local builds that regularly use clean profiles.

The obvious choice would be Ctrl+Shift+1, if not used elsewhere. I guess we'd need to know if the legacy add-on is there, so we don't steal its shortcut.

Bonus points (possibly a separate bug) : Automatically enable the icon if profiler is already started (e.g., for startup profiling).

After discussing with the team:

• Yet-another shortcut is probably not good, especially as a very small proportion of our users will use the Profiler, and the rest may be confused with the strange icon appearing.
• A useful compromise would be to just enable the popup icon in local builds, and maybe also in Nightly builds.

In the meantime the icon can be programmatically enabled with the "devtools.performance.popup.feature-flag" pref, which can be set with ./mach run --setpref "devtools.performance.popup.enabled=true"

(In reply to Gerald Squelart [:gerald] from comment #1)

...
In the meantime the icon can be programmatically enabled with the "devtools.performance.popup.feature-flag" pref, which can be set with ./mach run --setpref "devtools.performance.popup.enabled=true"

This doesn't seem to work on new profiles (until I manually enable it once). 😪

Ah is it possible that this doesn't work on the prefs that aren't defined in all.js? This pref is defined in code only :/

Maybe Andrew would know about this.

The --setpref option works by writing a user.js file in the profile. So all.js shouldn't affect anything on the tooling side, but maybe it has implications with Firefox that I'm not aware of. On first run (when the pref doesn't work), open the profile and inspect user.js to verify if the pref is in there or not.

Also reading the mach run source, looks like --setpref is only supported if you don't pass in -P/--profile.

(In reply to Gerald Squelart [:gerald] from comment #1)

After discussing with the team:

• Yet-another shortcut is probably not good, especially as a very small proportion of our users will use the Profiler, and the rest may be confused with the strange icon appearing.
• A useful compromise would be to just enable the popup icon in local builds, and maybe also in Nightly builds.

We do have some 'local development only' shortcuts (like ctrl+alt+r for doing a restart plus session restore): https://searchfox.org/mozilla-central/source/browser/base/content/browser-development-helpers.js. I think it'd be totally fine to add global shortcut for the profile either directly in browser-development-helpers, or guarded behind !AppConstants.MOZILLA_OFFICIAL in the profile code somewhere, as per https://searchfox.org/mozilla-central/rev/23f836a71cfe961373c8bd0d0219ec60a64b3c8f/browser/base/content/browser.xhtml#95-97.

A useful compromise would be to just enable the popup icon in local builds, and maybe also in Nightly builds.

I like that solution as well. Nightly might also be fine, which is our prime performance contributor base (at least testing it out and gathering feedback).

We touched on this a few times during planning and I didn't hear concerns. Lets enable this for local builds first and then see if devs would also like it for Nightly.

Related, bug 1385452 landed the restart button for local builds.

This patch will turn on the profiler button for all local builds.

Depends on D66407

I mentioned this work to mattn. He pointed out that adding icons for builds has to be done carefully to avoid changing the toolbar for when tests run. He recommended to tie it into mach run.

Yeah, we've had test failures in the past due to toolbar overflow and UITour availableTargets. The toolbar item could also skew local Talos results.

browser-development-helpers.js is what I was thinking of but that also seems to use AppConstants.MOZILLA_OFFICIAL. It looks like the proposed patch also aligns with how we handle debugging prefs. Maybe we don't actually do any additional changes for mach run anymore? bgrins would know.

I guess as long as tests pass in automation and people don't complain too much about local test failures due to different screen resolutions causing toolbar overflow then you're probably fine.

Yeah, I may hook into mach run. I'm holding off on landing this patch for now.

Here's where that would happen:
https://searchfox.org/mozilla-central/rev/f36cb2af46edd2659f446b7acdb2154e230ee445/python/mozbuild/mozbuild/mach_commands.py#1002-1146