Closed Bug 1567275 Opened 1 year ago Closed 2 months ago

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

Categories

(Firefox :: Search, defect, P3)

defect
Points:
3

Tracking

()

RESOLVED FIXED
82 Branch
Iteration:
82.2 - Sep 7 - Sep 20
Tracking Status
firefox82 --- fixed

People

(Reporter: mconley, Assigned: standard8)

References

Details

Attachments

(1 file)

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.

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

Flags: needinfo?(dharvey)

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

Flags: needinfo?(dharvey)

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

Flags: needinfo?(mconley)
Priority: -- → P3

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

Flags: needinfo?(mconley)
Severity: normal → N/A
Points: --- → 3
Severity: N/A → S4

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.

Assignee: nobody → standard8
Iteration: --- → 82.1 - Aug 24 - Sep 6

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

Iteration: 82.1 - Aug 24 - Sep 6 → 82.2 - Sep 7 - Sep 20
Pushed by mbanner@mozilla.com:
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.
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
You need to log in before you can comment on or make changes to this bug.