diff --git a/browser/components/readinglist/Sync.jsm b/browser/components/readinglist/Sync.jsm index 828dffcaed5c..54c1cfdec6f7 100644 --- a/browser/components/readinglist/Sync.jsm +++ b/browser/components/readinglist/Sync.jsm @@ -188,12 +188,7 @@ SyncImpl.prototype = { }, requests: requests, }, - headers: {}, }; - if (this._serverLastModifiedHeader) { - request.headers["If-Unmodified-Since"] = this._serverLastModifiedHeader; - } - let batchResponse = yield this._sendRequest(request); if (batchResponse.status != 200) { this._handleUnexpectedResponse("uploading changes", batchResponse); @@ -207,11 +202,10 @@ SyncImpl.prototype = { yield this._deleteItemForGUID(response.body.id); continue; } - if (response.status == 412 || response.status == 409) { - // 412 Precondition failed: The item was modified since the last sync. - // 409 Conflict: A change violated a uniqueness constraint. - // In either case, mark the item as having material changes, and - // reconcile and upload it in the material-changes phase. + if (response.status == 409) { + // "Conflict": A change violated a uniqueness constraint. Mark the item + // as having material changes, and reconcile and upload it in the + // material-changes phase. // TODO continue; } @@ -219,6 +213,10 @@ SyncImpl.prototype = { this._handleUnexpectedResponse("uploading a change", response); continue; } + // Don't assume the local record and the server record aren't materially + // different. Reconcile the differences. + // TODO + let item = yield this._itemForGUID(response.body.id); yield this._updateItemWithServerRecord(item, response.body); } @@ -255,12 +253,7 @@ SyncImpl.prototype = { }, requests: requests, }, - headers: {}, }; - if (this._serverLastModifiedHeader) { - request.headers["If-Unmodified-Since"] = this._serverLastModifiedHeader; - } - let batchResponse = yield this._sendRequest(request); if (batchResponse.status != 200) { this._handleUnexpectedResponse("uploading new items", batchResponse); @@ -323,12 +316,7 @@ SyncImpl.prototype = { }, requests: requests, }, - headers: {}, }; - if (this._serverLastModifiedHeader) { - request.headers["If-Unmodified-Since"] = this._serverLastModifiedHeader; - } - let batchResponse = yield this._sendRequest(request); if (batchResponse.status != 200) { this._handleUnexpectedResponse("uploading deleted items", batchResponse); @@ -337,13 +325,6 @@ SyncImpl.prototype = { // Delete local items based on the response. for (let response of batchResponse.body.responses) { - if (response.status == 412) { - // "Precondition failed": The item was modified since the last sync. - // Mark the item as having material changes, and reconcile and upload it - // in the material-changes phase. - // TODO - continue; - } // A 404 means the item was already deleted on the server, which is OK. // We still need to make sure it's deleted locally, though. if (response.status != 200 && response.status != 404) { @@ -370,18 +351,8 @@ SyncImpl.prototype = { let request = { method: "GET", path: path, - headers: {}, }; - if (this._serverLastModifiedHeader) { - request.headers["If-Modified-Since"] = this._serverLastModifiedHeader; - } - let response = yield this._sendRequest(request); - if (response.status == 304) { - // not modified - log.debug("No server changes"); - return; - } if (response.status != 200) { this._handleUnexpectedResponse("downloading modified items", response); return; @@ -417,6 +388,13 @@ SyncImpl.prototype = { {serverRecord, ex}); } } + + // Now that changes have been successfully applied, advance the server + // last-modified timestamp so that next time we fetch items starting from + // the current point. Response header names are lowercase. + if (response.headers && "last-modified" in response.headers) { + this._serverLastModifiedHeader = response.headers["last-modified"]; + } }), /** @@ -506,10 +484,6 @@ SyncImpl.prototype = { log.debug("Sending request", req); let response = yield this._client.request(req); log.debug("Received response", response); - // Response header names are lowercase. - if (response.headers && "last-modified" in response.headers) { - this._serverLastModifiedHeader = response.headers["last-modified"]; - } return response; }),