Record in telemetry how long it takes to display an autocomplete popup
Categories
(Toolkit :: Password Manager, task, P1)
Tracking
()
People
(Reporter: MattN, Assigned: MattN)
References
(Blocks 1 open bug)
Details
(Whiteboard: [passwords:telemetry] [passwords:generation])
User Story
category: form_autocomplete method: show object: logins value: (time in milliseconds to show the autocomplete popup) extra: (See https://searchfox.org/mozilla-central/rev/6cd54550a27e2f6ca0755a25328f769e41e524f4/toolkit/components/telemetry/Events.yaml#301-323 ) Caveats: * The event should only be recorded if the length of the text in the field is 0 or 1 (this is simply to reduce volume of telemetry) as 0 and 1 are the most common and expensive cases. * The event won't be recorded if the user disables the login autocomplete footer (but that's not common or really supported… we should remove the pref) * The event doesn't record if there are no autocomplete results shown since the point of the probe is to measure the time to show the popup.
Attachments
(13 files)
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
3.26 KB,
text/plain
|
tdsmith
:
data-review+
|
Details |
We have had reports of autocomplete being slow for dozens of logins (probably due to decryption time e.g. bug 1564074) and we would like to try using Fathom for determining whether to offer password generation (bug 1595244) so it's useful to understand how long it's taking to display autocomplete results in the wild.
Variables we may want to account for:
- password field or text field (username)
- generation enabled and offered
- empty
searchString
- has a
previousResult
- number of saved logins returned
resultStyles
returnedautocompleteInfo.fieldName
(to compare to previous "new-password" behaviour)
Given that there are so many variables that could affect the timing, I think we should use an event instead of a histogram so we can record these variables more easily in extra_keys
rather than hacking together some composite key e.g. password_1_1_0_1
which will be fragile if we add a variable. Using events also means not being able to use TelemetryStopwatch
, though I think we probably can't use it here anyways due to wanting to record times between different processes (bug 1354167), instead we'll need to use Services.telemetry.msSystemNow()
.
Other things to note:
- We should be careful not to record too many events in one session… maybe only handle searchString of 0 and 1?
- Since we know there are ideas about using Fathom for credit card autofill heuristics too, it would be nice to not limit this to login autocomplete results.
Assignee | ||
Comment 1•5 years ago
|
||
The autocomplete search starts in password manager at https://searchfox.org/mozilla-central/rev/91f6c02fcf4c16f78fdc4417f61f192688294066/toolkit/components/passwordmgr/LoginAutoCompleteResult.jsm#431
and is displayed here: https://searchfox.org/mozilla-central/rev/91f6c02fcf4c16f78fdc4417f61f192688294066/toolkit/actors/AutoCompleteParent.jsm#198,249
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
Assignee | ||
Comment 3•5 years ago
|
||
Depends on D66723
Assignee | ||
Comment 4•5 years ago
|
||
Depends on D66724
Assignee | ||
Comment 5•5 years ago
|
||
The only difference was the conversion to vanilla objects.
Depends on D66725
Assignee | ||
Comment 6•5 years ago
|
||
The file is more than just the LoginAutoCompleteResult class, it handles all login autocompletion.
Depends on D66726
Assignee | ||
Comment 7•5 years ago
|
||
Consolidate login autocomplete code in one file.
Depends on D66727
Assignee | ||
Comment 8•5 years ago
|
||
Depends on D66728
Assignee | ||
Comment 9•5 years ago
|
||
Depends on D66729
Assignee | ||
Comment 10•5 years ago
|
||
Try is green for the cleanup patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2597590a168e0e55e26f0438a2c3f0e75168b9ce
Assignee | ||
Comment 11•5 years ago
|
||
Depends on D66729
Updated•5 years ago
|
Assignee | ||
Comment 12•5 years ago
|
||
Also simplify checkAutoCompleteResults.
Depends on D66887
Assignee | ||
Comment 13•5 years ago
|
||
Also add some logging that seems generally useful.
Depends on D67087
Assignee | ||
Comment 14•5 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/aa78dbe7934f7b7c5ec3de348cb70109fa0e35aa#l8.32 deleted this member but didn't update this reference. This caused <!-- formless password field selector recipe test -->
in test_formless_autofill.html to fail test verification.
Depends on D67088
Assignee | ||
Comment 15•5 years ago
|
||
The actual telemetry recording is in https://phabricator.services.mozilla.com/D66730#change-F6M6PHCi3D7P
Assignee | ||
Comment 16•5 years ago
|
||
Assignee | ||
Comment 17•5 years ago
|
||
I'm landing all but D66730 (since data-review is pending) to unblock other work.
Comment 18•5 years ago
|
||
Comment 19•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f89d7b0e4a76
https://hg.mozilla.org/mozilla-central/rev/4ba98b434622
https://hg.mozilla.org/mozilla-central/rev/02dca53e816a
https://hg.mozilla.org/mozilla-central/rev/07a87d64d4d8
https://hg.mozilla.org/mozilla-central/rev/6e5ce5c2ad45
https://hg.mozilla.org/mozilla-central/rev/b6536f155576
https://hg.mozilla.org/mozilla-central/rev/de9538b840e9
https://hg.mozilla.org/mozilla-central/rev/1c8f108909cb
https://hg.mozilla.org/mozilla-central/rev/b3503cdeb434
https://hg.mozilla.org/mozilla-central/rev/c47897bb4792
https://hg.mozilla.org/mozilla-central/rev/18edd0bc728a
Assignee | ||
Updated•5 years ago
|
Comment 20•5 years ago
|
||
Sorry for the delay; where does this value come from? Can it contain arbitrary values from the page?
acFieldName: The "field name" token (last one) of the field's autocomplete attribute.
Assignee | ||
Comment 21•5 years ago
•
|
||
(In reply to Tim Smith 👨🔬 [:tdsmith] from comment #20)
Sorry for the delay; where does this value come from? Can it contain arbitrary values from the page?
acFieldName: The "field name" token (last one) of the field's autocomplete attribute.
It comes from the HTML standard and we only return valid values so it can't contain arbitrary values. These are the valid values: https://searchfox.org/mozilla-central/rev/61f224ec08ddc6f9a93ac45c8c3c5f7159be7c2a/dom/base/AutocompleteFieldList.h#91-147 (we actually only support a subset of those)
Comment 22•5 years ago
|
||
Comment 23•5 years ago
|
||
Assignee | ||
Comment 24•5 years ago
|
||
[Tracking Requested - why for this release]: Need to establish a performance baseline for bug 1595244 to know whether we can ship it in Fx76
Timea, is there any chance this can get verified soon so we can try uplift it to beta so we have a baseline for bug 1595244?
Comment 25•5 years ago
|
||
bugherder |
Comment 26•5 years ago
|
||
Verified-fixed on MacOS 10.13 and Ubuntu 16.04 on the latest Nightly 76.0a1 (2020-13-19).
The "form_autocomplete" "show" "logins" "time" telemetry is recorded.
The autocomplete dropdown is toggled for the Username/Mail or the Password field. Checked with 10, 20 and 1000 login entries.
Nightly updates on Windows are not working due to signing issues for now so can't verify on that. Will mark FX76 as verified after checking it on Windows too.
Submitted Bug 1623657 as a regression, more info there.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 27•5 years ago
|
||
(In reply to Timea Cernea [:tbabos] from comment #26)
Submitted Bug 1623657 as a regression, more info there.
Btw. I believe this isn't a problem because we record stringLength
in the event.
Assignee | ||
Comment 28•5 years ago
|
||
Comment on attachment 9133108 [details]
Bug 1619498 - Record in telemetry how long it takes to display an autocomplete popup. r=sfoster
Beta/Release Uplift Approval Request
- User impact if declined: Need to establish a performance baseline for bug 1595244 to know whether we can ship it in Fx76. Not having this data could mean that the major improvement to password generation will have to be delayed and we are trying to market 4 different password manager projects for Fx76.
This will also allow us to potentially prioritize bug 1564074 sooner as we will know how well login autocomplete performs in aggregate in the wild. - Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: Already done.
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The first 7 commits are smallish cleanups/refactorings to make the telemetry recording easier and more efficient. They are low risk given our test coverage.
The 8th fixes a regression from bug 1616356 which is straightforward as it just inverts the order of two checks. This is trivial.
The 9th and 10th are test-only fixes to avoid intermittent failures with this series. These have no product risk.
The 11th is the core patch that address the telemetry event recording and includes automated tests. Initial data in STMO seems to be valid with no unusual records. e.g. all time measurements are >0. It's pretty straightforward code as it passes data along with the autocomplete results in a way we already use so that it can be recorded when the popup is shown. We have a PI request for this feature so we could ask QA to do additional autocomplete smoke testing if wanted. - String changes made/needed: None
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 29•5 years ago
|
||
Verified on Windows 10 as well, waiting for uplift to Beta. Aside from this verification, we will also test telemetry on top sites as part of the PI request for FX76.
Comment 30•5 years ago
|
||
Comment on attachment 9133108 [details]
Bug 1619498 - Record in telemetry how long it takes to display an autocomplete popup. r=sfoster
thanks for the details in the request, very helpful.
Approved for 75.0b7
Updated•5 years ago
|
Comment 31•5 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/5d8eb3ea412e
https://hg.mozilla.org/releases/mozilla-beta/rev/c86f50cde1af
https://hg.mozilla.org/releases/mozilla-beta/rev/0a2a25c150af
https://hg.mozilla.org/releases/mozilla-beta/rev/78c29ec2230a
https://hg.mozilla.org/releases/mozilla-beta/rev/a59600f166e6
https://hg.mozilla.org/releases/mozilla-beta/rev/bb04263dc872
https://hg.mozilla.org/releases/mozilla-beta/rev/c7d18c20f9a5
https://hg.mozilla.org/releases/mozilla-beta/rev/91d222a72d3f
https://hg.mozilla.org/releases/mozilla-beta/rev/d123344fef1e
https://hg.mozilla.org/releases/mozilla-beta/rev/8ec15d9b0d49
https://hg.mozilla.org/releases/mozilla-beta/rev/b211ac31f030
https://hg.mozilla.org/releases/mozilla-beta/rev/1a8c76b92633
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 32•5 years ago
|
||
Verified-fixed on latest Beta 75.0b7 on Windows 10 x64, Ubuntu 16.04 and MacOS 10.13.
Description
•