Bug 968220 (developer-hud)

Add a "Developer HUD" sub-menu in "Developer" to control debug overlays.

RESOLVED FIXED in 1.4 S1 (14feb)

Status

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: janx, Assigned: janx)

Tracking

({dev-doc-needed})

unspecified
1.4 S1 (14feb)
dev-doc-needed
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

Instead of the "Settings > Developer > Show devtools overlay" checkbox, the tracked metrics in the devtools layers should be configurable with prefs, in a "Devtools overlay" submenu.

For example, a developer should be able to activate the errors, warnings and reflow widgets, but leave out the memory tracking widget.

Additionally, it could also be nice to configure which apps are being tracked (e.g. none, just an app with a specific manifest, only certified, or all apps).
Alias: developer-hud
Component: Developer Tools: App Manager → Gaia::Settings
Product: Firefox → Firefox OS
Summary: Make B2G devtools layers' tracked metrics configurable with prefs → Add a "Developer HUD" sub-menu in "Developer" to control debug overlays.
Version: Trunk → unspecified
Created attachment 8372393 [details] [review]
gaia pull request

Gaia patch. Vivien, care to take a look?
Attachment #8372393 - Flags: feedback?(21)
Created attachment 8372394 [details]
developerhud.png

A screenshot of the Developer HUD sub-menu.
Created attachment 8372399 [details]
developer_hud.png

Better screenshot.
Attachment #8372394 - Attachment is obsolete: true
TODO:
- Don't show Grid, FPS and TTL if Developer HUD.
- Gecko-side patch respecting Errors, Warnings and Reflows prefs.
> - Don't show Grid, FPS and TTL if Developer HUD.

... is disabled.
Comment on attachment 8372393 [details] [review]
gaia pull request

Comments on github.
Attachment #8372393 - Flags: feedback?(21) → feedback+
Created attachment 8374622 [details] [diff] [review]
Make B2G devtools overlay metrics configurable with prefs. r=ochameau
Comment on attachment 8374622 [details] [diff] [review]
Make B2G devtools overlay metrics configurable with prefs. r=ochameau

This should do the trick. Alex, could you please have a look?
Attachment #8374622 - Flags: review?(poirot.alex)
Comment on attachment 8372393 [details] [review]
gaia pull request

Thanks a lot for the feedback Vivien! I updated the pull request addressing the nits, killing the grid, and visually showing that FPS and TTL are not yet controlled by the same mechanism as Reflows/Warnings/Errors (disabling the Developer HUD only disables the latter 3 checkboxes).
Attachment #8372393 - Flags: review?(21)
Comment on attachment 8374622 [details] [diff] [review]
Make B2G devtools overlay metrics configurable with prefs. r=ochameau

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

Stealing the review since ochameau is out for a few days.

::: b2g/chrome/content/devtools.js
@@ +208,5 @@
>   */
>  let consoleWatcher = {
>  
>    _apps: new Map(),
> +  _watching: { reflows: false, warnings: false, errors: false },

Would be better for the blame history in the future if those are on multiple lines.

@@ +215,5 @@
>    init: function cw_init(client) {
>      this._client = client;
>      this.consoleListener = this.consoleListener.bind(this);
>  
> +    ['reflows', 'warnings', 'errors'].forEach(metric => {

You can probably iterate on the this._watching map.

@@ +283,3 @@
>          if (pageError.warning || pageError.strict) {
> +
> +          // WARNING

Sounds useless (same for the rest of those comments).

@@ +283,4 @@
>          if (pageError.warning || pageError.strict) {
> +
> +          // WARNING
> +          if (!this.bump(app, 'warnings')) {

The name |bump| seems like a little lie now. Any reason to not have a shouldBump method or anything like that.
Attachment #8374622 - Flags: review?(poirot.alex)
Created attachment 8374781 [details] [diff] [review]
Make B2G devtools overlay metrics configurable with prefs. r=vingtetun
Attachment #8374622 - Attachment is obsolete: true
Comment on attachment 8374622 [details] [diff] [review]
Make B2G devtools overlay metrics configurable with prefs. r=ochameau

Thanks for the review! Nits addressed.

> The name |bump| seems like a little lie now. Any reason to not have a shouldBump method or anything like
> that.

I believe "bump" can also be a synonym for "update", and I see it as a function that can refuse to comply, thus the `if (!) return`.
Attachment #8374622 - Flags: review?(21)
Attachment #8374622 - Flags: review?(21)
Attachment #8374781 - Flags: review?(21)
(In reply to Jan Keromnes [:janx] from comment #12)
> Comment on attachment 8374622 [details] [diff] [review]
> Make B2G devtools overlay metrics configurable with prefs. r=ochameau
> 
> Thanks for the review! Nits addressed.
> 
> > The name |bump| seems like a little lie now. Any reason to not have a shouldBump method or anything like
> > that.
> 
> I believe "bump" can also be a synonym for "update", and I see it as a
> function that can refuse to comply, thus the `if (!) return`.

Could be my international broken english that fails :)
Keywords: checkin-needed
Blocks: 962689
Blocks: 965307
Keywords: dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/9bdbe0677a42
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S1 (14feb)
Blocks: 963499
Blocks: 973188
It looks from the dev-doc-needed that this is the bug to document this.  I'm having trouble figuring out how to get this working on my buri/hamachi device, so I suspect whoever tries to document this will have trouble too without more information.

On trunky gaia (local checkout, engineer install)/gecko (engineer build), even with a "DEVICE_DEBUG=1 make reset-gaia" and reboots after using the GUI, I am unable to get the developer HUD to show up.  Specifically, I never get cool squares in the lower right.  The "Frames per second" checkbox does seem to control the 3 purple FPS-y numbers in the upper left (after reboot), and the "Time to load" checkbox does seem to immediately control the TTL widget's visibility.

Can someone provide details on the steps/requirements to get this working?  Thanks!  needinfo'ing :janx as very likely having gotten this working having implemented it ;)
Flags: needinfo?(janx)
Hi Andrew,

- Currently, the Developer HUD only works for non-certified apps like the Marketplace. This limitation is due to bug 962577. You can work around it by setting the "devtools.debugger.forbid-certified-apps" pref to false.

- However, the "Frames per second" and "Time to load" widgets are special: they currently use a different code path, work on certified and non-certified apps, and disabling the whole Developer HUD doesn't make them go away.

These issues are being worked on, and hopefully things will soon make more sense!
Depends on: 962577
Flags: needinfo?(janx)
(In reply to Jan Keromnes [:janx] from comment #17)
> - Currently, the Developer HUD only works for non-certified apps like the
> Marketplace. This limitation is due to bug 962577. You can work around it by
> setting the "devtools.debugger.forbid-certified-apps" pref to false.
> 
> - However, the "Frames per second" and "Time to load" widgets are special:
> they currently use a different code path, work on certified and
> non-certified apps, and disabling the whole Developer HUD doesn't make them
> go away.

Ah, I think these two things explain exactly what I was seeing!

Things didn't work initially because I had only turned on FPS.  When I did reset-gaia to get things to work, I lost the certified app debugging.  Things are working now that I have done reset-gaia again and put the certified apps pref back in.  Thanks for the clarification!
Component: Gaia::Settings → Developer Tools
You need to log in before you can comment on or make changes to this bug.