1502437 - Hide system addons on the new about:performance page

Status: VERIFIED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[:Harald Kirschner :digitarald]

Most about pages hide system addons by default. About debugging controls this via devtools.aboutdebugging.showSystemAddons, which is default off.

about:performance should follow the same pattern.

Comment 1

[Florian Quèze [:florian]]

(In reply to :Harald Kirschner :digitarald from comment #0)
> Most about pages hide system addons by default. About debugging controls
> this via devtools.aboutdebugging.showSystemAddons, which is default off.
> 
> about:performance should follow the same pattern.

I think the usual reason for hiding them is that they are part of the browser rather than add-ons. But about:performance also shows an item for the browser iteself; should we show them as subitems of the browser item then?

Comment 2

[Florian Quèze [:florian]]

Attachment: Patch

Comment 3

[Florian Quèze [:florian]]

I made this behavior depend on the same pref as bug 1502440. I think we'll want to uplift this patch to beta64, so I refrained from adding a "System add-on" new localizable string.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 9021532 [details] [diff] [review]
Patch

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

::: toolkit/components/aboutperformance/content/aboutPerformance.js
@@ +407,5 @@
> +   * The API to access addons is async, so we cache the list at the
> +   * first access. The list is unlikely to change while the about:performance
> +   * tab is open, so not updating seems fine.
> +   */
> +  _systemAddonIds: null,

I think it'd be better to initialize this to new Set() instead of null and have some other place to trigger the filling of this Set other than _promiseSnapshot.

Otherwise it's not obvious that the line `this._sysAddonsIds.has(addon.id)` won't potentially run before this Set is non-null. (maybe accidentally after a refactor of the code)

Comment 5

[Florian Quèze [:florian]]

Attachment: Patch v2

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/13d1718bccc0
Hide system add-ons in about:performance when browser internals are hidden, r=felipe.

Comment 7

[Florian Quèze [:florian]]

Would make sense to uplift this to 64 at the same time as bug 1502440.

Comment 8

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/13d1718bccc0

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 10

[Florian Quèze [:florian]]

Comment on attachment 9021585 [details] [diff] [review]
Patch v2

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1478831

User impact if declined: System add-ons that the user can't disable will be listed in about:performance, which can be confusing.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: QE is already working on verifying this feature more generally than just this bug. The step that matters here will be to test after setting the toolkit.aboutPerformance.showInternals pref to false when testing on Nightly.

List of other uplifts needed: Bug 1502440

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Straight forward patch.

String changes made/needed: none

Comment 11

[Julien Cristau [:jcristau]]

Brindusa can you verify this on nightly before we uplift?

Comment 12

[Gyula Palko[:gpalko]]

Verified on Nightly 65, that system addons are not shown on about:performance page, if toolkit.aboutPerformance.showInternals is set to false.

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9021585 [details] [diff] [review]
Patch v2

about:performance tweak, approved for 64.0b8

Comment 14

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/02f3ab06c2eb

Comment 15

[laszlo.bialis]

Verified on Beta 64.0b8, the system addons are not shown on about:performance page as toolkit.aboutPerformance.showInternals is set to false by default.