Every time an add-on's toolbar button is hovered, the button's popup gets recreated

VERIFIED FIXED in Firefox 61

Status

defect
VERIFIED FIXED
a year ago
11 months ago

People

(Reporter: mstange, Assigned: kmag)

Tracking

(Depends on 1 bug, {perf})

Trunk
mozilla61
Dependency tree / graph

Firefox Tracking Flags

(firefox61 verified)

Details

Attachments

(1 attachment)

Profile: https://perfht.ml/2HaVDNW

Steps to reproduce:
 1. Install the Gecko Profiler add-on from https://perf-html.io/
 2. Load a page that animates at 60fps on the compositor, for example attachment 8882831 [details]
 3. Move your mouse back and forth in the toolbar, hovering and unhovering the add-on's toolbar button.

Expected results:
Everything should stay smooth.

Actual results:
Whenever the mouse moves over the toolbar button, the animation skips a few frames.

We seem to be re-creating the menupopup on every hover, including its widget and its layer manager + compositor.
Creating a compositor is rather expensive at the moment.
Assignee

Comment 1

a year ago
I guess we can keep the hidden popup around after the first time we create it...
Assignee

Updated

a year ago
Component: WebExtensions: General → WebExtensions: Frontend
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8964646 [details]
Bug 1451058: Save temporary popup pre-load panel for remote popups.

https://reviewboard.mozilla.org/r/233352/#review239768

::: browser/components/extensions/ExtensionPopups.jsm:432
(Diff revision 1)
> +      return panel;
> +    };
> +
> +    // Create a temporary panel to hold the browser while it pre-loads its
> +    // content. This panel will never be shown, but the browser's docShell will
> +    // be swapped with the browser in the real panel when it's ready.

Add to this comment that the temporary panel is shared for all browser actions.  Also, is leaving it for shutdown going to cause any test issues?
Attachment #8964646 - Flags: review?(mixedpuppy) → review+
Assignee

Comment 4

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/29b2567d16eb2d8d58da6e3b616c42eebb0de9f1
Bug 1451058: Save temporary popup pre-load panel for remote popups. r=mixedpuppy
Backed out for failing browser chrome at browser/components/extensions/test/browser/test-oop-extensions/browser_ext_browserAction_popup.js

Push that caused the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=29b2567d16eb2d8d58da6e3b616c42eebb0de9f1

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=172246617&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/f60ce40b73c519a0d3b0b2a490de9b2475f84ed7
Flags: needinfo?(kmaglione+bmo)
Assignee

Updated

a year ago
Flags: needinfo?(kmaglione+bmo)
Depends on: 1452514

Comment 10

a year ago
I am not able to reproduce this issue in FF 59.02 with Windows10x64(same behavior observed as in Latest Nightly) so I don't exactly understand what was the initial issue and what was fixed. Do you have a specific configuration for reproducing this?
Flags: needinfo?(mstange)
On Windows, popup creation may be fast enough that there's no noticeable stuttering. You could try to reproduce it on Mac instead, which is where I noticed the problem.

I'm not seeing the problem any more on Mac so I can confirm that it is fixed.
Flags: needinfo?(mstange)

Comment 12

a year ago
I am not able to see any problem on Mac or Windows using Firefox 61, so I will mark the issue as verified.
Status: RESOLVED → VERIFIED

Updated

11 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.