Backed out changeset 705939ae18f0 (bug 1317223)
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Thu, 02 Mar 2017 13:08:46 +0100
changeset 374643 04f6663f94cb87ee4df010c6162b7fca5b9106cf
parent 374642 4c8faad8c333008dcd8eea59b18ea29d391d9a2c
child 374644 ae014ce1be8c09f4352fc7bc715df05fd8630622
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1317223
milestone54.0a1
backs out705939ae18f00dd3ccba81fc4e77acefdb51561f
Backed out changeset 705939ae18f0 (bug 1317223)
.eslintignore
browser/app/profile/firefox.js
services/sync/modules/bookmark_validator.js
services/sync/modules/doctor.js
services/sync/modules/stages/enginesync.js
services/sync/moz.build
services/sync/services-sync.js
services/sync/tests/unit/head_appinfo.js
services/sync/tests/unit/test_doctor.js
services/sync/tests/unit/xpcshell.ini
tools/lint/eslint/modules.json
--- a/.eslintignore
+++ b/.eslintignore
@@ -284,17 +284,16 @@ security/manager/ssl/security-prefs.js
 
 #NSS
 security/nss/**
 
 # services/ exclusions
 
 # Uses `#filter substitution`
 services/sync/modules/constants.js
-services/sync/services-sync.js
 
 # Third party services
 services/common/kinto-http-client.js
 services/common/kinto-offline-client.js
 services/sync/tps/extensions/mozmill
 
 # toolkit/ exclusions
 
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1576,16 +1576,22 @@ pref("browser.crashReports.unsubmittedCh
 
 // chancesUntilSuppress is how many times we'll show the unsubmitted
 // crash report notification across different days and shutdown
 // without a user choice before we suppress the notification for
 // some number of days.
 pref("browser.crashReports.unsubmittedCheck.chancesUntilSuppress", 4);
 pref("browser.crashReports.unsubmittedCheck.autoSubmit", false);
 
+#ifdef NIGHTLY_BUILD
+// Enable the (fairly costly) client/server validation on nightly only. The other prefs
+// controlling validation are located in /services/sync/services-sync.js
+pref("services.sync.validation.enabled", true);
+#endif
+
 // Preferences for the form autofill system extension
 pref("browser.formautofill.experimental", false);
 pref("browser.formautofill.enabled", false);
 pref("browser.formautofill.loglevel", "Warn");
 
 // Enable safebrowsing v4 tables (suffixed by "-proto") update.
 #ifdef NIGHTLY_BUILD
 pref("urlclassifier.malwareTable", "goog-malware-shavar,goog-unwanted-shavar,goog-malware-proto,goog-unwanted-proto,test-malware-simple,test-unwanted-simple");
--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -802,18 +802,16 @@ class BookmarkValidator {
       if (structuralDifferences.length) {
         problemData.structuralDifferences.push({ id, differences: structuralDifferences });
       }
     }
     return inspectionInfo;
   }
 
   _getServerState(engine) {
-// XXXXX - todo - we need to capture last-modified of the server here and
-// ensure the repairer only applys with if-unmodified-since that date.
     let collection = engine.itemSource();
     let collectionKey = engine.service.collectionKeys.keyForCollection(engine.name);
     collection.full = true;
     let items = [];
     collection.recordHandler = function(item) {
       item.decrypt(collectionKey);
       items.push(item.cleartext);
     };
deleted file mode 100644
--- a/services/sync/modules/doctor.js
+++ /dev/null
@@ -1,250 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-// A doctor for our collections. She can be asked to make a consultation, and
-// may just diagnose an issue without attempting to cure it, may diagnose and
-// attempt to cure, or may decide she is overworked and underpaid.
-// Or something - naming is hard :)
-
-"use strict";
-
-this.EXPORTED_SYMBOLS = ["Doctor"];
-
-const Cu = Components.utils;
-
-Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://gre/modules/Log.jsm");
-Cu.import("resource://services-common/async.js");
-Cu.import("resource://services-common/observers.js");
-Cu.import("resource://services-sync/service.js");
-Cu.import("resource://services-sync/resource.js");
-
-Cu.import("resource://services-sync/util.js");
-XPCOMUtils.defineLazyModuleGetter(this, "getRepairRequestor",
-  "resource://services-sync/collection_repair.js");
-XPCOMUtils.defineLazyModuleGetter(this, "getAllRepairRequestors",
-  "resource://services-sync/collection_repair.js");
-
-const log = Log.repository.getLogger("Sync.Doctor");
-
-this.REPAIR_ADVANCE_PERIOD = 86400; // 1 day
-
-this.Doctor = {
-  anyClientsRepairing(service, collection, ignoreFlowID = null) {
-    if (!service || !service.clientsEngine) {
-      log.info("Missing clients engine, assuming we're in test code");
-      return false;
-    }
-    return service.clientsEngine.remoteClients.some(client =>
-      client.commands && client.commands.some(command => {
-        if (command.command != "repairResponse" && command.command != "repairRequest") {
-          return false;
-        }
-        if (!command.args || command.args.length != 1) {
-          return false;
-        }
-        if (command.args[0].collection != collection) {
-          return false;
-        }
-        if (ignoreFlowID != null && command.args[0].flowID == ignoreFlowID) {
-          return false;
-        }
-        return true;
-      })
-    );
-  },
-
-  async consult(recentlySyncedEngines) {
-    if (!Services.telemetry.canRecordBase) {
-      log.info("Skipping consultation: telemetry reporting is disabled");
-      return;
-    }
-
-    let engineInfos = this._getEnginesToValidate(recentlySyncedEngines);
-
-    await this._runValidators(engineInfos);
-
-    // We are called at the end of a sync, which is a good time to periodically
-    // check each repairer to see if it can advance.
-    if (this._now() - this.lastRepairAdvance > REPAIR_ADVANCE_PERIOD) {
-      try {
-        for (let [collection, requestor] of Object.entries(this._getAllRepairRequestors())) {
-          try {
-            let advanced = requestor.continueRepairs();
-            log.info(`${collection} reparier ${advanced ? "advanced" : "did not advance"}.`);
-          } catch (ex) {
-            if (Async.isShutdownException(ex)) {
-              throw ex;
-            }
-            log.error(`${collection} repairer failed`, ex);
-          }
-        }
-      } finally {
-        this.lastRepairAdvance = this._now();
-      }
-    }
-  },
-
-  _getEnginesToValidate(recentlySyncedEngines) {
-    let result = {};
-    for (let e of recentlySyncedEngines) {
-      let prefPrefix = `engine.${e.name}.`;
-      if (!Svc.Prefs.get(prefPrefix + "validation.enabled", false)) {
-        log.info(`Skipping check of ${e.name} - disabled via preferences`);
-        continue;
-      }
-      // Check the last validation time for the engine.
-      let lastValidation = Svc.Prefs.get(prefPrefix + "validation.lastTime", 0);
-      let validationInterval = Svc.Prefs.get(prefPrefix + "validation.interval");
-      let nowSeconds = this._now();
-
-      if (nowSeconds - lastValidation < validationInterval) {
-        log.info(`Skipping validation of ${e.name}: too recent since last validation attempt`);
-        continue;
-      }
-      // Update the time now, even if we decline to actually perform a
-      // validation. We don't want to check the rest of these more frequently
-      // than once a day.
-      Svc.Prefs.set("validation.lastTime", Math.floor(nowSeconds));
-
-      // Validation only occurs a certain percentage of the time.
-      let validationProbability = Svc.Prefs.get(prefPrefix + "validation.percentageChance", 0) / 100.0;
-      if (validationProbability < Math.random()) {
-        log.info(`Skipping validation of ${e.name}: Probability threshold not met`);
-        continue;
-      }
-
-      let maxRecords = Svc.Prefs.get(prefPrefix + "validation.maxRecords");
-      if (!maxRecords) {
-        log.info(`Skipping validation of ${e.name}: No maxRecords specified`);
-        continue;
-      }
-      // OK, so this is a candidate - the final decision will be based on the
-      // number of records actually found.
-      result[e.name] = { engine: e, maxRecords };
-    }
-    return result;
-  },
-
-  async _runValidators(engineInfos) {
-    if (Object.keys(engineInfos).length == 0) {
-      log.info("Skipping validation: no engines qualify");
-      return;
-    }
-
-    if (Object.values(engineInfos).filter(i => i.maxRecords != -1).length != 0) {
-      // at least some of the engines have maxRecord restrictions which require
-      // us to ask the server for the counts.
-      let countInfo = this._fetchCollectionCounts();
-      for (let [engineName, recordCount] of Object.entries(countInfo)) {
-        if (engineName in engineInfos) {
-          engineInfos[engineName].recordCount = recordCount;
-        }
-      }
-    }
-
-    for (let [engineName, { engine, maxRecords, recordCount }] of Object.entries(engineInfos)) {
-      // maxRecords of -1 means "any number", so we can skip asking the server.
-      // Used for tests.
-      if (maxRecords >= 0 && recordCount > maxRecords) {
-        log.debug(`Skipping validation for ${engineName} because ` +
-                        `the number of records (${recordCount}) is greater ` +
-                        `than the maximum allowed (${maxRecords}).`);
-        continue;
-      }
-      let validator = engine.getValidator();
-      if (!validator) {
-        continue;
-      }
-
-      // Let's do it!
-      Services.console.logStringMessage(
-        `Sync is about to run a consistency check of ${engine.name}. This may be slow, and ` +
-        `can be controlled using the pref "services.sync.${engine.name}.validation.enabled".\n` +
-        `If you encounter any problems because of this, please file a bug.`);
-
-      // Make a new flowID just incase we end up starting a repair.
-      let flowID = Utils.makeGUID();
-      try {
-        // XXX - must get the flow ID to either the validator, or directly to
-        // telemetry. I guess it's probably OK to always record a flowID even
-        // if we don't end up repairing?
-        log.info(`Running validator for ${engine.name}`);
-        let result = await validator.validate(engine);
-        Observers.notify("weave:engine:validate:finish", result, engine.name);
-        // And see if we should repair.
-        await this._maybeCure(engine, result, flowID);
-      } catch (ex) {
-        if (Async.isShutdownException(ex)) {
-          throw ex;
-        }
-        log.error(`Failed to run validation on ${engine.name}!`, ex);
-        Observers.notify("weave:engine:validate:error", ex, engine.name)
-        // Keep validating -- there's no reason to think that a failure for one
-        // validator would mean the others will fail.
-      }
-    }
-  },
-
-  _maybeCure(engine, validationResults, flowID) {
-    if (!this._shouldRepair(engine)) {
-      log.info(`Skipping repair of ${engine.name} - disabled via preferences`);
-      return;
-    }
-
-    let requestor = this._getRepairRequestor(engine.name);
-    let didStart = false;
-    if (requestor) {
-      didStart = requestor.startRepairs(validationResults, flowID);
-    }
-    log.info(`${didStart ? "did" : "didn't"} start a repair of ${engine.name} with flowID ${flowID}`);
-  },
-
-  _shouldRepair(engine) {
-    return Svc.Prefs.get(`engine.${engine.name}.repair.enabled`, false);
-  },
-
-  // mainly for mocking.
-  _fetchCollectionCounts() {
-    let collectionCountsURL = Service.userBaseURL + "info/collection_counts";
-    try {
-      let infoResp = Service._fetchInfo(collectionCountsURL);
-      if (!infoResp.success) {
-        log.error("Can't fetch collection counts: request to info/collection_counts responded with "
-                        + infoResp.status);
-        return {};
-      }
-      return infoResp.obj; // might throw because obj is a getter which parses json.
-    } catch (ex) {
-      if (Async.isShutdownException(ex)) {
-        throw ex;
-      }
-      // Not running validation is totally fine, so we just write an error log and return.
-      log.error("Caught error when fetching counts", ex);
-      return {};
-    }
-  },
-
-  get lastRepairAdvance() {
-    return Svc.Prefs.get("doctor.lastRepairAdvance", 0);
-  },
-
-  set lastRepairAdvance(value) {
-    Svc.Prefs.set("doctor.lastRepairAdvance", value);
-  },
-
-  // functions used so tests can mock them
-  _now() {
-    // We use the server time, which is SECONDS
-    return AsyncResource.serverTime;
-  },
-
-  _getRepairRequestor(name) {
-    return getRepairRequestor(name);
-  },
-
-  _getAllRepairRequestors() {
-    return getAllRepairRequestors();
-  }
-}
--- a/services/sync/modules/stages/enginesync.js
+++ b/services/sync/modules/stages/enginesync.js
@@ -13,18 +13,16 @@ var {utils: Cu} = Components;
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/policies.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://services-common/observers.js");
 Cu.import("resource://services-common/async.js");
 Cu.import("resource://gre/modules/Task.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "Doctor",
-                                  "resource://services-sync/doctor.js");
 
 /**
  * Perform synchronization of engines.
  *
  * This was originally split out of service.js. The API needs lots of love.
  */
 this.EngineSynchronizer = function EngineSynchronizer(service) {
   this._log = Log.repository.getLogger("Sync.Synchronizer");
@@ -191,17 +189,17 @@ EngineSynchronizer.prototype = {
           this.service.uploadMetaGlobal(meta);
           delete meta.isNew;
           delete meta.changed;
         } catch (error) {
           this._log.error("Unable to upload meta/global. Leaving marked as new.");
         }
       }
 
-      Async.promiseSpinningly(Doctor.consult(enginesToValidate));
+      Async.promiseSpinningly(this._tryValidateEngines(enginesToValidate));
 
       // If there were no sync engine failures
       if (this.service.status.service != SYNC_FAILED_PARTIAL) {
         Svc.Prefs.set("lastSync", new Date().toString());
         this.service.status.sync = SYNC_SUCCEEDED;
       }
     } finally {
       Svc.Prefs.reset("firstSync");
@@ -210,16 +208,116 @@ EngineSynchronizer.prototype = {
       let dateStr = Utils.formatTimestamp(new Date());
       this._log.info("Sync completed at " + dateStr
                      + " after " + syncTime + " secs.");
     }
 
     this.onComplete(null);
   },
 
+  _tryValidateEngines: Task.async(function* (recentlySyncedEngines) {
+    if (!Services.telemetry.canRecordBase || !Svc.Prefs.get("validation.enabled", false)) {
+      this._log.info("Skipping validation: validation or telemetry reporting is disabled");
+      return;
+    }
+
+    let lastValidation = Svc.Prefs.get("validation.lastTime", 0);
+    let validationInterval = Svc.Prefs.get("validation.interval");
+    let nowSeconds = Math.floor(Date.now() / 1000);
+
+    if (nowSeconds - lastValidation < validationInterval) {
+      this._log.info("Skipping validation: too recent since last validation attempt");
+      return;
+    }
+    // Update the time now, even if we may return false still. We don't want to
+    // check the rest of these more frequently than once a day.
+    Svc.Prefs.set("validation.lastTime", nowSeconds);
+
+    // Validation only occurs a certain percentage of the time.
+    let validationProbability = Svc.Prefs.get("validation.percentageChance", 0) / 100.0;
+    if (validationProbability < Math.random()) {
+      this._log.info("Skipping validation: Probability threshold not met");
+      return;
+    }
+    let maxRecords = Svc.Prefs.get("validation.maxRecords");
+    if (!maxRecords) {
+      // Don't bother asking the server for the counts if we know validation
+      // won't happen anyway.
+      return;
+    }
+
+    // maxRecords of -1 means "any number", so we can skip asking the server.
+    // Used for tests.
+    let info;
+    if (maxRecords < 0) {
+      info = {};
+      for (let e of recentlySyncedEngines) {
+        info[e.name] = 1; // needs to be < maxRecords
+      }
+      maxRecords = 2;
+    } else {
+
+      let collectionCountsURL = this.service.userBaseURL + "info/collection_counts";
+      try {
+        let infoResp = this.service._fetchInfo(collectionCountsURL);
+        if (!infoResp.success) {
+          this._log.error("Can't run validation: request to info/collection_counts responded with "
+                          + infoResp.status);
+          return;
+        }
+        info = infoResp.obj; // might throw because obj is a getter which parses json.
+      } catch (e) {
+        // Not running validation is totally fine, so we just write an error log and return.
+        this._log.error("Can't run validation: Caught error when fetching counts", e);
+        return;
+      }
+    }
+
+    if (!info) {
+      return;
+    }
+
+    let engineLookup = new Map(recentlySyncedEngines.map(e => [e.name, e]));
+    let toRun = [];
+    for (let [engineName, recordCount] of Object.entries(info)) {
+      let engine = engineLookup.get(engineName);
+      if (recordCount > maxRecords || !engine) {
+        this._log.debug(`Skipping validation for ${engineName} because it's not an engine or ` +
+                        `the number of records (${recordCount}) is greater than the maximum allowed (${maxRecords}).`);
+        continue;
+      }
+      let validator = engine.getValidator();
+      if (!validator) {
+        continue;
+      }
+      // Put this in an array so that we know how many we're going to do, so we
+      // don't tell users we're going to run some validators when we aren't.
+      toRun.push({ engine, validator });
+    }
+
+    if (!toRun.length) {
+      return;
+    }
+    Services.console.logStringMessage(
+      "Sync is about to run a consistency check. This may be slow, and " +
+      "can be controlled using the pref \"services.sync.validation.enabled\".\n" +
+      "If you encounter any problems because of this, please file a bug.");
+    for (let { validator, engine } of toRun) {
+      try {
+        let result = yield validator.validate(engine);
+        Observers.notify("weave:engine:validate:finish", result, engine.name);
+      } catch (e) {
+        this._log.error(`Failed to run validation on ${engine.name}!`, e);
+        Observers.notify("weave:engine:validate:error", e, engine.name)
+        // Keep validating -- there's no reason to think that a failure for one
+        // validator would mean the others will fail.
+      }
+    }
+  }),
+
   // Returns true if sync should proceed.
   // false / no return value means sync should be aborted.
   _syncEngine: function _syncEngine(engine) {
     try {
       engine.sync();
     } catch (e) {
       if (e.status == 401) {
         // Maybe a 401, cluster update perhaps needed?
--- a/services/sync/moz.build
+++ b/services/sync/moz.build
@@ -18,17 +18,16 @@ EXTRA_COMPONENTS += [
 
 EXTRA_JS_MODULES['services-sync'] += [
     'modules/addonsreconciler.js',
     'modules/addonutils.js',
     'modules/bookmark_validator.js',
     'modules/browserid_identity.js',
     'modules/collection_repair.js',
     'modules/collection_validator.js',
-    'modules/doctor.js',
     'modules/engines.js',
     'modules/keys.js',
     'modules/main.js',
     'modules/policies.js',
     'modules/record.js',
     'modules/resource.js',
     'modules/rest.js',
     'modules/service.js',
@@ -65,11 +64,11 @@ EXTRA_JS_MODULES['services-sync'].stages
 
 TESTING_JS_MODULES.services.sync += [
     'modules-testing/fakeservices.js',
     'modules-testing/fxa_utils.js',
     'modules-testing/rotaryengine.js',
     'modules-testing/utils.js',
 ]
 
-JS_PREFERENCE_PP_FILES += [
+JS_PREFERENCE_FILES += [
     'services-sync.js',
 ]
--- a/services/sync/services-sync.js
+++ b/services/sync/services-sync.js
@@ -60,26 +60,21 @@ pref("services.sync.log.logger.identity"
 pref("services.sync.log.cryptoDebug", false);
 
 pref("services.sync.fxa.termsURL", "https://accounts.firefox.com/legal/terms");
 pref("services.sync.fxa.privacyURL", "https://accounts.firefox.com/legal/privacy");
 
 pref("services.sync.telemetry.submissionInterval", 43200); // 12 hours in seconds
 pref("services.sync.telemetry.maxPayloadCount", 500);
 
-#ifndef RELEASE_OR_BETA
-// Enable the (fairly costly) client/server validation on nightly/aurora only.
-pref("services.sync.engine.bookmarks.validation.enabled", true);
-// Enable repair of bookmarks - requires validation also be enabled.
-pref("services.sync.engine.bookmarks.repair.enabled", true);
-#endif
+// Note that services.sync.validation.enabled is located in browser/app/profile/firefox.js
 
 // We consider validation this frequently. After considering validation, even
 // if we don't end up validating, we won't try again unless this much time has passed.
-pref("services.sync.engine.bookmarks.validation.interval", 86400); // 24 hours in seconds
+pref("services.sync.validation.interval", 86400); // 24 hours in seconds
 
 // We only run validation `services.sync.validation.percentageChance` percent of
 // the time, even if it's been the right amount of time since the last validation,
 // and you meet the maxRecord checks.
-pref("services.sync.engine.bookmarks.validation.percentageChance", 10);
+pref("services.sync.validation.percentageChance", 10);
 
 // We won't validate an engine if it has more than this many records on the server.
-pref("services.sync.engine.bookmarks.validation.maxRecords", 1000);
+pref("services.sync.validation.maxRecords", 1000);
--- a/services/sync/tests/unit/head_appinfo.js
+++ b/services/sync/tests/unit/head_appinfo.js
@@ -14,21 +14,21 @@ gSyncProfile = do_get_profile();
 // Init FormHistoryStartup and pretend we opened a profile.
 var fhs = Cc["@mozilla.org/satchel/form-history-startup;1"]
             .getService(Ci.nsIObserver);
 fhs.observe(null, "profile-after-change", null);
 
 // An app is going to have some prefs set which xpcshell tests don't.
 Services.prefs.setCharPref("identity.sync.tokenserver.uri", "http://token-server");
 
-// Set the validation prefs to attempt bookmark validation every time to avoid non-determinism.
-Services.prefs.setIntPref("services.sync.engine.bookmarks.validation.interval", 0);
-Services.prefs.setIntPref("services.sync.engine.bookmarks.validation.percentageChance", 100);
-Services.prefs.setIntPref("services.sync.engine.bookmarks.validation.maxRecords", -1);
-Services.prefs.setBoolPref("services.sync.engine.bookmarks.validation.enabled", true);
+// Set the validation prefs to attempt validation every time to avoid non-determinism.
+Services.prefs.setIntPref("services.sync.validation.interval", 0);
+Services.prefs.setIntPref("services.sync.validation.percentageChance", 100);
+Services.prefs.setIntPref("services.sync.validation.maxRecords", -1);
+Services.prefs.setBoolPref("services.sync.validation.enabled", true);
 
 // Make sure to provide the right OS so crypto loads the right binaries
 function getOS() {
   switch (mozinfo.os) {
     case "win":
       return "WINNT";
     case "mac":
       return "Darwin";
deleted file mode 100644
--- a/services/sync/tests/unit/test_doctor.js
+++ /dev/null
@@ -1,94 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-const { Doctor, REPAIR_ADVANCE_PERIOD } = Cu.import("resource://services-sync/doctor.js", {});
-
-initTestLogging("Trace");
-
-function mockDoctor(mocks) {
-  // Clone the object and put mocks in that.
-  return Object.assign({}, Doctor, mocks);
-}
-
-add_task(async function test_repairs_start() {
-  let repairStarted = false;
-  let problems = {
-    missingChildren: ["a", "b", "c"],
-  }
-  let validator = {
-    validate(engine) {
-      return problems;
-    }
-  }
-  let engine = {
-    name: "test-engine",
-    getValidator() {
-      return validator;
-    }
-  }
-  let requestor = {
-    startRepairs(validationInfo, flowID) {
-      ok(flowID, "got a flow ID");
-      equal(validationInfo, problems);
-      repairStarted = true;
-      return true;
-    }
-  }
-  let doctor = mockDoctor({
-    _getEnginesToValidate(recentlySyncedEngines) {
-      deepEqual(recentlySyncedEngines, [engine]);
-      return {
-        "test-engine": { engine, maxRecords: -1 }
-      };
-    },
-    _getRepairRequestor(engineName) {
-      equal(engineName, engine.name);
-      return requestor;
-    },
-    _shouldRepair(e) {
-      return true;
-    },
-  });
-  let promiseValidationDone = promiseOneObserver("weave:engine:validate:finish");
-  await doctor.consult([engine]);
-  await promiseValidationDone;
-  ok(repairStarted);
-});
-
-add_task(async function test_repairs_advanced_daily() {
-  let repairCalls = 0;
-  let requestor = {
-    continueRepairs() {
-      repairCalls++;
-    }
-  }
-  // start now at just after REPAIR_ADVANCE_PERIOD so we do a a first one.
-  let now = REPAIR_ADVANCE_PERIOD + 1;
-  let doctor = mockDoctor({
-    _getEnginesToValidate() {
-      return {}; // no validations.
-    },
-    _runValidators() {
-      // do nothing.
-    },
-    _getAllRepairRequestors() {
-      return {
-        foo: requestor,
-      }
-    },
-    _now() {
-      return now;
-    },
-  });
-  await doctor.consult();
-  equal(repairCalls, 1);
-  now += 10; // 10 seconds later...
-  await doctor.consult();
-  // should not have done another repair yet - it's too soon.
-  equal(repairCalls, 1);
-  // advance our pretend clock by the advance period (eg, day)
-  now += REPAIR_ADVANCE_PERIOD;
-  await doctor.consult();
-  // should have done another repair
-  equal(repairCalls, 2);
-});
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -141,17 +141,16 @@ tags = addons
 [test_bookmark_decline_undecline.js]
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_bookmark_tracker.js]
 requesttimeoutfactor = 4
 [test_bookmark_validator.js]
 [test_clients_engine.js]
 [test_clients_escape.js]
-[test_doctor.js]
 [test_extension_storage_engine.js]
 [test_extension_storage_tracker.js]
 [test_forms_store.js]
 [test_forms_tracker.js]
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_history_engine.js]
 [test_history_store.js]
--- a/tools/lint/eslint/modules.json
+++ b/tools/lint/eslint/modules.json
@@ -52,17 +52,16 @@
   "CrashManagerTest.jsm": ["configureLogging", "getManager", "sleep", "TestingCrashManager"],
   "dbg-client.jsm": ["DebuggerTransport", "DebuggerClient", "RootClient", "LongStringClient", "EnvironmentClient", "ObjectClient"],
   "dbg-server.jsm": ["DebuggerServer", "ActorPool", "OriginalLocation"],
   "debug.js": ["NS_ASSERT"],
   "declined.js": ["DeclinedEngines"],
   "dispatcher.js": ["Dispatcher"],
   "distribution.js": ["DistributionCustomizer"],
   "DNSTypes.jsm": ["DNS_QUERY_RESPONSE_CODES", "DNS_AUTHORITATIVE_ANSWER_CODES", "DNS_CLASS_CODES", "DNS_RECORD_TYPES"],
-  "doctor.js": ["Doctor"],
   "dom.js": ["getAttributes"],
   "DOMRequestHelper.jsm": ["DOMRequestIpcHelper"],
   "DownloadCore.jsm": ["Download", "DownloadSource", "DownloadTarget", "DownloadError", "DownloadSaver", "DownloadCopySaver", "DownloadLegacySaver", "DownloadPDFSaver"],
   "DownloadList.jsm": ["DownloadList", "DownloadCombinedList", "DownloadSummary"],
   "E10SAddonsRollout.jsm": ["isAddonPartOfE10SRollout"],
   "elementslib.js": ["ID", "Link", "XPath", "Selector", "Name", "Anon", "AnonXPath", "Lookup", "_byID", "_byName", "_byAttrib", "_byAnonAttrib"],
   "engines.js": ["EngineManager", "Engine", "SyncEngine", "Tracker", "Store", "Changeset"],
   "enginesync.js": ["EngineSynchronizer"],