Check the ETag when downloading to avoid unnecessary downloads (bug 326116) and fix remote ICS calendar creation to not throw an error (bug 315511); thanks to Michiel van Leeuwen <mvl+moz@exedo.nl> for the patches, r=dmose

calendar/providers/ics/calICSCalendar.js

@@ -142,7 +142,7 @@ calICSCalendar.prototype = {
 
     refresh: function() {
         // Lock other changes to the item list.
-        this.locked = true;
+        this.lock();
         // set to prevent writing after loading, without any changes
         this.loading = true;
 
@@ -163,10 +163,7 @@ calICSCalendar.prototype = {
             streamLoader.init(channel, this, this);
         } catch(e) {
             // File not found: a new calendar. No problem.
-            this.mObserver.onEndBatch();
-            this.mObserver.onLoad();
-            this.locked = false;
-            this.processQueue();
+            this.unlock();
         }
     },
 
@@ -180,8 +177,18 @@ calICSCalendar.prototype = {
 
     onStreamComplete: function(loader, ctxt, status, resultLength, result)
     {
+        // No need to do anything if there was no result
+        if (!resultLength) {
+            this.unlock();
+            return;
+        }
+        
         // Allow the hook to get needed data (like an etag) of the channel
-        this.mHooks.onAfterGet();
+        var cont = this.mHooks.onAfterGet();
+        if (!cont) {
+            this.unlock();
+            return;
+        }
 
         // Create a new calendar, to get rid of all the old events
         this.mMemoryCalendar = Components.classes["@mozilla.org/calendar/calendar;1?type=memory"]
@@ -209,12 +216,7 @@ calICSCalendar.prototype = {
         // Wrap parsing in a try block. Will ignore errors. That's a good thing
         // for non-existing or empty files, but not good for invalid files.
         // That's why we put them in readOnly mode
-        tryParse:
         try {
-            if (!str) {
-                break tryParse; // new file (empty), so nothing to parse
-            }
-
             var rootComp = this.mICSService.parseICS(str);
 
             var calComp;
@@ -312,15 +314,13 @@ calICSCalendar.prototype = {
             dump("Parsing the file failed:"+e+"\n");
             this.mObserver.onError(e.result, e.toString());
         }
-
         this.mObserver.onEndBatch();
         this.mObserver.onLoad();
-        this.locked = false;
-        this.processQueue();
+        this.unlock();
     },
 
     writeICS: function () {
-        this.locked = true;
+        this.lock();
 
         if (!this.mUri)
             throw Components.results.NS_ERROR_FAILURE;
@@ -434,8 +434,7 @@ calICSCalendar.prototype = {
         // Allow the hook to grab data of the channel, like the new etag
         ctxt.mHooks.onAfterPut(channel);
 
-        ctxt.locked = false;
-        ctxt.processQueue();
+        ctxt.unlock();
     },
 
     addObserver: function (aObserver) {
@@ -484,7 +483,7 @@ calICSCalendar.prototype = {
 
     processQueue: function ()
     {
-        if (this.locked)
+        if (this.isLocked())
             return;
         var a;
         var hasItems = this.queue.length;
@@ -506,6 +505,19 @@ calICSCalendar.prototype = {
             this.writeICS();
     },
 
+    lock: function () {
+        this.locked = true;
+    },
+
+    unlock: function () {
+        this.locked = false;
+        this.processQueue();
+    },
+    
+    isLocked: function () {
+        return this.locked;
+    },
+
     startBatch: function ()
     {
         this.mObserver.onStartBatch();
@@ -853,6 +865,12 @@ dummyHooks.prototype = {
         return true;
     },
     
+    /**
+     * @return
+     *     a boolean, false if the previous data should be used (the datastore
+     *     didn't change, there might be no data in this GET), true in all
+     *     other cases
+     */
     onAfterGet: function() {
         return true;
     },
@@ -873,11 +891,30 @@ function httpHooks() {
 httpHooks.prototype = {
     onBeforeGet: function(aChannel) {
         this.mChannel = aChannel;
+        if (this.mEtag) {
+            var httpchannel = aChannel.QueryInterface(Components.interfaces.nsIHttpChannel);
+            // Somehow the webdav header 'If' doesn't work on apache when
+            // passing in a Not, so use the http version here.
+            httpchannel.setRequestHeader("If-None-Match", this.mEtag, false);
+        }
+
         return true;
     },
     
     onAfterGet: function() {
         var httpchannel = this.mChannel.QueryInterface(Components.interfaces.nsIHttpChannel);
+
+        // 304: Not Modified
+        // Can use the old data, so tell the caller that it can skip parsing.
+        if (httpchannel.responseStatus == 304)
+            return false;
+
+        // 404: Not Found
+        // This is a new calendar. Shouldn't try to parse it. But it also
+        // isn't a failure, so don't throw.
+        if (httpchannel.responseStatus == 404)
+            return false;
+
         try {
             this.mEtag = httpchannel.getResponseHeader("ETag");
         } catch(e) {
@@ -891,6 +928,9 @@ httpHooks.prototype = {
     onBeforePut: function(aChannel) {
         if (this.mEtag) {
             var httpchannel = aChannel.QueryInterface(Components.interfaces.nsIHttpChannel);
+
+            // Apache doesn't work correctly with if-match on a PUT method,
+            // so use the webdav header
             httpchannel.setRequestHeader("If", '(['+this.mEtag+'])', false);
         }
         return true;
@@ -900,7 +940,6 @@ httpHooks.prototype = {
         var httpchannel = aChannel.QueryInterface(Components.interfaces.nsIHttpChannel);
         try {
             this.mEtag = httpchannel.getResponseHeader("ETag");
-            dump("new etag: "+this.mEtag+"\n");
         } catch(e) {
             // There was no ETag header on the response. This means that
             // putting is not atomic. This is bad. Race conditions can happen,