If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Stop listing invalid GMPlugins in AddonManager.getAddonsByTypes




Add-ons Manager
2 years ago
2 years ago


(Reporter: Dexter, Unassigned)



Firefox Tracking Flags

(firefox45 affected)




2 years ago
In TelemetryEnvironment.jsm [0] we gather and report active GMPlugins. We noticed that |AddonManager.getAddonsByTypes| also reports invalid GMPlugins (with plugin.isValid being false).

Does it make sense to return invalid plugins in getAddonsByTypes?

[0] - https://dxr.mozilla.org/mozilla-central/rev/96377bdbcdf3e444a22aeaa677da696243b00d98/toolkit/components/telemetry/TelemetryEnvironment.jsm#624

Comment 1

2 years ago
spohl, what do you think about not returning invalid plugins in getAddonsByTypes?
Flags: needinfo?(spohl.mozilla.bugs)
Chris, do you know the answer to this? It looks like getAddonByID has the same issue, no?
Flags: needinfo?(spohl.mozilla.bugs) → needinfo?(cpearce)
Just so we're clear, what's being asked here is should we not report GMPAddons for which GMPAddon.isValid [1] returns false?

What does it mean for this to return false? That the install hasn't happened yet? So we'll report via telemetry this when on startup the GMPs haven't had a chance to install yet?

We have telemetry to explicitly track why the Adobe GMP fails to install (VIDEO_EME_ADOBE_INSTALL_FAILED_REASON), so I guess it's not useful to track when installation hasn't happened yet.

Or am I missing something?

[1] http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/GMPInstallManager.jsm#354
Flags: needinfo?(cpearce)
So, as far as i can tell, the "isValid" check is there to handle when .version is null.
This means that that GMP is not actually installed or disabled.

Currently it shows up like an active addon/plugin, which can be confusing to users of the addon manager API (e.g. collecting lists active addons for Telemetry data).

Questions here:
* do "invalid" GMPs have to show up via getAddonsByTypes() at all? (is this needed to show them as "installing shortly" in about:addons?)
* if yes, can we at least have them .appDisabled or similar?
You need to log in before you can comment on or make changes to this bug.