1602384 - browser.menus.onShown event listener is not firing when used in multiple contexts and one of them unloads

Status: VERIFIED FIXED

(WebExtensions :: Frontend, defect, P2)

Description

[Rob Wu [:robwu]]

Attachment: menus-onShown-forgotten.zip

gOnShownSubscribers in ext-menus.js is used to determine whether to dispatch the menus.onShown event, and is keyed by extension, but the key is deleted from an unregister handler that may have multiple instances per extension (specifically up to one per context).

Because of this, if one context unloads, all other listeners of the extension will cease to receive events.

STR:

1. Load attached extension.
2. Open the global JS console, and check the "Show Content Messages" checkbox.
3. Click on the extension button to open a tab.
4. Right-click anywhere (e.g. in the current tab's content area) to open a context menu.
5. Close the tab from step 3.
6. Repeat step 4.

Expected:

• At step 4, the log should contain: "onShown in background" and "onShown in tab"
• At step 6, the log should contain: "onShown in background"

Actual:

• At step 4, the log should contain: "onShown in background" and "onShown in tab"
• At step 6, the log entry is missing.

Comment 1

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

Attachment: Bug 1602384 - Make sure menu.onShown API event is still emitted after a destroyed extension context. r?robwu!

Comment 2

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

Attachment: Bug 1602384 - Fix browser_ext_menus_incognito.js timeout failure when running locally on linux. r?robwu!

The openContextMenu test helper uses a default selector parameter that doesn't actually exist in the
test page used (which is about:robots) and so the test was getting stuck because the context menu
was not being opened.

This change is unrelated to the issue fixed in this bugzilla issue, but the timeout failure was
preventing to run all the menus test successfully in a local linux build and so it did seem
worth to look into where the test was getting stuck and ensure that it does pass consistently.

Depends on D60493

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9ef5cbf2ff6
Make sure menu.onShown API event is still emitted after a destroyed extension context. r=robwu
https://hg.mozilla.org/integration/autoland/rev/72146fca884a
Fix browser_ext_menus_incognito.js timeout failure when running locally on linux. r=robwu

Comment 4

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/e9ef5cbf2ff6
https://hg.mozilla.org/mozilla-central/rev/72146fca884a

Comment 5

[Miruna Curtean]

Verified fixed on Windows Pro 10 64-bit and macOS Catalina 10.15 on FF 74.0a1 (20200129213721).

Comment 6

[Miruna Curtean]

Attachment: browser.menu.onShown.PNG