Bug 1148201 - Fix desktop reading list client use of Last-Modified, If-Unmodified-Since, If-Modified-Since headers. r=rnewman

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;
   }),