1559530 - Remove or refactor reInit

Status: RESOLVED FIXED

(Firefox :: Search, task, P3)

Description

[Shane Caraveo (:mixedpuppy)]

In bug 1556789 I fixed search.init to return the actual promise that is resolved/rejected during initialization. This fixed the ability to actually use then/catch on the promise.

reInit is a funky thing, but it can be fixed. Basically the primary item that is introduces is flushing the cache file. It then replicates search.reset and search.init.

reInit should become an async, returning a Promise in the IDL. It should then simply flush the cache, call reset, then return Init. This will reduce code duplication and make it possible to get rid of a bunch of observer code used to detect when reinit is done.

Comment 1

[Shane Caraveo (:mixedpuppy)]

I'd actually consider adding a flush param to reset and have reset flush the cache. Then tests should call reset and init.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:daleharvey, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Mark Banner (:standard8)]

With the work currently ongoing to tidy up the search service, we'd are thinking about either removing or refactoring reInit. It has crept into production code, but really it shouldn't be necessary - we should be able to handle changes of configuration correctly, even if it is a big change.

If a test really needs to clean the search service out, then we should reconsider what we're actually testing, and why, and see if there's an alternative - if we really need to, then create a new search service object.

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1559530 - Remove test_require_engines_in_cache.js as it is testing the same as test_json_cache_broken.js. r?daleharvey!

test_json_cache_broken.js is a nicer test that doesn't need reInit and tests more, so remove the older test_require_engines_in_cache.js.

Depends on D88274

Comment 5

[Mark Banner (:standard8)]

Attachment: Bug 1559530 - When changing the separatePrivateDefault preference, also reset the sort order. r?daleharvey!

Depends on D88275

Comment 6

[Mark Banner (:standard8)]

Attachment: Bug 1559530 - Remove nsISearchService.reInit. r?daleharvey!

Depends on D88276

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38832c1185b8
Remove test_require_engines_in_cache.js as it is testing the same as test_json_cache_broken.js. r=daleharvey
https://hg.mozilla.org/integration/autoland/rev/82a8305543e5
When changing the separatePrivateDefault preference, also reset the sort order. r=daleharvey
https://hg.mozilla.org/integration/autoland/rev/84ab4021d403
Remove nsISearchService.reInit. r=daleharvey

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48c95b737c1b
Remove test_require_engines_in_cache.js as it is testing the same as test_json_cache_broken.js. r=daleharvey
https://hg.mozilla.org/integration/autoland/rev/36fdfab5156a
When changing the separatePrivateDefault preference, also reset the sort order. r=daleharvey
https://hg.mozilla.org/integration/autoland/rev/7101ff70c61f
Remove nsISearchService.reInit. r=daleharvey

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/48c95b737c1b
https://hg.mozilla.org/mozilla-central/rev/36fdfab5156a
https://hg.mozilla.org/mozilla-central/rev/7101ff70c61f