1406475 - Remove Preferences.sys.mjs usage from Sync

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P3)

Description

[Lina Butler (ex-Mozilla)]

`nsIPrefBranch` grew default value support in bug 1338306, and methods to get and set strings in bug 1345294. We also use `Services.prefs` directly in some Sync functions, so we might as well jettison Preferences.jsm entirely.

Here's how we'd do the conversion:

* In https://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/services/sync/modules/util.js#639, replace `new Preferences(PREFS_BRANCH)` with `Services.prefs.getBranch(PREFS_BRANCH)`.

* Replace all uses of `Svc.Prefs.get(...)` with type-specific getter methods from `nsIPrefBranch` (https://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/modules/libpref/nsIPrefBranch.idl#67,97,140-141,167). This requires auditing the Sync code to determine the type of each pref (you can check https://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/services/sync/services-sync.js for the complete list of built-in Sync prefs to determine their types, or infer the type from the context), and then replacing everything in https://searchfox.org/mozilla-central/search?q=Svc.Prefs.get&case=false&regexp=false&path=%5Eservices%2Fsync with `Svc.Prefs.getIntPref(...)` for string prefs, `Svc.Prefs.getStringPref(...)` for string prefs, and so on.

* Replace all uses of `Svc.Prefs.set(...)` with type-specific setters. Same as above; everything in https://searchfox.org/mozilla-central/search?q=Svc.Prefs.set&case=false&regexp=false&path=%5Eservices%2Fsync. `Svc.Prefs.set("pref", "value")` changes to `Svc.Prefs.setStringPref("pref", "value")`, `Svc.Prefs.set("pref", true)` changes to `Svc.Prefs.setBoolPref("pref", true)`, and so on.

* Replace `Svc.Prefs.reset(...)` with `Svc.Prefs.clearUserPref(...)`.

* Replace `Svc.Prefs.observe(...)` and `Svc.Prefs.ignore(...)` with `Svc.Prefs.addObserver(...)` and `Svc.Prefs.removeObserver(...)`, respectively. The arguments are the same.

* Replace `Svc.Prefs.has(...)` with `Svc.Prefs.prefHasUserValue(...)`.

Then run the tests via `./mach xpcshell-test services/sync`, and make sure everything still passes. This is a fairly mechanical change, but it's more of a "good next bug" than a first bug because it requires swapping out APIs and touching many different files.

Comment 1

[nikshepsvn]

Hey Kit, I would love to take this on and give it my best shot (this is my first open source contribution!). I was reading through the steps you laid out and was confused by what I was supposed to get/ do with info from this (https://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/services/sync/services-sync.js) link.

Comment 2

[Lina Butler (ex-Mozilla)]

Hi nikshepsvn, welcome and thanks for getting involved! I linked to https://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/services/sync/services-sync.js because it might help with figuring out the types of the prefs.

For example, "services.sync.scheduler.fxa.singleDeviceInterval" is an integer, so https://searchfox.org/mozilla-central/rev/1a4a26905f923458679a59a4be1e455ebc53c333/services/sync/modules/policies.js#85 would change to `Svc.Prefs.getIntPref("syncInterval", this.singleDeviceInterval)`, while https://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/services/sync/services-sync.js#36 is a boolean, so https://searchfox.org/mozilla-central/rev/1a4a26905f923458679a59a4be1e455ebc53c333/services/sync/modules/service.js#51 would change to `Svc.Prefs.getBoolPref("engine.addresses.available", false)`.

Most of the types can be inferred from the default values, though, or the context in which they're used.

Comment 3

[nikshepsvn]

Oh that makes so much sense -- thank you for the detailed response Kit. I'm pumped about getting involved and giving back! I'll submit a patch soon! (Also a question about submitting patches, after I make the changes locally all I have to do is "hg export . > my-change.patch" right?)

Comment 4

[Lina Butler (ex-Mozilla)]

(In reply to nikshepsvn from comment #3)
> (Also a question about submitting patches, after I make the changes locally
> all I have to do is "hg export . > my-change.patch" right?)

Yup, that works! A lot of us use MozReview these days; it's a lot easier to push to a real repo instead of exporting and attaching patches, especially if you're planning to work on more bugs. https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html has instructions to get you started.

I think you might need to be set as an assignee before you can push. If you'd like to give MozReview a try, please flag me when your patch is ready, and I'll assign you to the bug. Thanks again!

Comment 5

[Sudheesh Singanamalla (:ShellHacker)]

I would like to take this up and work on it for a while :-) Should all the occurrences here be changed? https://searchfox.org/mozilla-central/search?q=Svc.Prefs.%28set%7Cget%7Creset%7Cobserve%7Chas%29&regexp=true&path=%5Eservices%2Fsync

I am using a regex match for `Svc.Prefs.(set|get|reset|observe|has)`

and after the changes to these, I'd run `./mach xpcshell-test services/sync`
Is this the right thing to do?

Comment 6

[Lina Butler (ex-Mozilla)]

Cool, thanks for giving this a try, Sudheesh! :-) That looks right, yes. It's not quite a straightforward find-and-replace, because `Svc.Prefs.{get, set}` don't mention the pref type: some of the `get` calls might need to be replaced with `getIntPref`, others with `getStringPref`, and so on.

Comment 7

[Lina Butler (ex-Mozilla)]

Are you still planning to work on this, Sudheesh? Totally OK if not.

Comment 8

[Sudheesh Singanamalla (:ShellHacker)]

@Kit, Okay that's great, I'll give it a shot.

Sorry I've been caught up lately and didn't have the time to work on this.

Comment 9

[nikshepsvn]

Is this still open? Would love to get this finished up today since I have a bunch of time :)

Comment 10

[Sudheesh Singanamalla (:ShellHacker)]

@nikshepsvn Sure go ahead with this one. I might take longer to find time to fix this.

Assigning this to you.

Comment 11

[Arshad Kazmi [:arshadkazmi42]]

Can I take this up? If its still available

Comment 12

[Lina Butler (ex-Mozilla)]

(In reply to Arshad Kazmi [:arshadkazmi42] from comment #11)
> Can I take this up?

Please do! :-) I think comment 0 is still accurate, though the exact files and lines might have shifted around a bit. Thanks for taking this up, and please let me know if you'd like help with anything!

Comment 13

[Arshad Kazmi [:arshadkazmi42]]

Cool. Will start working on it

Comment 14

[Arshad Kazmi [:arshadkazmi42]]

Lina,
I started working on this. Tried modifying this

`Svc.Prefs.get`

to this

`Svc.Prefs.getStringPref`

in this file [1]

On running test for the same file

`mach test services/sync/tests/unit/test_bookmark_engine.js`

I am getting this error

`Unexpected exception TypeError: Svc.Prefs.getStringPref is not a function`



[1] https://searchfox.org/mozilla-central/source/services/sync/tests/unit/test_bookmark_engine.js

Comment 15

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

(In reply to Arshad Kazmi [:arshadkazmi42] from comment #14)
> Lina,
> I started working on this. Tried modifying this
> 
> `Svc.Prefs.get`
> 
> to this
> 
> `Svc.Prefs.getStringPref`

You need to use Services.prefs.getStringPref() - and note that Svc.Prefs.get("foo") needs to be changed to Services.prefs.getStringPref("services.sync.foo"), and you need to be careful regarding the expected type (eg, .getBoolPref or .getIntPref will be necessary in some cases)

Comment 16

[Lina Butler (ex-Mozilla)]

(In reply to Mark Hammond [:markh] from comment #15)
> You need to use Services.prefs.getStringPref() - and note that
> Svc.Prefs.get("foo") needs to be changed to
> Services.prefs.getStringPref("services.sync.foo")

You can also change https://searchfox.org/mozilla-central/rev/a7f4d3ba4fbfe3efbde832869f1d672fce7122f6/services/sync/modules/util.js#774 to `Services.prefs.getBranch(PREFS_BRANCH)`; that'll let you omit the "services.sync." prefix. :-)

Comment 17

[Arshad Kazmi [:arshadkazmi42]]

I tried both the things mentioned in Comment 15 and Comment 16.

I am getting this error


`
ERROR NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getStringPref]
SyncTelemetryImpl@resource://services-sync/telemetry.js
`

It's giving me an error in this file [1]



[1] https://searchfox.org/mozilla-central/source/services/sync/modules/telemetry.js

Comment 18

[Arshad Kazmi [:arshadkazmi42]]

I also tried using getIntPref. But it gives same error

Comment 19

[Lina Butler (ex-Mozilla)]

Mind attaching a diff with what you have so far? (You can use `hg diff --git`, then attach it from https://bugzilla.mozilla.org/attachment.cgi?bugid=1406475).

Comment 20

[Arshad Kazmi [:arshadkazmi42]]

Attachment: diff.txt

Comment 21

[Arshad Kazmi [:arshadkazmi42]]

I have done changes in few files and getting the above error on running this test

services/sync/tests/unit/test_bookmark_engine.js

I have attached diff content in a txt file

Comment 22

[Lina Butler (ex-Mozilla)]

Comment on attachment 9022460 [details] [diff] [review]
diff.txt

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

This looks like a great start, Arshad, thanks so much for working on this! 🎉🎉 I know it's a chore to update all the callers in so many files, but this is really helpful!

I think the issues you're seeing are:

* Using `Services.prefs` instead of `Svc.Prefs`. `Services.prefs` _could_ work, but then you'd need to add `services.sync` to all the pref names, and that gets repetitive pretty quickly. Using `Svc.Prefs` lets you omit the prefix and keep the names as they are.
* Most prefs need a default value. In general, something like 0, "", or false is a safe choice. Some, like `addonutils`, are exceptions, but I can help you with those.
* `Svc.Prefs.has` needs to change to `Svc.Prefs.prefHasUserValue`.
* `Svc.Prefs.set` needs the same treatment as `get`: `setIntPref`, `setStringPref`, `setBoolPref`, and so on.
* `Svc.Prefs.reset` needs to change to `Svc.Prefs.clearUserPref`.

Does that make sense? Thanks again for taking this, and feel free to needinfo? me if you have more questions! :-)

::: services/sync/modules/addonutils.js
@@ +17,3 @@
>  function AddonUtilsInternal() {
>    this._log = Log.repository.getLogger("Sync.AddonUtils");
> +  this._log.Level = Log.Level[Services.prefs.getStringPref("og.logger.addonutils")];

Oops, this had a typo before. :-/ Should be `this._log.level` (lowercase "l"); also, `log`, not `og`, and needs a default value in case the pref isn't set.

Also, this needs to be `Svc.Prefs`, not `Services.prefs`. `Svc.Prefs` is the pref branch for Sync; `Services.prefs` is the global pref branch. It's confusing that the two are subtly different; sorry this is so crufty. :-/

So let's change this to `this._log.level = Log.Level[Svc.Prefs.getStringPref("log.logger.addonutils", "Error")]`.

::: services/sync/modules/browserid_identity.js
@@ +87,3 @@
>        throw new Error(`invalid state ${status}`);
>      }
> +    let when = Services.prefs.getIntPref(this.PREFS.REJECTED_AT);

Needs a default value: `Svc.Prefs.getIntPref(this.PREFS.REJECTED_AT, 0)`.

@@ +90,2 @@
>      let howLong = when ? this.nowInMinutes() - when : 0; // minutes.
> +    let isNewState = Services.prefs.getIntPref(this.PREFS.LAST_RECORDED_STATE) != status;

States are strings (https://searchfox.org/mozilla-central/rev/efc0d9172cb6a5849c6c4fc0f19d7fd5a2da9643/services/sync/modules/browserid_identity.js#57-61), so this needs to be `getStringPref`, and also needs a default value.

`Svc.Prefs.getStringPref(this.PREFS.LAST_RECORDED_STATE, "")` should do the trick.

You'll also need to replace https://searchfox.org/mozilla-central/rev/efc0d9172cb6a5849c6c4fc0f19d7fd5a2da9643/services/sync/modules/browserid_identity.js#97,117 with `setStringPref`...

And `reset` (in https://searchfox.org/mozilla-central/rev/efc0d9172cb6a5849c6c4fc0f19d7fd5a2da9643/services/sync/modules/browserid_identity.js#108-109,316) with `clearUserPref`.

@@ +101,4 @@
>        if (when) {
>          // If we are "permanently rejected" we've already recorded for how
>          // long, so don't do it again.
> +        if (!Services.prefs.getIntPref(this.PREFS.APPEARS_PERMANENTLY_REJECTED)) {

`Svc.Prefs`, and `getBoolPref` with a default value of false, since it's a Boolean.

And `setBoolPref` in https://searchfox.org/mozilla-central/rev/efc0d9172cb6a5849c6c4fc0f19d7fd5a2da9643/services/sync/modules/browserid_identity.js#123.

@@ +109,4 @@
>        Svc.Prefs.reset(this.PREFS.APPEARS_PERMANENTLY_REJECTED);
>      } else {
>        // We are in a failure state.
> +      if (Services.prefs.getIntPref(this.PREFS.APPEARS_PERMANENTLY_REJECTED)) {

getBoolPref

@@ +248,4 @@
>        // If we have, then we are probably just reauthenticating so it's a normal sync.
>        // We can use any pref that must be set if we've synced before, and check
>        // the sync lock state because we might already be doing that first sync.
> +      let isFirstSync = !Weave.Service.locked && !Services.prefs.getStringPref("client.syncID", null);

Svc.Prefs, not Services.prefs, here and elsewhere.

@@ +429,4 @@
>      // We used to support services.sync.tokenServerURI but this was a
>      // pain-point for people using non-default servers as Sync may auto-reset
>      // all services.sync prefs. So if that still exists, it wins.
> +    let url = Services.prefs.getStringPref("tokenServerURI"); // Svc.Prefs "root" is services.sync

Needs a default. `getStringPref("tokenServerURI", "")`.

::: services/sync/modules/doctor.js
@@ +108,1 @@
>        Svc.Prefs.set(prefPrefix + "validation.lastTime", Math.floor(nowSeconds));

setIntPref

@@ +241,2 @@
>    },
>    set lastRepairAdvance(value) {

setIntPref in this function. Ditto for the others.

::: services/sync/modules/engines/tabs.js
@@ +131,3 @@
>    },
>    async getAllTabs(filter) {
> +    let filteredUrls = new RegExp(Services.prefs.getStringPref("engine.tabs.filteredUrls"), "i");

Svc.Prefs and needs a default.

::: services/sync/modules/policies.js
@@ +33,4 @@
>  // from themselves (and us from them!) the minimum time they can specify
>  // is 60s.
>  function getThrottledIntervalPreference(prefName) {
> +  return Math.max(Services.prefs.getIntPref(prefName), 60) * 1000;

Svc.Prefs

@@ +345,4 @@
>        case "weave:service:ready":
>          // Applications can specify this preference if they want autoconnect
>          // to happen after a fixed delay.
> +        let delay = Services.prefs.getStringPref("autoconnectDelay");

getIntPref with default of 0.

@@ +864,3 @@
>          this.service.clusterURL = null;
>          let delay = 0;
> +        if (Services.prefs.getStringPref("lastSyncReassigned")) {

`lastSyncReassigned` is a Boolean, so `getBoolPref`.

::: services/sync/modules/service.js
@@ +361,3 @@
>      // We allow a pref, which has no default value, to limit the engines
>      // which are registered. We expect only tests will use this.
>      if (Svc.Prefs.has("registerEngines")) {

I think this needs to be `Svc.Prefs.prefHasUserValue`. Ditto for everywhere else we use `Svc.Prefs.has`.

@@ +361,4 @@
>      // We allow a pref, which has no default value, to limit the engines
>      // which are registered. We expect only tests will use this.
>      if (Svc.Prefs.has("registerEngines")) {
> +      engines = Services.prefs.getStringPref("registerEngines").split(",");

Svc.Prefs, default value ("").

@@ +369,3 @@
>      }
>      let declined = [];
> +    let pref = Services.prefs.getStringPref("declinedEngines");

Ditto.

@@ +1092,4 @@
>      } else if ((this.status.login == MASTER_PASSWORD_LOCKED) &&
>               Utils.mpLocked()) {
>        reason = kSyncMasterPasswordLocked;
> +    } else if (Services.prefs.getStringPref("firstSync") == "notReady") {

Ditto.

::: services/sync/modules/stages/enginesync.js
@@ +93,4 @@
>      // a first sync.
>      let allowEnginesHint = false;
>      // Wipe data in the desired direction if necessary
> +    switch (Services.prefs.getStringPref("firstSync")) {

Ditto.

::: services/sync/modules/telemetry.js
@@ +472,5 @@
>      this.events = [];
>      this.histograms = {};
> +    this.maxEventsCount = Services.prefs.getIntPref("telemetry.maxEventsCount", 1000);
> +    this.maxPayloadCount = Services.prefs.getIntPref("telemetry.maxPayloadCount");
> +    this.submissionInterval = Services.prefs.getIntPref("telemetry.submissionInterval") * 1000;

These are OK without default values, since an interval or max payload count of 0 will cause Bad Things™ to happen.

::: services/sync/modules/util.js
@@ +633,4 @@
>      let env = Cc["@mozilla.org/process/environment;1"]
>                  .getService(Ci.nsIEnvironment);
>      let user = env.get("USER") || env.get("USERNAME") ||
> +               Services.prefs.getStringPref("account") || Services.prefs.getStringPref("username");

...But these need default empty strings. :-)

::: services/sync/tests/unit/test_bookmark_engine.js
@@ +1125,3 @@
>    strictEqual(await legacyEngine.getLastSync(), 0,
>      "Changing legacy engine sync ID should clear last sync in Places");
> +  equal(Services.prefs.getStringPref(`${legacyEngine.name}.syncID`), newSyncID,

`Svc.Prefs`, here and below.

Comment 23

[Lina Butler (ex-Mozilla)]

I left some comments.

Comment 24

[Arshad Kazmi [:arshadkazmi42]]

Thank you for the detailed review Lina.

I have one doubt, I need to use `Svc.Prefs` everywhere instead of `Services.prefs` except for addonutils, am I right?

Comment 25

[Lina Butler (ex-Mozilla)]

(In reply to Arshad Kazmi [:arshadkazmi42] from comment #24)
> I have one doubt, I need to use `Svc.Prefs` everywhere instead of
> `Services.prefs` except for addonutils, am I right?

Nope, I _think_ `addonutils` should use `Svc.Prefs`, too. Are you seeing issues with that?

(Also, to clarify: we do have some uses of `Services.prefs` in Sync already, like https://searchfox.org/mozilla-central/rev/17f55aee76b7c4610a974cffd3453454e0c8de7b/services/sync/Weave.js#126. Those should be left alone; you don't need to change them to `Svc.Prefs`).

TL;DR: Keep all the `Svc.Prefs` parts as they are, but change the method names. :-)

Comment 26

[Arshad Kazmi [:arshadkazmi42]]

Thanks for the quick reply. I am still making those changes. 
I will check if anything fails after making all the changes and will ping you if I am stuck any where :)

Comment 27

[Lina Butler (ex-Mozilla)]

Awesome, thank you!

Comment 28

[Arshad Kazmi [:arshadkazmi42]]

Attachment: Bug - 1406475 Remove Preferences.jsm usage from Sync

Comment 29

[Arshad Kazmi [:arshadkazmi42]]

I have made all the changes mentioned in Comment 0 and Comment 22.
But still getting some error.

`./mach xpcshell-test services/sync/tests/unit/test_addon_utils.js`

I tried running this test

and it gave this error

`JavaScript error: resource://services-sync/addonutils.js, line 20: TypeError: Svc.Prefs.getStringPref is not a function
 ERROR Error: resource://services-sync/addonutils.js - Could not get symbol 'AddonUtils'. at /mozilla-central/obj-x86_64-apple-darwin17.7.0/_tests/xpcshell/services/sync/tests/unit/test_addon_utils.js:8` 

Added my changes in Phabricator.

Comment 30

[Ryan Kelly [:rfkelly]]

Clearing ancient ni? for :lina

Comment 31

[Marco Bonardo [:mak]]

I think it could make sense to completely remove Weave.Svc and replace it with straight Service.
Mark, could someone in the Services Team be the new mentor for this bug?

Comment 32

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

(In reply to Marco Bonardo [:mak] from comment #31)

I think it could make sense to completely remove Weave.Svc and replace it with straight Service.

This is a huge mess. There already is a Weave.Service, different from Weave.Svc - and you might think/hope that would at least be setup in Weave.js, but no, that would be too easy - they are setup here - so Weave.Svc is a wrapper around the observer service and Preferences.jsm - so once this bug is resolved, there would be no more references to Weave.Svc.Prefs, so then it would certainly make sense to kill Weave.Svc entirely and use Services.obs for the remaining references.

Regardless, I'll take over the mentoring from Lina, even though I suspect it might end up being too large for a new contributor, as we can see above.

Comment 33

[BugBot [:suhaib / :marco/ :calixte]]

The following patch is waiting for review from an inactive reviewer:

ID	Title	Author	Reviewer Status
D11350	Bug - 1406475 Remove Preferences.jsm usage from Sync	arshadkazmi42	lina: Inactive

:arshadkazmi42, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit auto_nag documentation.

Comment 34

[Arshad Kazmi [:arshadkazmi42]]

Hi.

As per the last comment by lina, this patch was depended on another bug.

I don't have much context on that. Also, I didn't had complete context on the exact issue, as the work was in progress that time and during the review process lina find the depenedency on a different bug.

I am not sure about anyone else, who can help here. I guess we can abandon the patch, and let someone else take a look before doing any more work on this.

I think :markh can add more details on this

Comment 35

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

Sorry for leaving this hanging, but I'm not quite sure how this patch works. Svc.Prefs is setup here - but the attached patch doesn't seem to update that. So it looks like the patch is still leaving sync using Preferences.jsm so I don't expect references to, eg, Svc.Prefs..getStringPref() to work as Preferences.jsm doesn't define those methods.

I also don't see much effort towards bug 1385954 and see new code continue to use it. So TBH, I only see value in pushing this forward is that bug is going to go somewhere. I'll ask in that bug to try and work out what the appetite is for that.

Comment 37

[Marco Castelluccio [:marco]]

Attachment: WIP: Bug 1406475 - Remove Preferences.sys.mjs usage from Sync. r=#sync-reviewers

Comment 38

[Marco Castelluccio [:marco]]

We can simply iterate through all prefs using getChildList and remove them one by one, so this doesn't have to depend on bug 720419.

Comment 39

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Add Svc.PrefBranch to util. r=#sync-reviewers

This makes it easier to convert sync to Services.prefs from Preferences.jsm incrementally.

Comment 40

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Convert sync tests to use Services.prefs instead of Preferences.sys.mjs. r=#sync-reviewers

Depends on D180801

Comment 41

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Convert some sync modules to using Services.prefs. r=#sync-reviewers

Depends on D180802

Comment 42

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Convert utils sync module to using Services.prefs. r=#sync-reviewers

Depends on D180803

Comment 43

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Convert logmanager sync module to using Services.prefs. r=#sync-reviewers

Depends on D180804

Comment 44

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Convert FxAccounts module to using Services.prefs. r=#sync-reviewers

Depends on D180805

Comment 45

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Use Svc.PrefBranch instead of Svc.Prefs in PlacesUIUtils module. r=#sync-reviewers

Depends on D180806

Comment 46

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Convert ServicesAutomation module to using Services.prefs. r=#sync-reviewers

Depends on D180807

Comment 47

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Convert tps sync module to using Services.prefs. r=#sync-reviewers

Depends on D180808

Comment 48

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Convert engines-related sync modules to using Services.prefs. r=#sync-reviewers

Depends on D180809

Comment 49

[Marco Castelluccio [:marco]]

Attachment: Bug 1406475 - Remove Svc.Prefs as it's no longer used. r=#sync-reviewers

Depends on D180810

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bccad101b6a2
Add Svc.PrefBranch to util. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/fd33b53e905a
Convert sync tests to use Services.prefs instead of Preferences.sys.mjs. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/b327642402db
Convert some sync modules to using Services.prefs. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/aeb80976d2f2
Convert utils sync module to using Services.prefs. r=sync-reviewers,skhamis

Comment 51

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/bccad101b6a2
https://hg.mozilla.org/mozilla-central/rev/fd33b53e905a
https://hg.mozilla.org/mozilla-central/rev/b327642402db
https://hg.mozilla.org/mozilla-central/rev/aeb80976d2f2

Comment 52

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e30f1f8da420
Convert logmanager sync module to using Services.prefs. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/71b3a148cf7d
Convert FxAccounts module to using Services.prefs. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/89241805ba61
Use Svc.PrefBranch instead of Svc.Prefs in PlacesUIUtils module. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/d1bd2cc70ae3
Convert ServicesAutomation module to using Services.prefs. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/662799769363
Convert tps sync module to using Services.prefs. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/d9edfe8c007e
Convert engines-related sync modules to using Services.prefs. r=sync-reviewers,skhamis
https://hg.mozilla.org/integration/autoland/rev/f3ec770d7511
Remove Svc.Prefs as it's no longer used. r=sync-reviewers,skhamis

Comment 53

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/e30f1f8da420
https://hg.mozilla.org/mozilla-central/rev/71b3a148cf7d
https://hg.mozilla.org/mozilla-central/rev/89241805ba61
https://hg.mozilla.org/mozilla-central/rev/d1bd2cc70ae3
https://hg.mozilla.org/mozilla-central/rev/662799769363
https://hg.mozilla.org/mozilla-central/rev/d9edfe8c007e
https://hg.mozilla.org/mozilla-central/rev/f3ec770d7511