search-metadata.json files were created by Firefox up to 44. Bug 1203167 changed the storage format for 45. We are still attempting to import search settings from this old file, which we could stop doing. 2 ESR cycles (45 and 52) happened since that. And the few users upgrading from a 44 or older to a current release will be forced to go through 48 anyway (https://chuttenblog.wordpress.com/2017/02/14/the-most-satisfying-graph/).

This could be folded into part 1, but part 1 was already big enough.

This breaks 25 xpcshell tests of the search service; these tests rely on dropping a test engine in the profile folder before initializing the search service.

> This breaks 25 xpcshell tests of the search service; these tests rely on dropping a test engine in the profile folder before initializing the search service. What is the plan for that?

(In reply to Mike Kaply [:mkaply] from comment #6) > > This breaks 25 xpcshell tests of the search service; these tests rely on dropping a test engine in the profile folder before initializing the search service. > > What is the plan for that? Assuming the other patches I attached here are wanted, I'll need to fixup the tests. Likely use addEngineWithDetails instead.

This fixes the otherwise still relevant tests that were relying on: - us importing automatically engines from .xml files dropped in the searchplugins/ folder of the profile. - directory service constants I'm removing.

These tests are obsolete. I also removed lots of attempts to remove the cache file at the very beginning or very end of an xpcshell test. This was just doing a lot of pointless main thread I/O while running the tests. Each xpcshell test starts with a new process and profile folder anyway.

This looks straightforward to me -- shutting off avenues for installing/using engines that we don't want to be available anymore -- but I'm not super familiar with this code. Most of this is code removal, which makes sense, and I don't see any problems in any of the other changes, not even any nits.

Pushed by florian@queze.net: https://hg.mozilla.org/integration/mozilla-inbound/rev/74f28655378e stop importing old search plugins from <profile>/searchplugins/*.xml when the cache file is missing, r=adw. https://hg.mozilla.org/integration/mozilla-inbound/rev/3beeb64fb0da stop saving the last modification date of directories we loaded engines from, r=adw. https://hg.mozilla.org/integration/mozilla-inbound/rev/e05f84b0effd stop importing old search metadata from search-metadata.json, r=adw. https://hg.mozilla.org/integration/mozilla-inbound/rev/297b54c04fb1 remove support for NS_APP_SEARCH_DIR_LIST and NS_APP_SEARCH_DIR from the directory service, r=adw. https://hg.mozilla.org/integration/mozilla-inbound/rev/12152163c058 fix search service tests relying on dropping .xml files in the profile folder, r=adw. https://hg.mozilla.org/integration/mozilla-inbound/rev/160a2d1e4540 remove tests for search-metadata.json migration, r=adw.

https://hg.mozilla.org/mozilla-central/rev/74f28655378e https://hg.mozilla.org/mozilla-central/rev/3beeb64fb0da https://hg.mozilla.org/mozilla-central/rev/e05f84b0effd https://hg.mozilla.org/mozilla-central/rev/297b54c04fb1 https://hg.mozilla.org/mozilla-central/rev/12152163c058 https://hg.mozilla.org/mozilla-central/rev/160a2d1e4540

Comment on attachment 8928529 [details] [diff] [review] part 1 - stop importing old search plugins from <profile>/searchplugins/*.xml Approval Request Comment [Feature/Bug causing the regression]: not a regression, this is long overdue follow-up cleanup after the changes from bug 1203167. But this is more relevant now that we dropped legacy add-ons in 57, as we want to avoid carrying forward customizations that were done in the pre-WebExtension world. [User impact if declined]: Easier to get search plugins injected against the user's will by dropping files in the user profile. [Is this code covered by automated tests?]: Yes. But it's mostly a code removal, so also removing tests. [Has the fix been verified in Nightly?]: It has baked on Nightly for a couple days without us hearing complaints, but I don't think QA verified. [Needs manual test from QE? If yes, steps to reproduce]: currently being discussed with QA directly [List of other uplifts needed for the feature/fix]: most other patches in this bug. I would like to let "part 4" ride the trains without uplifting it though, to avoid needlessly breaking eg. Thunderbird. [Is the change risky?]: Low risk [Why is the change risky/not risky?]: it's a code removal. [String changes made/needed]: none.

Comment on attachment 8928529 [details] [diff] [review] part 1 - stop importing old search plugins from <profile>/searchplugins/*.xml Let's take that. Sheriff, please don't land part 4.

https://hg.mozilla.org/releases/mozilla-beta/rev/0abc56d5a1b1 https://hg.mozilla.org/releases/mozilla-beta/rev/fe0befa1aa9d https://hg.mozilla.org/releases/mozilla-beta/rev/6ad58e06da8c https://hg.mozilla.org/releases/mozilla-beta/rev/e4307e3d7f1f https://hg.mozilla.org/releases/mozilla-beta/rev/1922cbee6cf4 https://hg.mozilla.org/releases/mozilla-beta/rev/827c49cd8d40

https://hg.mozilla.org/releases/mozilla-release/rev/17e3d0eb9cdf https://hg.mozilla.org/releases/mozilla-release/rev/1f25e81939ea https://hg.mozilla.org/releases/mozilla-release/rev/037f3ba3a7b6 https://hg.mozilla.org/releases/mozilla-release/rev/12f0d3bf63bd https://hg.mozilla.org/releases/mozilla-release/rev/61902e3ade32 https://hg.mozilla.org/releases/mozilla-release/rev/7ea28f7a42fe

Basic QA was indirectly covered on bug 1419941 for the reset search testing. Marking this issue as a qe- after consulting with :florian about what extra QA should be done for this issue in particular.