Closed Bug 1358415 Opened 6 years ago Closed 6 years ago

1.08ms uninterruptible reflow at get width@chrome://browser/content/ext-utils.js:528:5


(WebExtensions :: Frontend, defect, P1)

54 Branch


(Performance Impact:?, firefox-esr52 unaffected)

55.4 - May 1
Performance Impact ?
Tracking Status
firefox-esr52 --- unaffected


(Reporter: rjward0, Assigned: kmag)


(Blocks 1 open bug)


(Keywords: regression, Whiteboard: [ohnoreflow][photon-performance])


(1 file)

Here's the stack:

get width@chrome://browser/content/ext-utils.js:528:5
promise callback*create@chrome://browser/content/ext-tabs.js:301:18
Flags: qe-verify?
Priority: -- → P2
There's a call to the clientWidth getter at from

Calling .clientWidth (or clientHeight a few lines above) is very expensive as it triggers a synchronous reflow. Could this either be made lazy (so that it's computed only if the add-on really needs this value), or would it be OK to use stale data from getBoundsWithoutFlushing?
Blocks: 1260548
Component: Untriaged → WebExtensions: General
Product: Firefox → Toolkit
We actually fixed this in bug 1240326, but it got regressed when bug 1260548 landed (since the patch on that was started before the original fix).
Assignee: nobody → kmaglione+bmo
Blocks: 1240326
Component: WebExtensions: General → WebExtensions: Frontend
Keywords: regression
Priority: P2 → P1
Comment on attachment 8860510 [details]
Bug 1358415: Don't trigger reflow just to compute tab geometry.

::: browser/components/extensions/ext-tabs.js:115
(Diff revision 1)
>            };
>          }).api(),
>          onCreated: new SingletonEventManager(context, "tabs.onCreated", fire => {
>            let listener = (eventName, event) => {
> -            fire.async(tabManager.convert(event.nativeTab));
> +            fire.async(tabManager.convert(event.nativeTab, event.currentTab));

do we need the same change for fennec?

::: browser/components/extensions/ext-utils.js:250
(Diff revision 1)
> +
>          // We need to delay sending this event until the next tick, since the
>          // tab does not have its final index when the TabOpen event is dispatched.
>          Promise.resolve().then(() => {
>            if (event.detail.adoptedTab) {
> -            this.emitAttached(event.originalTarget);
> +            this.emitAttached(event.originalTarget, currentTab);

this appears to not be needed/used

::: toolkit/components/extensions/ExtensionTabs.jsm:291
(Diff revision 1)
>    /**
> +   * @property {nsIFrameLoader} browser
> +   *        Returns the frameloader for the given tab.
> +   *        @readonly
> +   *        @abstract

this is not abstract...
Attachment #8860510 - Flags: review?(aswan) → review+
Comment on attachment 8860510 [details]
Bug 1358415: Don't trigger reflow just to compute tab geometry.

> do we need the same change for fennec?

Ideally, but it's less of an issue on fennec, since it doesn't have much XUL UI.

> this is not abstract...

Yeah, I already fixed that.
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Iteration: --- → 55.4 - May 1
Should we consider this for Beta uplift or let it ride the 55 train?
Flags: needinfo?(kmaglione+bmo)
Version: unspecified → 54 Branch
Flags: qe-verify? → qe-verify-
Comment on attachment 8860510 [details]
Bug 1358415: Don't trigger reflow just to compute tab geometry.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1260548
[User impact if declined]: This has the potential to cause responsiveness issues and jank during tab animations.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No 
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low-risk
[Why is the change risky/not risky?]: This is a relatively minor change that restores the logic that we used prior to a refactoring.
[String changes made/needed]: None.
Flags: needinfo?(kmaglione+bmo)
Attachment #8860510 - Flags: approval-mozilla-beta?
Comment on attachment 8860510 [details]
Bug 1358415: Don't trigger reflow just to compute tab geometry.

Fix a regression. Beta54+. Should be in 54 beta 6.
Attachment #8860510 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1374597
Product: Toolkit → WebExtensions
Performance Impact: --- → ?
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][photon-performance]
You need to log in before you can comment on or make changes to this bug.