1312226 - Use async queries to read annos in `PlacesSyncUtils`

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P1)

Description

[Lina Butler [:lina]]

The annotations service doesn't have an async API, and implementing a full one (bug 1295519) would require us to fire observers properly. We can at least avoid blocking the main thread for reads, though.

Comment 1

[Lina Butler [:lina]]

Attachment: Bug 1312226 - Use async queries to read annos in `PlacesSyncUtils`.

The annotations service doesn't have an async API, and reimplementing
inserts and deletes would require us to fire observers properly. We can
avoid blocking the main thread for reads, though.

Review commit: https://reviewboard.mozilla.org/r/87814/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/87814/

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8803659 [details]
Bug 1312226 - Use async queries to read annos in `PlacesSyncUtils`.

https://reviewboard.mozilla.org/r/87814/#review86906

This looks great, although it seems a missed opportunity in terms of that one remaining sync call :)

::: toolkit/components/places/PlacesSyncUtils.jsm:481
(Diff revision 1)
>          orphanGuids[i]} to ${item.syncId}`, ex);
>      }
>      if (isReparented) {
>        // Remove the annotation once we've reparented the item.
> -      PlacesUtils.annotations.removeItemAnnotation(orphanIds[i],
> +      let orphanId = yield PlacesUtils.promiseItemId(orphanGuids[i]);
> +      PlacesUtils.annotations.removeItemAnnotation(orphanId,

removeItemAnnotation seems to have fairly simple SQL - is there a reason this hasn't been moved to be async too? Tweaking the sql to directly use a GUID would even save the promiseItemId

Comment 3

[Mark Hammond [:markh] [:mhammond]]

(In reply to Mark Hammond [:markh] from comment #2)
> removeItemAnnotation seems to have fairly simple SQL - is there a reason
> this hasn't been moved to be async too? Tweaking the sql to directly use a
> GUID would even save the promiseItemId

Oh, you already mentioned that in comment 0.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7ad8590b1e7
Use async queries to read annos in `PlacesSyncUtils`. r=markh

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c7ad8590b1e7