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

Status: VERIFIED FIXED

(WebExtensions :: Frontend, defect)

Description

[Markus Stange [:mstange]]

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.

Comment 1

[Kris Maglione [:kmag]]

I guess we can keep the hidden popup around after the first time we create it...

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1451058: Save temporary popup pre-load panel for remote popups.

Creating the temporary panel for a pre-load popup can be expensive, especially
for OOP popups, where we need to create a compositor. Since we currently need
to create a pre-load popup every time we hover over a browser action with a
popup, it's probably worth saving and re-using them, rather than always
creating a new one.

Review commit: https://reviewboard.mozilla.org/r/233352/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/233352/

Comment 3

[Shane Caraveo (:mixedpuppy)]

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?

Comment 4

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/29b2567d16eb2d8d58da6e3b616c42eebb0de9f1
Bug 1451058: Save temporary popup pre-load panel for remote popups. r=mixedpuppy

Comment 5

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7047a6811fdc12b5a7f5367f5abc80923c4613cb
Bug 1451058: Follow-up: Fix unhandled rejections. r=bustage

Comment 6

[Andreea Pavel [:apavel]]

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

Comment 7

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4899cdabbf2535c430a041442bc9eb4aa37f38ab
Bug 1451058: Save temporary popup pre-load panel for remote popups. r=mixedpuppy

https://hg.mozilla.org/integration/mozilla-inbound/rev/834489268b1ebf0ce3ded0862c710f16afc93f15
Bug 1451058: Follow-up: Fix some errors that show up during tests. r=me

Comment 8

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a51eff8c27bfe75861ffe0d9c296367114eb9b4e
Bug 1451058: Follow-up: Fix window leak in popup tests. r=bustage

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/4899cdabbf25
https://hg.mozilla.org/mozilla-central/rev/834489268b1e
https://hg.mozilla.org/mozilla-central/rev/a51eff8c27bf

Comment 10

[Victor Carciu]

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?

Comment 11

[Markus Stange [: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.

Comment 12

[Victor Carciu]

I am not able to see any problem on Mac or Windows using Firefox 61, so I will mark the issue as verified.