699850 - (asyncHistory) [meta] Refactor all synchronous history APIs to use async Storage APIs

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P2)

Description

[Marco Bonardo [:mak]]

All of the history APIs used in browsing and Sync are already async, we should remove and replace all the remainig synchronous APIs. This may require some deep test rewriting, but we can do fake-sync wrappers for that.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

I'll take it, but I'll need some help to get started.

Comment 2

[Marco Bonardo [:mak]]

I think you choose the hardest one, this will have to be splitted in a lot of smaller bugs, but don't want to scare you :)
History changes (in form of visits addition or removal) in the browser UI are already using async APIs, but we have to convert all of nsIBrowserHistory and nsINavHistoryService (there also other interfaces, but those are mostly dependent on these and should be done later).

A first dependent bug may be to deprecate addVisit and addPageWithDetails, that are used in a LOT of tests, to avoid rewriting all the tests we can likely write an addVisit wrapper that uses updatePlaces async API and spin the events loop till we get the callback. We should push people toward updatePlaces API.
Related to this, we should rewrite the importExport service (bug 482911), hsivonen wanted to do that but he's likely really busy.

If we kill any possibility to add visits synchronously, we can likely start converting nsIBrowserHistory the other methods, like removals.

If you prefer starting from a smaller project, Favicons service has to be finished (is partially async) and has less connections to other services.
Bug 672681 is another interesting entry point.

Comment 3

[Marco Bonardo [:mak]]

hm I said the importExport service, but I meant the migrators, whatever uses addPageWithDetails.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

De-assigning myself in case someone wants to take it. Otherwise, I will eventually return to it.

Comment 5

[Marco Bonardo [:mak]]

This is basically a tracking bug, so making it properly that.

Comment 6

[Marco Bonardo [:mak]]

And... 1 is gone.

Comment 7

[Mike Conley (:mconley) (:⚙️) (PTO: May 24 - June 2nd)]

\o/

Comment 8

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

mak, is this something you'd like to note for 62 release?

Comment 9

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Maybe on MDN and not in the release notes. Let me know what you think.

Comment 10

[Marco Bonardo [:mak]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #8)
> mak, is this something you'd like to note for 62 release?

It's mostly an internal detail, the most common operations were already converted in the past, this changed a few remaining APIs that are less often used (but still, when you hit one of these the jank is unprodictable).

I don't think it's facing users so easily to deserve a relnote.