Methods like getPlacesMetaDataFor and the annotations cache in the DownloadHistory module are currently synchronous. It should not be too difficult to change them to have asynchronous signatures, in preparation for making the Places annotations API asynchronous.

This has been partly fixed in bug 1468980, and only getPlacesMetaDataFor remains, right? Nice job in bug 1468980, by the way. Thanks for simplifying the code!

(In reply to :Paolo Amadini from comment #1) > This has been partly fixed in bug 1468980, and only getPlacesMetaDataFor > remains, right? I believe so, yes. I also have a patch ready for getPlacesMetaDataFor, but I'm just completing another patch to go with it before I upload that. > Nice job in bug 1468980, by the way. Thanks for simplifying the code! Thanks, no problem! It certainly seemed clearer that way when working with the newer APIs :-)

Not just getPlacesMetaDataFor unfortunately, gCachedPlacesMetaData is still synchronous and uses getAnnotationsWithName that should go away.

obviously the 2 things can be splitted into separate bugs to keep things simpler.

From discussion with Marco on irc, the idea is to make gCachedPlacesMetaData be initialised asynchronously (which bug 1480049 is now likely to enable), and then we can add an API to history which will fetch all pages with certain annotations, e.g. history.getAnnotatedPages(arrayOfAnnos) return Map("anno1" => [{page, content}, {page, content}]), "anno2" => ....);

So, gCachedPlacesMetaData would be initialized once asynchronously, for example when getList is called, then it would be the only place from which we read the metadata, and any changes would be written synchronously to gCachedPlacesMetaData and also asynchronously to the Places database, for the next session. Correct?

(In reply to :Paolo Amadini from comment #6) > So, gCachedPlacesMetaData would be initialized once asynchronously, for > example when getList is called, then it would be the only place from which > we read the metadata, and any changes would be written synchronously to > gCachedPlacesMetaData and also asynchronously to the Places database, for > the next session. Correct? Yes, that's what I'm heading towards. The obvious slight compromise is that we have to keep annotation details in memory, but that seems easier than the other options.

Depends on D2597

Depends on D4062

Comment on attachment 9003407 [details] Bug 1387446 - Replace getAnnotationsWithName calls in DownloadHistory with new asynchronous PlacesUtils.history.fetchAnnotatedPages. :Paolo Amadini has approved the revision.

Comment on attachment 9003408 [details] Bug 1387446 - Remove nsIAnnotationService.getAnnotationsWithName. Lina Cambridge (she/her) [:lina] has approved the revision.

Comment on attachment 9003407 [details] Bug 1387446 - Replace getAnnotationsWithName calls in DownloadHistory with new asynchronous PlacesUtils.history.fetchAnnotatedPages. Lina Cambridge (she/her) [:lina] has approved the revision.

Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e1734b0648a9 Replace getAnnotationsWithName calls in DownloadHistory with new asynchronous PlacesUtils.history.fetchAnnotatedPages. r=paolo,lina

Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f7c5d8d8be89 Remove nsIAnnotationService.getAnnotationsWithName. r=lina

https://hg.mozilla.org/mozilla-central/rev/e1734b0648a9 https://hg.mozilla.org/mozilla-central/rev/f7c5d8d8be89