Closed Bug 1640045 Opened 1 year ago Closed 1 year ago

Remove PlacesSearchAutocompleteProvider.parseSubmissionURL and use Services.search.parseSubmissionURL instead, and initialize the search service at some point

Categories

(Firefox :: Address Bar, task, P3)

task
Points:
3

Tracking

()

RESOLVED FIXED
Firefox 79
Iteration:
79.2 - June 15 - June 28
Tracking Status
firefox79 --- fixed

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(1 file)

This is spun out from bug 1398416. Background: In that bug, we call PlacesSearchAutocompleteProvider.parseSubmissionURL from the muxer, and that method depends on PlacesSearchAutocompleteProvider being initialized, but the muxer doesn't call PlacesSearchAutocompleteProvider.ensureReady anywhere. So the muxer try-catches its call and depends on the fact that other parts of the urlbar trigger a PlacesSearchAutocompleteProvider initialization at some (early) point. We could have called ensureReady right before parseSubmissionURL, but we didn't want to make muxer.sort async due to a single one-time initialization.

However, the only thing parseSubmissionURL ultimately depends on is search service initialization, and unlike the other PlacesSearchAutocompleteProvider methods, it's sync and doesn't init the search service itself. Also, there's only one other caller of PlacesSearchAutocompleteProvider.parseSubmissionURL, in UnifiedComplete's search-restyling logic (which is disabled).

There are a couple of actions here:

  • Remove PlacesSearchAutocompleteProvider.parseSubmissionURL and instead call Services.search.parseSubmissionURL directly.
  • Make sure we initialize the search service at some point before the muxer calls Services.search.parseSubmissionURL. There are at least a few options:
    • Introduce an async muxer.init method that the default muxer would use to init the search service.
    • Do it as part of the very first query in the urlbar (e.g., in Query.start or UrlbarProvidersManager.startQuery).
    • The muxer isn't the only consumer of the search service in the urlbar. There's also the other methods of PlacesSearchAutocompleteProvider and the value formatter. So maybe the urlbar should init the service early on, so that each consumer doesn't need to do it. That seems tricky though because we'd still want to do it lazily, and it would still be async, so consumers likely still couldn't avoid awaiting a promise.
  • Replace PlacesSearchAutocompleteProvider with UrlbarSearchUtils.
    • Move the module from toolkit to browser. The only consumers of
      PlacesSearchAutocompleteProvider are urlbar and UnifiedComplete.
    • I'd like to add functions to UrlbarUtils instead, but
      PlacesSearchAutocompleteProvider adds itself as an observer for search
      engine changes, and it keeps some state so that alias lookups are O(1). It
      has an init function to set that up. That's not quite as easy to do if I
      just added some functions to UrlbarUtils, and I think that O(1) lookups of
      aliases are worth keeping (vs. O(number of installed engines)), so I kept a
      separate module, now called UrlbarSearchUtils.
  • Init the search service (via UrlbarSearchUtils) from
    UrlbarProvidersManager.startQuery so that every module involved in querying
    doesn't need to do it.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Iteration: --- → 79.1 - June 1 - June 14
Points: 2 → 3
Iteration: 79.1 - June 1 - June 14 → 79.2 - June 15 - June 28
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/58e905055fe2
Replace PlacesSearchAutocompleteProvider with UrlbarSearchUtils, remove its parseSubmissionURL function, and init the search service on the first query. r=mak
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 79
Flags: qe-verify-
Blocks: 1628015
You need to log in before you can comment on or make changes to this bug.