Bug 1930984 - Implement default search engine selection on the search engine selector.

[Standard8]

This adds handling of the default search engines from the search configuration.

This changes the object returned by filter_engine_configuration to use an Option for app_default_engine_id as that allows us to highlight there may have been an issue with the configuration - in theory, this should never happen, but just in case, I'd rather the application had potential to be aware of it.

No API/changelog necessary as this isn't currently used.

Pull Request checklist

• Breaking changes: This PR follows our breaking change policy
  • This PR follows the breaking change policy:
    • This PR has no breaking API changes, or
    • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
• Quality: This PR builds and tests run cleanly
  • Note:
    • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
    • If this pull request includes a breaking change, consider cutting a new release after merging.
• Tests: This PR includes thorough tests or an explanation of why it does not
• Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
  • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
• Dependencies: This PR follows our dependency management guidelines
  • Any new dependencies are accompanied by a summary of the due diligence applied in selecting them.

Branch builds: add [firefox-android: branch-name] to the PR title.

[linabutler]

Looks great to me, thank you! Just a couple of idiomatic Rust suggestions, plus switching over to once_cell.

[linabutler]

Terrific, thank you!

[mandysGit]

I understand we're testing the default_engine_id is "engine2" but how does this test make sure global default engine2 is not overridden?

Same comment for the test below.

[mandysGit]

Actually, I'm still not understanding this assertion statement because I don't understand why or how the global engine would ever become overridden?

[Standard8]

We're trying to test that if the specific environment is overriding the private default, then it is not overriding the global default as well.

I've updated the wording for that.

[mandysGit]

Thank you for writing this. Interesting to see how the filtering is done in Rust for default engines.

I was reviewing test_engine_selector_defaults.js#311-321 and wanted to ask if you think we should add a test for matching distro? So we can have the same tests are that file?

[Standard8]

I was reviewing test_engine_selector_defaults.js#311-321 and wanted to ask if you think we should add a test for matching distro? So we can have the same tests are that file?

From a unit testing perspective, I don't think that's necessary. environment_matching.rs has the tests for checking that the environments match in appropriate conditions (e.g. when distributions are specified).

We're using the exact same matches_user_environment for matching the specific defaults. As that's the case, I'm not convinced there's much additional value another test here would add - except for checking that you can indeed match on distribution and someone hasn't re-implemented matches_user_environment separately - which I think would be unlikely.

I do notice, however, that I don't have an integration test involving specificDefaults in the selector, which is what I was intended the tests in that file to be - so I'll add something there.

[mandysGit]

Thank you. Changes look good to me 👍🏼

There's one assertion statement I don't quite understand - I left it as unresolved. It might be I'm misinterpreting it, if you could help me clarify what you mean there?