1118871 - Add telemetry to count the number of sites in the password manager blocklist

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P1)

Description

[Matthew N. [:MattN]]

We can report the number of origins in the blacklist.

Comment 1

[Matthew N. [:MattN]]

We just need to report the size of the array returned from Services.logins.getAllDisabledHosts();

Comment 2

[Chenxia Liu [:liuche]]

I'll take this one as a first bug, since I haven't done very much Telemetry/toolkit work.

Comment 3

[Chenxia Liu [:liuche]]

Attachment: MozReview Request: bz://1118871/liuche

/r/2871 - Bug 1118871 - Add telemetry to count the number of sites in the password manager blocklist. r=MattN

Pull down this commit:

hg pull review -r 1c589669886fcd032ec61c89739626150f22a2fa

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8553364 [details]
MozReview Request: bz://1118871/liuche

https://reviewboard.mozilla.org/r/2869/#review2127

Pushed with fixes: https://hg.mozilla.org/integration/fx-team/rev/40079e3249d1

::: toolkit/components/telemetry/Histograms.json
(Diff revision 1)
> +    "description": "The number of host sites for which the user has explicitly rejected saving a login"

Nit: s/host /

Nit: I think the description could cause this probe to be confused with the user choosing not to save a login after submitting. How about:
s/a login/logins/.

::: toolkit/components/passwordmgr/nsLoginManager.js
(Diff revision 1)
> +      let numPasswordsBlocklist = Services.telemetry.getHistogramById("PWMGR_NUM_PASSWORDS_BLOCKLIST");

This histogram name doesn't match Histograms.json so this will throw.

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/40079e3249d1

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 8553364 [details]
MozReview Request: bz://1118871/liuche

Approval Request Comment
[Feature/regressing bug #]: Improvements to password manager in 2015 - Bug 1121127
[User impact if declined]: Possibly less ideal decisions based on lack of data.
[Describe test coverage new/current, TreeHerder]: No tests for the telemetry since it's outside the normal code flow so shouldn't break other aspects of password manager if an exception occurs. No problems have been reported on Nightly and data looks good there.
[Risks and why]: Low risk since it's new code only run during the telemetry gathering phase.
[String/UUID change made/needed]: None

Comment 7

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8553364 [details]
MozReview Request: bz://1118871/liuche

I'm not sure about the value of uplifting this patch to Beta at this point in the cycle. (I'll leave it to Sylvestre to make the call on that.) Happy to take this on Aurora.

Aurora+

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ccb525f5d1d3

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8553364 [details]
MozReview Request: bz://1118871/liuche

Too late for beta.

Comment 11

[Chenxia Liu [:liuche]]

Attachment: MozReview Request: Bug 1118871 - Add telemetry to count the number of sites in the password manager blocklist. r=MattN