Closed
Bug 1150903
Opened 10 years ago
Closed 10 years ago
about:passwords is too slow during filtering
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: rnewman, Assigned: mfinkle)
References
Details
Attachments
(1 file)
(deleted),
patch
|
liuche
:
review+
|
Details | Diff | Splinter Review |
See also Bug 1147534. This bug is: it reloads the content on every search keypress. And it seems to do it on the main thread. And each load takes six seconds.
Regrettably I'm back to the add-on I was using before: it loads everything into a page, never touching the login manager until it really needs to.
Reporter | ||
Updated•10 years ago
|
Blocks: mobile-about-passwords
Summary: about:passwords is too slow during use → about:passwords is too slow during use, particularly search
Assignee | ||
Comment 1•10 years ago
|
||
As pointed out in comment 0, we try to filter on each keypress/input event, which is not a good idea. We should create a timer to allow for a 200ms (?) pause before kicking off a filter.
Pulling the items from the loginManager is a sync call, no way around that and likely a waste of time to try any service worker shenanigans for a small (less than 1000) list of passwords. Building the DOM is a main thread activity too. The slow loading in bug 1147534 coupled with the "each keypress" behavior are the first things to fix. Then we see where we are with performance.
Next steps would be looking at the | sort | we do when fetching the list from the loginManager, although this is likely not a big deal.
If we need more complexity we could try:
1. filtering the existing DOM list with each new | _filter | call instead of starting with a fresh full list from the loginManager. This could save a lot of time wasted when building the DOM list, but also has the drawback of causing a DOM reflow on each "removal", so don't jump blindly into this.
2. Cache a full, sorted list of logins so we don't always go back to the loginManager until we know the cache might be stale. The cached list can be updated when we get the "passwordmgr-storage-changed" notification.
2.
Assignee | ||
Comment 2•10 years ago
|
||
This patch uses the common pattern used in Mozilla code to provide a "debouncing" timer for filtering calls. We wait for a 500ms (default for XUL search textboxes) pause during typing to kick off a filter.
Assignee: nobody → mark.finkle
Attachment #8588590 -
Flags: review?(liuche)
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #1)
> If we need more complexity we could try:
> 2. Cache a full, sorted list of logins so we don't always go back to the
> loginManager until we know the cache might be stale. The cached list can be
> updated when we get the "passwordmgr-storage-changed" notification.
We already do this.
Comment 4•10 years ago
|
||
Comment on attachment 8588590 [details] [diff] [review]
passwords-delay-filter v0.1
Review of attachment 8588590 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks, this is a good perf win for small effort.
Attachment #8588590 -
Flags: review?(liuche) → review+
Updated•10 years ago
|
Summary: about:passwords is too slow during use, particularly search → about:passwords is too slow during filtering
Comment 5•10 years ago
|
||
I'm updating the bug name to reflect that, and we'll see if fixing bug 1147534 is enough; otherwise, we'll spin off other perf bugs.
Assignee | ||
Comment 6•10 years ago
|
||
Comment 7•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•