CCing Jen. This is the remainder of the compat code removal work.

Comment on attachment 533494 [details] [diff] [review] Part 1 (v1): Services.jsm Beautifully formulaic. Two identical nits: > + let logins = Services.logins.findLogins({}, login.hostname, login.formSubmitURL, > login.httpRealm); and > + return Services.logins.QueryInterface(Ci.nsIInterfaceRequestor) > .getInterface(Ci.mozIStorageConnection); Indentation on the second line no longer matches the first line. I will trust that it builds and tests pass: I am trying to avoid being labeled a masochist ;)

Address review comments

Rebase on top of Part 1 (v1.1)

Comment on attachment 533496 [details] [diff] [review] Part 2 (v1): PlacesUtils.jsm + let mRoot = PlacesUtils.bookmarks.createFolder(root, "mobile", -1); + PlacesUtils.annotations.setItemAnnotation(mRoot, MOBILEROOT_ANNO, 1, 0, + PlacesUtils.annotations.EXPIRE_NEVER); + PlacesUtils.annotations.setItemAnnotation(mRoot, EXCLUDEBACKUP_ANNO, 1, 0, + PlacesUtils.annotations.EXPIRE_NEVER); and + PlacesUtils.annotations.setItemAnnotation(itemId, DESCRIPTION_ANNO, val, 0, + PlacesUtils.annotations.EXPIRE_NEVER); and + PlacesUtils.annotations.setItemAnnotation(itemId, SIDEBAR_ANNO, true, 0, + PlacesUtils.annotations.EXPIRE_NEVER); and + PlacesUtils.annotations.setItemAnnotation(itemId, SMART_BOOKMARKS_ANNO, val, 0, + PlacesUtils.annotations.EXPIRE_NEVER); ... indentation. (Our style guide suggests that multi-line params should each get their own line, but I don't think it applies in this case! :D) if (!parent) - parent = Svc.Bookmark.getFolderIdForItem(itemid); - Svc.Bookmark.moveItem(itemid, parent, idx - delta); + parent = PlacesUtils.bookmarks.getFolderIdForItem(itemid); + PlacesUtils.bookmarks.moveItem(itemid, parent, idx - delta); While you're touching this part, can we get {} around the if consequent? Otherwise, bravo! Sidenote: I can't wait for standardization of anonymous function syntax.

Comment on attachment 533551 [details] [diff] [review] Part 3 (v1.1): XPCOMUtils.jsm http://etler.com/clapping-b.gif

Comment on attachment 533562 [details] [diff] [review] Part 4 (v1): FileUtils.jsm http://etler.com/applause.gif

Addressed rnewman's review comments

Part 1: http://hg.mozilla.org/services/services-central/rev/a839caa0357b Part 2: http://hg.mozilla.org/services/services-central/rev/aa2c607672e1 Part 3: http://hg.mozilla.org/services/services-central/rev/94124bd1a596 Part 4: http://hg.mozilla.org/services/services-central/rev/6d296a1726a5 Part 5: http://hg.mozilla.org/services/services-central/rev/713b31e0cba9

One last part: removing some (brain-)dead test helpers. Also make SyncTestingInfrastructure sane(r). Need to rethinking this for the long term, but in the short term this already so much better.

Comment on attachment 534179 [details] [diff] [review] Part 6 (v1): dead test code Looks good to me! Interested to see all greens when you push this :D

Part 6: https://hg.mozilla.org/services/services-central/rev/2c2e81d7fc04

Re https://hg.mozilla.org/services/services-central/rev/2c2e81d7fc04 I believe there's a missing case here: case "ProfD": return ds.get("CurProcD", Ci.nsIFile); Patch arriving momentarily.

Sorry for failing to spot this in review.

Part 7: https://hg.mozilla.org/services/services-central/rev/5d709f5dd2e2

Part 1: http://hg.mozilla.org/mozilla-central/rev/a839caa0357b Part 2: http://hg.mozilla.org/mozilla-central/rev/aa2c607672e1 Part 3: http://hg.mozilla.org/mozilla-central/rev/94124bd1a596 Part 4: http://hg.mozilla.org/mozilla-central/rev/6d296a1726a5 Part 5: http://hg.mozilla.org/mozilla-central/rev/713b31e0cba9 Part 6: http://hg.mozilla.org/mozilla-central/rev/2c2e81d7fc04 Part 7: http://hg.mozilla.org/mozilla-central/rev/5d709f5dd2e2