1647320 - Make the search cache responsible for listening to notifications of changes, rather than being told about updates

Status: RESOLVED FIXED

(Firefox :: Search, task, P2)

Description

[Mark Banner (:standard8)]

Changing this means we can make the new SearchCache more responsible for itself, by not relying on the search service to tell it when to write.

We can also move to a model whereby we don't write the cache immediately after init/re-init but let it be a delayed write - we have protections for shutdown, and for when it is re-read, so we should be fine there.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1647320 - Make the search cache responsible for listening to notifications of changes. r?daleharvey!.

This patch additionally makes the search cache responsible for listening to notifications rather than being directly told by the search service to write the cache.

It also makes writes after init/reinit/maybeReloadEngines into delayed writes as they don't need to be immediate - the code already ensures we write any pending cache before reading, and that we write it before shutdown. Therefore, it doesn't matter if we wait a second or so.

Depends on D80455

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ebc3ec422ec
Make the search cache responsible for listening to notifications of changes. r=daleharvey.

Comment 3

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on /test_json_cache.js

backout: https://hg.mozilla.org/integration/autoland/rev/0348dd58e21a32de8363b27f3807615c4dbc8a56

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=fdf086cfd0f9db89938e3b6e0452042ba702414e&group_state=expanded&selectedTaskRun=AQrg3rt5QieqDpofCXh17A.0 . Also failed on tier-1 https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307902555&repo=autoland&lineNumber=3123

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307901709&repo=autoland&lineNumber=1612

[task 2020-06-29T18:14:56.806Z] 18:14:56 INFO - TEST-START | xpcshell-legacyconfig.ini:toolkit/components/search/tests/xpcshell/test_json_cache.js
[task 2020-06-29T18:14:58.565Z] 18:14:58 WARNING - TEST-UNEXPECTED-FAIL | xpcshell-legacyconfig.ini:toolkit/components/search/tests/xpcshell/test_json_cache.js | xpcshell return code: 0
[task 2020-06-29T18:14:58.566Z] 18:14:58 INFO - TEST-INFO took 1756ms
[task 2020-06-29T18:14:58.566Z] 18:14:58 INFO - >>>>>>>
[task 2020-06-29T18:14:58.566Z] 18:14:58 INFO - PID 1291 | [1291, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp, line 2924
[task 2020-06-29T18:14:58.566Z] 18:14:58 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2020-06-29T18:14:58.567Z] 18:14:58 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2020-06-29T18:14:58.567Z] 18:14:58 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2020-06-29T18:14:58.567Z] 18:14:58 INFO - running event loop
[task 2020-06-29T18:14:58.567Z] 18:14:58 INFO - xpcshell-legacyconfig.ini:toolkit/components/search/tests/xpcshell/test_json_cache.js | Starting setup
[task 2020-06-29T18:14:58.567Z] 18:14:58 INFO - (xpcshell/head.js) | test setup pending (2)
[task 2020-06-29T18:14:58.568Z] 18:14:58 INFO - (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2020-06-29T18:14:58.568Z] 18:14:58 INFO - (xpcshell/head.js) | test run_next_test 1 pending (2)
[task 2020-06-29T18:14:58.568Z] 18:14:58 INFO - (xpcshell/head.js) | test setup finished (2)
[task 2020-06-29T18:14:58.569Z] 18:14:58 INFO - xpcshell-legacyconfig.ini:toolkit/components/search/tests/xpcshell/test_json_cache.js | Starting test_legacy_cached_engine_properties
[task 2020-06-29T18:14:58.569Z] 18:14:58 INFO - (xpcshell/head.js) | test test_legacy_cached_engine_properties pending (2)
[task 2020-06-29T18:14:58.569Z] 18:14:58 INFO - "init search service"

Comment 4

[Mark Banner (:standard8)]

Looks like this was the result of initing two search services, and not shutting one down (so there were two listeners for the cache). Not quite sure why I wasn't seeing this on Mac, must be timing.

I've now fixed the issues anyway, and try looks good.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7407a44542be
Make the search cache responsible for listening to notifications of changes. r=daleharvey.

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/7407a44542be