Closed Bug 1296767 Opened 8 years ago Closed 8 years ago

Remove legacy Sync code from Sync

Categories

(Firefox :: Sync, defect, P1)

Product:
Firefox
Component:
Sync
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 54
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: lina, Assigned: eoger)

References

Details

(Whiteboard: [sync-quality])

Attachments

(13 files)

Bug 1296767 part 1 - Remove J-PAKE from Sync.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 3 - Make BrowserID the only Sync identity manager.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 2 - Remove Sync UserApi.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 4 - Update tests to work with the BrowserID identity manager.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 5 - Remove add_identity_test helper.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 6 - Remove startOverKeepIdentity pref.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 7 - Remove non-in-content legacy Sync pref screens.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 8 - Remove legacy Sync in-content pref screens.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 9 - Remove Old Sync link in about:accounts.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 10 - Remove FxA Migration code.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 11 - Remove Weave.Service.serverURL and friends.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 12 - Remove Weave.fxAccountsEnabled.
59 bytes, text/x-review-board-request
markh
: review+
Details
Bug 1296767 part 13 - Repair Weave Service getStorageInfo.
59 bytes, text/x-review-board-request
markh
: review+
Details 
IIUC, the entire J-PAKE flow is obsolete in the FxA world, and Sync's jpakeclient appears to be the only consumer of http://searchfox.org/mozilla-central/source/services/crypto/component/nsISyncJPAKE.idl.

We can probably ditch that, jpakeclient, and the old IdentityManager for good measure.
I agree we should do this and I believe tests are where the pain will lie. I'm not sure this should be only P5 though - a case could well be made that we should do this in the interests of general quality.
Priority: P5 → --
Triage meeting: The pain factor will increase until we do this. But, until such time...P3!
Priority: -- → P3
Whiteboard: [sync-quality]
And <services/sync/services-sync.js> has a bunch of J-PAKE prefs (including references to servers that are no longer live!).
See also Bug 1312157. This code is a maintenance burden.
See Also: → 1312157
See Also: → 1310576
Priority: P3 → P2
Priority: P2 → P3
So here's a WIP of what I've been working on. This patch removes J-PAKE and also removes the old identity manager (and the UserAPI too).
I updated the unit tests and they all seem to pass.

I also found out that the Weave service methods |getStorageInfo()| and |getStorageRequest()| are broken with the FxA identity manager, hence disabled some tests.

We should probably go all the way and remove the old pref screens and the migration assistants.
Comment on attachment 8827231 [details]
Bug 1296767 part 1 - Remove J-PAKE from Sync.

https://reviewboard.mozilla.org/r/104978/#review105800

A drive-by, but I think this is worthwhile - thanks! :)

::: services/sync/tests/unit/xpcshell.ini:45
(Diff revision 1)
>  [test_addon_utils.js]
>  run-sequentially = Restarts server, can't change pref.
>  tags = addons
>  [test_httpd_sync_server.js]
> -[test_jpakeclient.js]
>  # Bug 618233: this test produces random failures on Windows 7.

I think these lines need to be deleted too - now they apply to the previous test.
Comment on attachment 8827232 [details]
Bug 1296767 part 3 - Make BrowserID the only Sync identity manager.

https://reviewboard.mozilla.org/r/104980/#review105842

I *love* seeing code die :)

::: services/sync/modules-testing/utils.js:173
(Diff revision 1)
>          key: "key",
>          hashed_fxa_uid: "f".repeat(32), // used during telemetry validation
>          // uid will be set to the username.
>        }
>      },
>      sync: {

I think we can kill "sync" here?

::: services/sync/modules-testing/utils.js:185
(Diff revision 1)
>    // Now handle any specified overrides.
>    if (overrides) {
>      if (overrides.username) {
>        result.username = overrides.username;
>      }
>      if (overrides.sync) {

and here.

::: services/sync/modules-testing/utils.js:262
(Diff revision 1)
>  
>    if (server) {
>      ns.Service.serverURL = server.baseURI;
>    }
>  
>    ns.Service._clusterManager = ns.Service.identity.createClusterManager(ns.Service);

I suspect this line can die too

::: services/sync/modules-testing/utils.js:283
(Diff revision 1)
> -  ns.Service.identity.username = config.username;
> -  ns.Service._updateCachedURLs();
> -  setBasicCredentials(config.username, config.sync.password, config.sync.syncKey);
>  }
>  
>  this.SyncTestingInfrastructure = async function(server, username, password) {

We should try and kill |password| here too

::: services/sync/modules-testing/utils.js:322
(Diff revision 1)
>  // * The test itself should be passed as 'test' - ie, test code will generally
>  //   pass |this|.
>  // * The test function is a regular test function - although note that it must
>  //   be a generator - async operations should yield them, and run_next_test
>  //   mustn't be called.
>  this.add_identity_test = function(test, testFunction) {

TBH I'd be inclined to kill this too and replace existing use with a simple add_task (but probably the removal of this would make sense as its own patch in this bug)

(I'm happy to keep it if you disagree though)

::: services/sync/modules/browserid_identity.js:400
(Diff revision 1)
> +  get username() {
> +    return Svc.Prefs.get("username", null);
>    },
>  
>    /**
> -   * Obtain the Sync Key.
> +   * Set the username value.

I wonder if we can remove .username completely? Or maybe just the setter (ie, have the .account setter roll that code up)

::: services/sync/modules/browserid_identity.js:442
(Diff revision 1)
>    },
>  
>    /**
> -   * Resets/Drops the sync key we hold for the current user.
> +   * Resets/Drops the sync key bundle we hold for the current user.
>     */
>    resetSyncKey() {

did you mean to rename this to resetSyncKeyBundle?

::: services/sync/modules/browserid_identity.js:467
(Diff revision 1)
>    },
>  
>    /**
> +   * Deletes Sync credentials from the password manager.
> +   */
> +  deleteSyncCredentials: function deleteSyncCredentials() {

use new-style function del here.

::: services/sync/modules/service.js:323
(Diff revision 1)
>     * Prepare to initialize the rest of Weave after waiting a little bit
>     */
>    onStartup: function onStartup() {
>      this._migratePrefs();
>  
>      // Status is instantiated before us and is the first to grab an instance of

I think we can probably kill this check and comment now - we've since refactored such that it should be impossible to hit

::: services/sync/modules/status.js:27
(Diff revision 1)
>  
>    get _authManager() {
>      if (this.__authManager) {
>        return this.__authManager;
>      }
>      let service = Components.classes["@mozilla.org/weave/service;1"]

I think we can kill |service| here too
Comment on attachment 8827233 [details]
Bug 1296767 part 2 - Remove Sync UserApi.

https://reviewboard.mozilla.org/r/104982/#review105848

f+
Comment on attachment 8827234 [details]
Bug 1296767 part 4 - Update tests to work with the BrowserID identity manager.

https://reviewboard.mozilla.org/r/104984/#review105850

Thanks for doing this!  I only scanned this quickly, but it LGTM! Other things to consider doing in one of these patches:

* have Service.login drop all params - IIRC, it is now never called with any args by sync itself, so we could nuke the code that handles them and nuke the params from all tests which call it.
* and even more generally, kill anything that references a "password" at all :)
* Look into killing FxaMigrator.jsm completely (in this bug or a different bug) and nuke any references to the migrator.
* Look at Service.startOver() and see if we can kill the |services.sync-testing.startOverKeepIdentity| pref - that was added simply to tests could arrange for .startOver() to be called, but still be left with the legacy identity when it completes (as it switches to the bid identity by default). Killing the |weave:service:start-over:init-identity| notification will also be possible if the migrator is completely killed.

::: services/sync/tests/unit/test_keys.js:177
(Diff revision 1)
> -  let identity = new IdentityManager();
> +  var identityConfig = makeIdentityConfig();
> +  var browseridManager = new BrowserIDManager();
> +  configureFxAccountIdentity(browseridManager, identityConfig);
> +  await browseridManager.initializeWithCurrentIdentity();
> +  await browseridManager.whenReadyToAuthenticate.promise;
> +  await browseridManager.ensureLoggedIn();

IIUC, this line is the only one necessary here - the 2 lines above should be done by ensureLoggedIn. Ideally initWithCurrentID and whenReadyToAuthenticate will eventually die - these patched should make it far easier to refactor bid_identity in the future :)

::: services/sync/tests/unit/test_node_reassignment.js:93
(Diff revision 1)
>   * setup, detach observers, etc.
>   */
>  async function syncAndExpectNodeReassignment(server, firstNotification, between,
>                                         secondNotification, url) {
>    let deferred = PromiseUtils.defer();
> +  let oldGetAssertion = Service.identity._fxaService.internal._getAssertion;

So IIUC, these tests are basically just "ensure we've re-fetched a token after a 401", right? If so, I wonder if we could, say, have util.js's mock of getTokenFromBrowserIDAssertion() increment a counter somewhere to indicate how many token fetches have happened, and most of these tests could just check that counter is exactly 1 (or maybe 2, assuming there is an initial token fetch before the 401)

I'm totally making that up, but it seems there might be an opportunity to have these checks made without repeating the same mock/undo-mock dance

::: services/sync/tests/unit/test_resource.js:243
(Diff revision 1)
>    do_check_false(content.success);
>  
>    _("GET a password protected resource");
>    let res3 = new Resource(server.baseURI + "/protected");
> -  let identity = new IdentityManager();
> -  let auth = identity.getBasicResourceAuthenticator("guest", "guest");
> +  let identityConfig = makeIdentityConfig();
> +  let browseridManager = new BrowserIDManager();

I'm surprised we need to make a new BrowserIDManager() now - isn't the existing one OK to reconfigure?
Assignee: nobody → eoger
Priority: P3 → P1
I increased the scope of this patch: all the legacy sync code is removed (+migration) and we also take care of bug 1319457 and bug 1328458.
Comment on attachment 8827231 [details]
Bug 1296767 part 1 - Remove J-PAKE from Sync.

https://reviewboard.mozilla.org/r/104978/#review107316
Attachment #8827231 - Flags: review?(markh) → review+
Comment on attachment 8827233 [details]
Bug 1296767 part 2 - Remove Sync UserApi.

https://reviewboard.mozilla.org/r/104982/#review107320

::: services/sync/modules/service.js:108
(Diff revision 2)
> -    }
> -
> -    return url + USER_API_VERSION + "/";
> -  },
> -
>    get pwResetURL() {

kill this too (including a reference in test_service_attributes.js)

::: services/sync/modules/service.js:812
(Diff revision 2)
> -    this.identity.basicPassword = newPassword;
> -    this.persistLogin();
> -    return true;
> -  },
> -
>    changePassphrase: function changePassphrase(newphrase) {

I think this can die too and test ref in test_service_sync_remoteSetup.js (obviously ignore these if they are already removed in a later patch :)
Attachment #8827233 - Flags: review?(markh) → review+
Comment on attachment 8827232 [details]
Bug 1296767 part 3 - Make BrowserID the only Sync identity manager.

https://reviewboard.mozilla.org/r/104980/#review107322

This looks good in general, but I think the removal of the .username pref and spinning in getters is taking things a bit too far :)

::: browser/components/preferences/in-content/sync.js:89
(Diff revision 2)
>  
>      xps.ensureLoaded();
>    },
>  
>    _showLoadPage(xps) {
> -    let username;
> +    fxAccounts.getSignedInUser().then(user => {

I'm a little worried about this change - how will the page look between the page being loaded and getSignedInUser resolving?

I think I mis-led you in my previous comments - IMO it probably makes sense to keep that pref - addons etc might use it and it's a reasonable synchronous approximation of both "is sync enabled" and "what's the user's email address.) That said though, I think it *does* make sense to move away from that pref where we can - eg, the change above in browser-syncui seems fine as it's already promise based, but here and below I think the pref should stay.

::: services/sync/Weave.js:123
(Diff revision 2)
>     * It does *not* perform a robust check to see if the client is working.
>     * For that, you'll want to check Weave.Status.checkSetup().
>     */
>    get enabled() {
> -    let prefs = Services.prefs.getBranch(SYNC_PREFS_BRANCH);
> -    return prefs.prefHasUserValue("username");
> +    let fxaService = this.fxaServiceMock || fxAccounts;
> +    return Async.promiseSpinningly(fxaService.getSignedInUser()) != null;

ditto here - using promiseSpinningly seems painful as there's no path forward to removing it later (eg, even if sync becomes fully promise-based, there's still no sane way to make this promise based.

::: services/sync/modules/browserid_identity.js
(Diff revision 2)
>  
>    initialize() {
>      for (let topic of OBSERVER_TOPICS) {
>        Services.obs.addObserver(this, topic, false);
>      }
> -    // and a background fetch of account data just so we can set this.account,

yay to killing test-only hacks :)

::: services/sync/modules/browserid_identity.js:386
(Diff revision 2)
>    },
>  
>    /**
>      * Return credentials hosts for this identity only.
>      */
>    _getSyncCredentialsHosts() {

there's probably an opportunity to clean some of this up (ie, remove the functions from util.js and have them live just here)

::: services/sync/modules/browserid_identity.js:428
(Diff revision 2)
>  
>      return STATUS_OK;
>    },
>  
> +  // Do we have a configured FxAccount associated with that identity?
> +  get configured() {

similarly here - I'm not too keen on a getter that spins as it will be difficult to remove without further refactoring.

::: services/sync/modules/browserid_identity.js:752
(Diff revision 2)
>  
>  /* An implementation of the ClusterManager for this identity
>   */
>  
>  function BrowserIDClusterManager(service) {
> -  ClusterManager.call(this, service);
> +  this._log = Log.repository.getLogger("Sync.Service");

I'd be inclined to just use the existing "log" global here (although it also looks like that global uses a pref |log.logger.identity|, which should maybe change? I'm not too bothered by that though)
Attachment #8827232 - Flags: review?(markh)
Comment on attachment 8827234 [details]
Bug 1296767 part 4 - Update tests to work with the BrowserID identity manager.

https://reviewboard.mozilla.org/r/104984/#review107328

::: services/sync/tests/unit/head_http_server.js:28
(Diff revision 2)
>    return timestamp;
>  }
>  
> +function has_hawk_header(req) {
> +  return req.hasHeader("Authorization") &&
> +      req.getHeader("Authorization").startsWith("Hawk");

this indentation looks odd.

::: services/sync/tests/unit/test_keys.js:172
(Diff revision 2)
>  
> -add_test(function test_collections_manager() {
> +add_task(async function test_ensureLoggedIn() {
>    let log = Log.repository.getLogger("Test");
>    Log.repository.rootLogger.addAppender(new Log.DumpAppender());
>  
> -  let identity = new IdentityManager();
> +  var identityConfig = makeIdentityConfig();

any reason not to use |let| here?

::: services/sync/tests/unit/test_status_checkSetup.js:22
(Diff revision 2)
>      do_check_eq(Status.checkSetup(), CLIENT_NOT_CONFIGURED);
>      do_check_eq(Status.login, LOGIN_FAILED_NO_USERNAME);
>      Status.resetSync();
>  
> -    _("Let's provide a username.");
> -    Status._authManager.username = "johndoe";
> +    _("Let's provide the syncKeyBundle");
> +    Svc.Prefs.set("username", "carotsalad");

I don't understand why this pref needs to be set here (although it *would* make more sense if the previous patch didn't seem to nuke it entirely?)

::: services/sync/tests/unit/test_syncstoragerequest.js:60
(Diff revision 2)
>      Svc.Prefs.resetBranch("");
>      server.stop(run_next_test);
>    });
>  });
>  
> -add_test(function test_auth() {
> +// XXX - DISABLED BECAUSE getStorageRequest broken with browserid_manager

Is this removed in a later patch?

::: services/sync/tests/unit/xpcshell.ini:79
(Diff revision 2)
>  skip-if = os == "mac" || os == "linux"
> -[test_service_checkAccount.js]
>  [test_service_cluster.js]
>  [test_service_detect_upgrade.js]
> -[test_service_getStorageInfo.js]
> +# XXX - Disabled because getStorageInfo is broken
> +# [test_service_getStorageInfo.js]

ditto here - is this removed later?
Attachment #8827234 - Flags: review?(markh) → review+
Comment on attachment 8828496 [details]
Bug 1296767 part 5 - Remove add_identity_test helper.

https://reviewboard.mozilla.org/r/105872/#review107330
Attachment #8828496 - Flags: review?(markh) → review+
Comment on attachment 8828497 [details]
Bug 1296767 part 6 - Remove startOverKeepIdentity pref.

https://reviewboard.mozilla.org/r/105874/#review107332

::: services/fxaccounts/tests/xpcshell/head.js:8
(Diff revision 1)
>  
>  var {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components;
>  
>  "use strict";
>  
> +Cu.import("resource://gre/modules/Services.jsm");

this change doesn't seem necessary?
Attachment #8828497 - Flags: review?(markh) → review+
Comment on attachment 8828498 [details]
Bug 1296767 part 7 - Remove non-in-content legacy Sync pref screens.

https://reviewboard.mozilla.org/r/105876/#review107334

::: browser/base/content/aboutaccounts/aboutaccounts.js:199
(Diff revision 1)
>     * @param accountData the user's account data and credentials
>     */
>    onLogin(accountData) {
>      log("Received: 'login'. Data:" + JSON.stringify(accountData));
>  
> -    if (accountData.customizeSync) {
> +    // We don't act on customizeSync anymore, it used to open a dialog inside

Can you please check if customizeSync is sent here any more? If not, just delete the delete :) and the new comment. The comments and further delete below can probably also be removed now given that content-server issue has been resolved.

::: browser/base/content/sync/utils.js:38
(Diff revision 1)
> -    if (Weave.Utils.ensureMPUnlocked())
> -      this.openChange("UpdatePassphrase");
> -  },
> -
>    resetPassword() {
>      this._openLink(Weave.Service.pwResetURL);

I think we can kill this too (I think I suggested pwResetURL be removed in an earlier patch - removing the impl in this patch would be fine too)

::: browser/base/content/sync/utils.js:41
(Diff revision 1)
> -
>    resetPassword() {
>      this._openLink(Weave.Service.pwResetURL);
>    },
>  
>    get tosURL() {

are tosURL and privacyPolicyURL still used now that we've removed the `open*` functions?

::: services/fxaccounts/FxAccountsWebChannel.jsm:262
(Diff revision 1)
>     */
>    login(accountData) {
> -    if (accountData.customizeSync) {
> -      this.setShowCustomizeSyncPref(true);
> +
> +    // We don't act on customizeSync anymore, it used to open a dialog inside
> +    // the browser to selecte the engines to sync but we do it on the web now.
> -      delete accountData.customizeSync;
> +    delete accountData.customizeSync;

similarly here, can you please check if this is even sent, and if not, delete the delete and comment?
Attachment #8828498 - Flags: review?(markh) → review+
Comment on attachment 8828499 [details]
Bug 1296767 part 8 - Remove legacy Sync in-content pref screens.

https://reviewboard.mozilla.org/r/105878/#review107336

::: browser/components/preferences/in-content/sync.js:367
(Diff revision 1)
>    },
>  
>    openContentInBrowser(url, options) {
>      let win = Services.wm.getMostRecentWindow("navigator:browser");
>      if (!win) {
>        // no window to use, so use _openLink to create a new one.  We don't

this comment is now stale (and it seems a shame that if I'm reading openUILinkIn correctly it doesn't support the switchToTabHavingURI capability, but there you have it!)
Attachment #8828499 - Flags: review?(markh) → review+
Comment on attachment 8828500 [details]
Bug 1296767 part 9 - Remove Old Sync link in about:accounts.

https://reviewboard.mozilla.org/r/105880/#review107340
Attachment #8828500 - Flags: review?(markh) → review+
Comment on attachment 8828501 [details]
Bug 1296767 part 10 - Remove FxA Migration code.

https://reviewboard.mozilla.org/r/105882/#review107342

::: browser/components/uitour/test/browser.ini:11
(Diff revision 1)
>    ../UITour-lib.js
>  
>  [browser_backgroundTab.js]
>  [browser_closeTab.js]
>  [browser_fxa.js]
> -skip-if = debug || asan # updateAppMenuItem leaks
> +skip-if = debug || asan # updateUI leaks

I think we should get a bug on file for looking further at this - if the leaks are real it would be bad! Can you please open a bug to investigate and add the bug number here?

::: browser/components/uitour/test/browser_fxa.js:38
(Diff revision 1)
>  
>    taskify(function* test_highlight_accountStatus_loggedIn() {
>      yield setSignedInUser();
>      let userData = yield fxAccounts.getSignedInUser();
>      isnot(userData, null, "Logged in now");
> -    gFxAccounts.updateAppMenuItem(); // Causes a leak
> +    gFxAccounts.updateUI(); // Causes a leak

(And I guess add the bug# here too)
Attachment #8828501 - Flags: review?(markh) → review+
Comment on attachment 8828502 [details]
Bug 1296767 part 11 - Remove Weave.Service.serverURL and friends.

https://reviewboard.mozilla.org/r/105884/#review107344

::: services/sync/modules/service.js
(Diff revision 1)
> -    if (misc.indexOf(":") == -1)
> -      misc = this.serverURL + misc;
> -    return misc + MISC_API_VERSION + "/";
> -  },
> -
> -  get pwResetURL() {

ah, here's the removal I mentioned earlier :)
Attachment #8828502 - Flags: review?(markh) → review+
Comment on attachment 8828503 [details]
Bug 1296767 part 12 - Remove Weave.fxAccountsEnabled.

https://reviewboard.mozilla.org/r/105886/#review107348

This looks good, but please check TPS passes with this (and the other) changes. There are docs somewhere and tcsc can help if you have problems.

Thanks for all these patches, it's very nice to see this killed! We should also consider seeing if we can get softvision to perform some testing for us here to make sure we haven't broken the UI in subtle ways.
Attachment #8828503 - Flags: review?(markh) → review+
(In reply to Mark Hammond [:markh] from comment #36)
> >  [browser_fxa.js]
> > -skip-if = debug || asan # updateAppMenuItem leaks
> > +skip-if = debug || asan # updateUI leaks
> 
> I think we should get a bug on file for looking further at this - if the
> leaks are real it would be bad! Can you please open a bug to investigate and
> add the bug number here?

Ah, I just found that this came from bug 1185579 comment 21 and *I* was asked to open a bug ;) I opened bug 1332985
Comment on attachment 8828497 [details]
Bug 1296767 part 6 - Remove startOverKeepIdentity pref.

https://reviewboard.mozilla.org/r/105874/#review107608

::: services/fxaccounts/tests/xpcshell/head.js:8
(Diff revision 1)
>  
>  var {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components;
>  
>  "use strict";
>  
> +Cu.import("resource://gre/modules/Services.jsm");

It actually is, because in a previous patch I remove utils.js, which pulled Services.jsm for us in these tests.
Comment on attachment 8828498 [details]
Bug 1296767 part 7 - Remove non-in-content legacy Sync pref screens.

https://reviewboard.mozilla.org/r/105876/#review107334

> are tosURL and privacyPolicyURL still used now that we've removed the `open*` functions?

We actually remove the whole file in the next commits of this patch.

> similarly here, can you please check if this is even sent, and if not, delete the delete and comment?

It is still sent, but declinedSyncEngines overrides it anyway.
Summary: Remove J-PAKE from Sync → Remove legacy Sync code from Sync
I made adjustments to my patch, and put back the username getter/setter.
Comment on attachment 8827232 [details]
Bug 1296767 part 3 - Make BrowserID the only Sync identity manager.

https://reviewboard.mozilla.org/r/104980/#review108078

\o/
Attachment #8827232 - Flags: review?(markh) → review+
Try looks bad, but I'm sure they will be easy to resolve :)
Comment on attachment 8829958 [details]
Bug 1296767 part 13 - Repair Weave Service getStorageInfo.

https://reviewboard.mozilla.org/r/106622/#review108080

LGTM and you should land this, but I *think* we can probably kill getStorageInfo() and that part of rest.js too at some point?
Attachment #8829958 - Flags: review?(markh) → review+
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9f59a0b75c1f
part 1 - Remove J-PAKE from Sync. r=markh
https://hg.mozilla.org/integration/autoland/rev/0a989b0cea67
part 2 - Remove Sync UserApi. r=markh
https://hg.mozilla.org/integration/autoland/rev/8219686be6a2
part 3 - Make BrowserID the only Sync identity manager. r=markh
https://hg.mozilla.org/integration/autoland/rev/5d70d87d2a8f
part 4 - Update tests to work with the BrowserID identity manager. r=markh
https://hg.mozilla.org/integration/autoland/rev/46a52b10a868
part 5 - Remove add_identity_test helper. r=markh
https://hg.mozilla.org/integration/autoland/rev/6c9b792cc296
part 6 - Remove startOverKeepIdentity pref. r=markh
https://hg.mozilla.org/integration/autoland/rev/f6f59447d22a
part 7 - Remove non-in-content legacy Sync pref screens. r=markh
https://hg.mozilla.org/integration/autoland/rev/4e71cf94e4ee
part 8 - Remove legacy Sync in-content pref screens. r=markh
https://hg.mozilla.org/integration/autoland/rev/0362a78d6978
part 9 - Remove Old Sync link in about:accounts. r=markh
https://hg.mozilla.org/integration/autoland/rev/26c065f79c54
part 10 - Remove FxA Migration code. r=markh
https://hg.mozilla.org/integration/autoland/rev/50294db1d871
part 11 - Remove Weave.Service.serverURL and friends. r=markh
https://hg.mozilla.org/integration/autoland/rev/1c0c9289b532
part 12 - Remove Weave.fxAccountsEnabled. r=markh
https://hg.mozilla.org/integration/autoland/rev/41ed77788333
part 13 - Repair Weave Service getStorageInfo. r=markh
Looks like I got caught up in the middle of bug 1313045 landing. Rebase fixed.
Flags: needinfo?(eoger)
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a0022f621594
part 1 - Remove J-PAKE from Sync. r=markh
https://hg.mozilla.org/integration/autoland/rev/b2f0066f5949
part 2 - Remove Sync UserApi. r=markh
https://hg.mozilla.org/integration/autoland/rev/e32665726c60
part 3 - Make BrowserID the only Sync identity manager. r=markh
https://hg.mozilla.org/integration/autoland/rev/34e62bf16dd0
part 4 - Update tests to work with the BrowserID identity manager. r=markh
https://hg.mozilla.org/integration/autoland/rev/5f82f1fc554a
part 5 - Remove add_identity_test helper. r=markh
https://hg.mozilla.org/integration/autoland/rev/438324d172d7
part 6 - Remove startOverKeepIdentity pref. r=markh
https://hg.mozilla.org/integration/autoland/rev/a30baab71ce2
part 7 - Remove non-in-content legacy Sync pref screens. r=markh
https://hg.mozilla.org/integration/autoland/rev/ff779cc6775b
part 8 - Remove legacy Sync in-content pref screens. r=markh
https://hg.mozilla.org/integration/autoland/rev/5d2d98c37b0e
part 9 - Remove Old Sync link in about:accounts. r=markh
https://hg.mozilla.org/integration/autoland/rev/f08ca0b9ec72
part 10 - Remove FxA Migration code. r=markh
https://hg.mozilla.org/integration/autoland/rev/31eb6865049c
part 11 - Remove Weave.Service.serverURL and friends. r=markh
https://hg.mozilla.org/integration/autoland/rev/026b733245ee
part 12 - Remove Weave.fxAccountsEnabled. r=markh
https://hg.mozilla.org/integration/autoland/rev/da1c5b3e73f9
part 13 - Repair Weave Service getStorageInfo. r=markh
Depends on: 1335409
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: