Bug 1321396 - Skip oversized records without aborting the sync by default in sync engines. r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Wed, 30 Nov 2016 17:28:52 -0500
changeset 446108 f8d33e475480cbc3d8ee2a6a037a5bf67d1a3d5d
parent 446107 25a53a6b7aa16f57825487dbaa17302ab1a4fb21
child 538700 97aacc9f76090ebd24e3faf135b5224dda0e60ec
push id37694
push userbmo:tchiovoloni@mozilla.com
push dateWed, 30 Nov 2016 22:29:29 +0000
reviewersmarkh
bugs1321396
milestone53.0a1
Bug 1321396 - Skip oversized records without aborting the sync by default in sync engines. r?markh MozReview-Commit-ID: 7hgzyKb2UxY
services/sync/modules/engines.js
services/sync/modules/engines/bookmarks.js
services/sync/modules/engines/clients.js
services/sync/modules/engines/extension-storage.js
services/sync/modules/engines/forms.js
services/sync/modules/engines/history.js
services/sync/modules/engines/prefs.js
services/sync/tests/unit/test_syncengine_sync.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -648,17 +648,17 @@ Engine.prototype = {
   // Local 'constant'.
   // Signal to the engine that processing further records is pointless.
   eEngineAbortApplyIncoming: "error.engine.abort.applyincoming",
 
   // Should we keep syncing if we find a record that cannot be uploaded (ever)?
   // If this is false, we'll throw, otherwise, we'll ignore the record and
   // continue. This currently can only happen due to the record being larger
   // than the record upload limit.
-  allowSkippedRecord: false,
+  allowSkippedRecord: true,
 
   get prefName() {
     return this.name;
   },
 
   get enabled() {
     return Svc.Prefs.get("engine." + this.prefName, false);
   },
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -270,16 +270,17 @@ BookmarksEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _recordObj: PlacesItem,
   _storeObj: BookmarksStore,
   _trackerObj: BookmarksTracker,
   version: 2,
   _defaultSort: "index",
 
   syncPriority: 4,
+  allowSkippedRecord: false,
 
   // A diagnostic helper to get the string value for a bookmark's URL given
   // its ID. Always returns a string - on error will return a string in the
   // form of "<description of error>" as this is purely for, eg, logging.
   // (This means hitting the DB directly and we don't bother using a cached
   // statement - we should rarely hit this.)
   _getStringUrlForId(id) {
     let url;
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -76,16 +76,17 @@ this.ClientEngine = function ClientEngin
   // Reset the last sync timestamp on every startup so that we fetch all clients
   this.resetLastSync();
 }
 ClientEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: ClientStore,
   _recordObj: ClientsRec,
   _trackerObj: ClientsTracker,
+  allowSkippedRecord: false,
 
   // Always sync client data as it controls other sync behavior
   get enabled() {
     return true;
   },
 
   get lastRecordUpload() {
     return Svc.Prefs.get(this.name + ".lastRecordUpload", 0);
--- a/services/sync/modules/engines/extension-storage.js
+++ b/services/sync/modules/engines/extension-storage.js
@@ -36,16 +36,17 @@ this.ExtensionStorageEngine = function E
 ExtensionStorageEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _trackerObj: ExtensionStorageTracker,
   // we don't need these since we implement our own sync logic
   _storeObj: undefined,
   _recordObj: undefined,
 
   syncPriority: 10,
+  allowSkippedRecord: false,
 
   _sync: function () {
     return Async.promiseSpinningly(ExtensionStorageSync.syncAll());
   },
 
   get enabled() {
     // By default, we sync extension storage if we sync addons. This
     // lets us simplify the UX since users probably don't consider
--- a/services/sync/modules/engines/forms.js
+++ b/services/sync/modules/engines/forms.js
@@ -106,17 +106,16 @@ this.FormEngine = function FormEngine(se
   SyncEngine.call(this, "Forms", service);
 }
 FormEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: FormStore,
   _trackerObj: FormTracker,
   _recordObj: FormRec,
   applyIncomingBatchSize: FORMS_STORE_BATCH_SIZE,
-  allowSkippedRecord: true,
 
   syncPriority: 6,
 
   get prefName() {
     return "history";
   },
 
   _findDupe: function _findDupe(item) {
--- a/services/sync/modules/engines/history.js
+++ b/services/sync/modules/engines/history.js
@@ -37,17 +37,16 @@ this.HistoryEngine = function HistoryEng
 }
 HistoryEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _recordObj: HistoryRec,
   _storeObj: HistoryStore,
   _trackerObj: HistoryTracker,
   downloadLimit: MAX_HISTORY_DOWNLOAD,
   applyIncomingBatchSize: HISTORY_STORE_BATCH_SIZE,
-  allowSkippedRecord: true,
 
   syncPriority: 7,
 
   _processIncoming: function (newitems) {
     // We want to notify history observers that a batch operation is underway
     // so they don't do lots of work for each incoming record.
     let observers = PlacesUtils.history.getObservers();
     function notifyHistoryObservers(notification) {
--- a/services/sync/modules/engines/prefs.js
+++ b/services/sync/modules/engines/prefs.js
@@ -37,16 +37,17 @@ this.PrefsEngine = function PrefsEngine(
 PrefsEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: PrefStore,
   _trackerObj: PrefTracker,
   _recordObj: PrefRec,
   version: 2,
 
   syncPriority: 1,
+  allowSkippedRecord: false,
 
   getChangedIDs: function () {
     // No need for a proper timestamp (no conflict resolution needed).
     let changedIDs = {};
     if (this._tracker.modified)
       changedIDs[PREFS_GUID] = 0;
     return changedIDs;
   },
--- a/services/sync/tests/unit/test_syncengine_sync.js
+++ b/services/sync/tests/unit/test_syncengine_sync.js
@@ -1547,17 +1547,17 @@ add_task(async function test_uploadOutgo
 
 add_task(async function test_uploadOutgoing_largeRecords() {
   _("SyncEngine._uploadOutgoing throws on records larger than MAX_UPLOAD_BYTES");
 
   Service.identity.username = "foo";
   let collection = new ServerCollection();
 
   let engine = makeRotaryEngine();
-
+  engine.allowSkippedRecord = false;
   engine._store.items["large-item"] = "Y".repeat(MAX_UPLOAD_BYTES*2);
   engine._tracker.addChangedID("large-item", 0);
   collection.insert("large-item");
 
 
   let meta_global = Service.recordManager.set(engine.metaURL,
                                               new WBORecord(engine.metaURL));
   meta_global.payload.engines = {rotary: {version: engine.version,