Closed Bug 1630681 Opened 4 months ago Closed 4 months ago

Avoid a sync layout flush in the main thread of the parent process when rendering the Password Manager autocomplete popup

Categories

(Toolkit :: Password Manager, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox76 --- fixed
firefox77 --- fixed

People

(Reporter: bdanforth, Assigned: bdanforth)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [fxperf:p3])

Attachments

(1 file)

As part of rendering the autocomplete popup in the Password Manager on username or password fields, the width of the popup is set based on the width of the corresponding input field in the page or a specified minimum width.

Currently, the desired width is obtained by calling getBoundingClientRect on an ancestor node and then setting the style.width on the target node. This forces a synchronous layout flush taking on the order of 10ms that can likely be avoided, since we already have at least the input field's width from the content process stored in an attribute on another ancestor node.

There may be some display issues to resolve with reading the attribute instead in certain situations like when the input field on the page is using a default width, but hopefully those can be addressed, as this would reduce the time it takes to display the autocomplete popup (Bug 1619498) by a little more than 20% (at least on my machine).

Priority: -- → P1
See Also: → 1595244
Assignee: nobody → bdanforth
Status: NEW → ASSIGNED
Keywords: perf
Whiteboard: [fxperf]

Instead of forcing a sync layout flush every time the autocomplete popup is opened, this forces a style flush only the first time (see diff profile). This results in a 31% performance improvement in the time it takes to show the autocomplete popup on my machine (Bug 1619498; average time across 5 measurements for the first time the popup is shown went from 45 ms down to 31 ms).

Other approaches that were considered to avoid any flushing:

  • windowUtils.getBoundsWithoutFlushing: Unfortunately, getBoundingClientRect couldn't be replaced with windowUtils.getBoundsWithoutFlushing, because the previously computed width of the popup is 0 when it is first shown, due to it starting out hidden (i.e. display: none).
  • promiseDocumentFlushed: This method is async while all of the surrounding autocomplete UI code is sync, so while it's not impossible, it would be difficult to implement a simple fix.
Whiteboard: [fxperf] → [fxperf:p3]
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cf04c0d8cc00
Avoid a sync layout flush in the main thread of the parent process when rendering the Password Manager autocomplete popup r=MattN
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77

Comment on attachment 9141144 [details]
Bug 1630681 - Avoid a sync layout flush in the main thread of the parent process when rendering the Password Manager autocomplete popup r=MattN

Beta/Release Uplift Approval Request

  • User impact if declined: If declined, the time it takes to show the autocomplete popup on login fields will be 20-30% slower for users.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This change modifies which web API is used to set the width of the autocomplete popup (instead of element.getBoundingClientRect, we use window.getComputedStyle), but the final width value is the same as before and is set in the same place in the code as before. In general, the autocomplete popup in the Password Manager has extensive test coverage, and all existing tests are passing.
  • String changes made/needed:
Attachment #9141144 - Flags: approval-mozilla-beta?

Comment on attachment 9141144 [details]
Bug 1630681 - Avoid a sync layout flush in the main thread of the parent process when rendering the Password Manager autocomplete popup r=MattN

Approved for 76.0b7.

Attachment #9141144 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1564074
You need to log in before you can comment on or make changes to this bug.