548385 - audit prefapi usage in prefs sync engine

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Mike Connor [:mconnor]]

http://mxr.mozilla.org/labs-central/source/weave/source/modules/engines/prefs.js

Comment 1

[Mike Connor [:mconnor]]

Notes:

* no good reason to sync pref type, we could just check the local type
** This would let us do prefs[prefname] = value and have a simpler data format.
** would be a storage version bump, but that's not especially terrible for prefs
* we don't check whether the local client allows syncing of this preference before setting it
* engine/store/tracker bites us with code duplication in store/tracker
* itemExists: function FormStore_itemExists should be PrefStore
* we could use Preferences.js to simplify a lot of this if we stop syncing type!

  _prefs:     new Preferences();

  _getAllPrefs: function () {
    let values = {};
    let toSync = this._syncPrefs;

    for (let i = 0; i < toSync.length; i++) {
      if (!this._prefs.get(WEAVE_SYNC_PREFS + toSync[i]), false)
        continue;

      let val = this._prefs.get(toSync[i], "");
      if (val)
        values[toSync[i]] = val;
    }

    return values;
  },

  _setAllPrefs: function(values) {
    for (let pref in values) {
      if (!this._prefs.get(WEAVE_SYNC_PREFS + toSync[i]), false)
        continue;
      try {
        this._prefs.set(pref, values[pref]);
      catch(e) {
        this._log.trace("Failed to set pref: " + pref + " Reason: " + e);
      } 
    }   
  }

Comment 2

[Ed Lee :Mardak]

It might be possible to sync a pref that only existed on one profile, so checking the type on the remote end might not always work. But assuming we only sync bool, int, string prefs, Preferences can figure out what we want.

Comment 3

[Mike Connor [:mconnor]]

Well, right now we ignore the pref if it's anything else, so... yeah, we can probably assume that we're only syncing those values.

Comment 4

[Mike Connor [:mconnor]]

filed Bug 560580 for cleanup, and tweaked Bug 550033 to cover per-app pref syncing.

No other issues in play.