990872 - Eliminate Places dependency in Sync utils

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Tatiana Meshkova (:tatiana)]

Attachment: Allow to use services/sync without MOZ_PLACES defined

It would be nice to be able include and use some of services/sync components without dependency on MOZ_PLACES.

Comment 1

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

Does this actually allow sync to be used?  It seems both services/sync/modules/engines/bookmarks.js and services/sync/modules/engines/history.js need PlacesUtils and would fail without it?

Comment 2

[Richard Newman [:rnewman]]

Comment on attachment 8400394 [details] [diff] [review]
Allow to use services/sync without MOZ_PLACES defined

Review of attachment 8400394 [details] [diff] [review]:
-----------------------------------------------------------------

I'll upload a better fix in just a moment.

Comment 3

[Richard Newman [:rnewman]]

Attachment: Eliminate Places dependency in Sync utils. r=

This simply moves that code closer to the only place it's used, and eliminates the (largely pointless) test. This makes Weave.Utils independent of Places.

Comment 4

[Richard Newman [:rnewman]]

Comment on attachment 8400406 [details] [diff] [review]
Eliminate Places dependency in Sync utils. r=

Services peer for review.

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8400406 [details] [diff] [review]
Eliminate Places dependency in Sync utils. r=

Review of attachment 8400406 [details] [diff] [review]:
-----------------------------------------------------------------

I also would have accepted a patch that moved this method into PlacesUtils.jsm itself :)

Comment 6

[Richard Newman [:rnewman]]

Pushed with tests fixed to match:

https://hg.mozilla.org/integration/fx-team/rev/94fa2e17d418

Tatiana, I don't understand what relationship this has with Bug 746800, but given that I just landed the patch for this, I'll assume that it doesn't actually depend on that bug.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/94fa2e17d418

Comment 8

[Tatiana Meshkova (:tatiana)]

Thanks a lot!