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

Status: RESOLVED FIXED

(Toolkit :: Password Manager, enhancement, P1)

Description

[Bianca Danforth [:bdanforth]]

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).

Comment 1

[Bianca Danforth [:bdanforth]]

Attachment: Bug 1630681 - Avoid a sync layout flush in the main thread of the parent process when rendering the Password Manager autocomplete popup r=MattN

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.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
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

Comment 3

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/cf04c0d8cc00

Comment 4

[Bianca Danforth [:bdanforth]]

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:

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fec0c5441dff