2039637 - WebExtensions commands API shortcut keys may trigger pageAction popup to open while the extension is already shutting down

Status: RESOLVED FIXED

(WebExtensions :: Frontend, task, P3)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

While we have been investigating the unexpected failure being hit by browser_ext_commands_execute_page_action.js with the trustPanel enabled, see Bug 2033932, we have noticed that the unexpected Error: PageActions: No anchor node for <no action> (raised from browser-pageActions.js here) was being hit due to the second time the test named test_execute_page_action_with_popup triggers the "Alt+Shift+J" keyboard shotcut getting to trigger the _execute_page_action command while the test is about to exit and it is shutting down the extension here, by that time often enough the browserPageAction property of the ext-pageAction extension API class has been already nullified here.

In Bug 2033932 the failure will be consider to only workarounding the issue (by only sending the "popup-opened" runtime message when the popup panel has been fully loaded, which seems to be enough to reduce the chances to hit that race), but we should property fix the underlying issue and so this bugzilla issue is tracking following up with some more investigation and determining the more complete fix for the actual underlying issue.

Comment 1

[Florian Quèze [:florian]]

Attachment: Bug 2039637 - Don't open a page action popup for an extension that has already shut down, r=#extension-reviewers.

browser_ext_pageAction_simple.js fails with "uncaught rejection:
PageActions: No anchor node for <no action>", 1-2 times a week on central.

pageAction's handleClick() creates the PanelPopup, awaits
popup.contentReady and only then anchors the panel to
this.browserPageAction. The test clicks the page action and unloads the
extension as soon as the popup script's message arrives, which can be well
before the popup content reports its size, so handleClick() resumes after
onShutdown() has nulled browserPageAction. panelAnchorNodeForAction(null)
then finds no anchor and throws, and the message says "<no action>"
precisely because there is no action left to name.

Nothing cleans up after that either. BasePopup registers itself with
extension.callOnClose(), so shutdown calls PanelPopup.closePopup(), which
only knows how to close a popup that is already opening: it waits for a
popupshown event that is never coming for a popup nobody has opened, and
the panel keeps its browser, and that browser's refresh driver, for the
lifetime of the window. ViewPopup.closePopup() distinguishes shown,
attached and neither; PanelPopup handles shown only.

So tear the popup down when it has not started opening, resolve
contentReady when a popup is destroyed so that whoever is waiting to show
it can resume, and give up in handleClick() when the popup it created has
been destroyed. With the #urlbar anchor fallback of bug 1378104 in place
the rejection no longer fires, but without this patch the panel of an
extension that has fully shut down is anchored to the urlbar and shown.
Measured with the race forced, by delaying handleClick() 250ms after
contentReady: without the fallback, 5/5 runs fail with the exact CI
message and 0/5 with this patch.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a0b1be3c1555
https://hg.mozilla.org/integration/autoland/rev/1a1ef0cfde76
Don't open a page action popup for an extension that has already shut down, r=extension-reviewers,robwu.

Comment 3

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/1a1ef0cfde76