contextMenus tests can open a context menu before the parent process has registered the menu item
Categories
(WebExtensions :: Frontend, defect, P3)
Tracking
(firefox156 fixed)
| Tracking | Status | |
|---|---|---|
| firefox156 | --- | fixed |
People
(Reporter: florian, Assigned: florian)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
menus.create in child/ext-menus.js calls callParentAsyncFunction("menusInternal.create") and returns the id without awaiting the reply, and nothing in extension startup waits for that call: BackgroundBuilder.build() awaits only context.listenerPromises, which recvAddListener populates and recvAPICall does not. Parent-side, menusInternal.create awaits #promiseInitialized, which covers opening the extension-store-menus kvstore, before adding the item to gRootItems.
So await extension.startup() can return with the create still in flight, gMenuBuilder.build() finds nothing to insert, and the extension's item is absent from an otherwise correct menu: contextMenu item for page was found - Got +0, expected 1. 3/9584 for browser_ext_browserAction_contextMenu.js and 4/9474 for browser_ext_sidebarAction_contextMenu.js on central.
The fix uses create()'s completion callback as the readiness signal, the only handshake that observes the parent-side registration; sending a message straight after create() races #promiseInitialized exactly as startup() does. browser_ext_pageAction_contextMenu.js has the same shape and has never hit it in CI only because awaiting action-shown buys two extra round trips; with parent-side registration delayed by 3s all three tests fail with the CI message and all three pass with the change.
Latently affected and not fixed here: browser_ext_menus_visible.js and browser_ext_contextMenus_checkboxes.js use a sendMessage("ready") shape that looks like a handshake but does not await the create reply.
|
Assignee | |
Comment 1•29 days ago
|
||
browser.contextMenus.create() is fire-and-forget in the child:
menus.create in child/ext-menus.js calls
callParentAsyncFunction("menusInternal.create") and returns the id without
awaiting the reply. Nothing in extension startup waits for that call
either: BackgroundBuilder.build() awaits only context.listenerPromises,
which ExtensionParent.recvAddListener populates for addListener calls and
recvAPICall does not populate at all. So the test's await
extension.startup() returns with menusInternal.create still in flight.
Parent-side, menusInternal.create awaits #promiseInitialized before adding
the item to gMenuMap/gRootItems, and that promise covers
ExtensionMenus.asyncInitForExtension, which opens the
extension-store-menus kvstore. That is real off-main-thread I/O, so a test
that only awaits startup() can open its context menu while gRootItems is
still empty. gMenuBuilder.build() iterates gRootItems, finds nothing to
insert, and the extension's item is absent from an otherwise correctly
opened menu - "contextMenu item for page was found - Got +0, expected 1",
3/9584 for browser_ext_browserAction_contextMenu.js on central.
So use the create() completion callback to report readiness and await it,
the same handshake browser_ext_contextMenus.js already uses, and the only
one that actually observes the parent-side registration. Sending a message
straight after create() would not: it does not wait for the create reply,
so it races #promiseInitialized just as startup() does.
browser_ext_sidebarAction_contextMenu.js hits the same failure;
browser_ext_pageAction_contextMenu.js has not, because awaiting
"action-shown" costs two extra parent round trips after create(), but that
is incidental slack rather than ordering, so it gets the same handshake.
|
||
Updated•28 days ago
|
Backed out for causing bc failures @browser_ext_url_overrides_newtab.js.
|
||
Comment 5•24 days ago
|
||
| bugherder | ||
Description
•