1567275 - Context menu tests fail complaining that "Something tried to use the search service before it's been properly intialized" when run in isolation

Status: RESOLVED FIXED

(Firefox :: Search, defect, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

I believe the content context menu has some stuff in it that touches the Search Service when it's been opened. When running some of the context menu tests in isolation, the test fails with the "Something tried to use the search service before it's been properly intialized" error with the following stack:

 0:27.30 GECKO(21803) _ensureInitialized@resource://gre/modules/SearchService.jsm:620:15
 0:27.30 GECKO(21803) get defaultEngine@resource://gre/modules/SearchService.jsm:2436:10
 0:27.30 GECKO(21803) formatSearchContextItem@chrome://browser/content/nsContextMenu.js:1971:22
 0:27.30 GECKO(21803) CM_initMiscItems@chrome://browser/content/nsContextMenu.js:709:12
 0:27.30 GECKO(21803) CM_initItems@chrome://browser/content/nsContextMenu.js:382:10
 0:27.30 GECKO(21803) CM_initMenu@chrome://browser/content/nsContextMenu.js:231:10
 0:27.30 GECKO(21803) nsContextMenu@chrome://browser/content/nsContextMenu.js:146:8
 0:27.30 GECKO(21803) onpopupshowing@chrome://browser/content/browser.xhtml:1:119
 0:27.30 GECKO(21803) openContextMenu@chrome://browser/content/nsContextMenu.js:141:9
 0:27.30 GECKO(21803) receiveMessage@resource:///actors/ContextMenuParent.jsm:35:9

toolkit/components/viewsource/test/browser/browser_contextmenu.js is one such test.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Hey daleharvey, what's the recommended way to deal with this?

Comment 2

[Dale Harvey (:daleharvey)]

So https://searchfox.org/mozilla-central/source/browser/base/content/nsContextMenu.js#1971 should use the async version

await Service.search.getDefault();

Or if more convenient the component can wait for SearchService to init on startup

await Services.search.init();

This code all looks sync so hopefully someone who knows the context menu code a little better knows where to have it wait for the searchservice to initialise

Comment 3

[Mark Banner (:standard8)]

Hmm yes, that does all seem rather sync. I'm not quite sure if we could change how this is initialised. Mike, any ideas?

Comment 4

[Mike Conley (:mconley) (:⚙️)]

It probably would be fine to make openContextMenu here in nsContextMenu async, and to await Search Service initialization there: https://searchfox.org/mozilla-central/rev/29cce9a2684ef64c4f1f996087da8b7545d31f65/browser/base/content/nsContextMenu.js#40

Comment 5

[Mark Banner (:standard8)]

I noticed that the context menu initialisation is triggered from nsContextMenu's constructor. After discussion with Mike on irc, we decided that it would be easiest to skip displaying the search context menu items if the search service is not registered yet. In practice, this is unlikely to be hit, as it would mean starting up Firefox and then right-clicking on something almost straight away with the intent to search - that is probably not a very common case.

Comment 6

[Mark Banner (:standard8)]

Attachment: Bug 1567275 - Don't display the search related context items if the search service is not initialized yet. r?daleharvey!.

This also moves gInitialised into SearchService, as it no longer needs to be separate, and it makes changing it for tests easier.

Depends on D88462

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0aa9ce5a9790
Don't display the search related context items if the search service is not initialized yet. r=daleharvey.

Comment 8

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/0aa9ce5a9790