1610293 - maybeReloadEngines should not rebuild the entire engine list, only handle new/update/remove of engines

Status: RESOLVED FIXED

(Firefox :: Search, task, P3)

Description

[Mark Banner (:standard8)]

maybeReloadEngines currently assumes that the entire engine list is going to change. That is not usually the case, and ends up with doing more work than necessary, including writing to the cache.

Ideally we should only add or remove engines from the existing list. For the modern configuration we also need to remember to handle engine updates where the configuration has actually changed.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1610293 - Split out delay region notification test to separate test file, so that test_reload_engines can concentrate on testing the reload engines function. r?daleharvey!

Depends on D82942

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1610293 - Rewrite maybeReloadEngines to not completely reload the search engines. r?daleharvey!

Depends on D83123

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 1610293 - When reloading engines, ensure useDBForOrder isn't set, and handle more cases of updating engines. r?daleharvey!

Depends on D83124

Comment 4

[Mark Banner (:standard8)]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7daf7a3426c589dbc71bf7b8a6e6dcd347cabc15&selectedTaskRun=b0G3laJVR6u_wDQAwUK4mg.0

Comment 5

[Mark Banner (:standard8)]

Also note: I intentionally haven't read from the cache for maybeReloadEngines in the new version. Existing engines already have the cached information stored within them, and the engine's settings don't overwrite that. Hence, we really don't need to read the cache here.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db308be7d25e
Split out delay region notification test to separate test file, so that test_reload_engines can concentrate on testing the reload engines function. r=daleharvey
https://hg.mozilla.org/integration/autoland/rev/9c01746f3b05
Rewrite maybeReloadEngines to not completely reload the search engines. r=daleharvey
https://hg.mozilla.org/integration/autoland/rev/c4388ff019db
When reloading engines, ensure useDBForOrder isn't set, and handle more cases of updating engines. r=daleharvey

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/db308be7d25e
https://hg.mozilla.org/mozilla-central/rev/9c01746f3b05
https://hg.mozilla.org/mozilla-central/rev/c4388ff019db