Reworked the cookie synchronization between cookie service, $browser and document.cookie.

Now we finally correctly handle situations when browser refuses to set a cookie, due to
storage quota or other (file:// protocol) limitations.

src/Browser.js

@@ -138,11 +138,10 @@ function Browser(location, document, head, XHR, $log) {
 
     if (name) {
       if (value === _undefined) {
-        delete lastCookies[name];
         rawDocument.cookie = escape(name) + "=;expires=Thu, 01 Jan 1970 00:00:00 GMT";
       } else {
         if (isString(value)) {
-          rawDocument.cookie = escape(name) + '=' + escape(lastCookies[name] = value);
+          rawDocument.cookie = escape(name) + '=' + escape(value);
 
           cookieLength = name.length + value.length + 1;
           if (cookieLength > 4096) {

src/services.js

@@ -405,52 +405,71 @@ angularService('$resource', function($xhr){
  * cookies are created or deleted from the browser at the end of the current eval.
  */
 angularService('$cookies', function($browser) {
-  var cookies = {},
-      rootScope = this,
-      lastCookies;
+  var rootScope = this,
+      cookies = {},
+      lastCookies = {},
+      lastBrowserCookies;
 
   //creates a poller fn that copies all cookies from the $browser to service & inits the service
   $browser.addPollFn(function() {
     var currentCookies = $browser.cookies();
-    if (lastCookies != currentCookies) {
-      lastCookies = currentCookies;
+    if (lastBrowserCookies != currentCookies) { //relies on browser.cookies() impl
+      lastBrowserCookies = currentCookies;
+      copy(currentCookies, lastCookies);
       copy(currentCookies, cookies);
       rootScope.$eval();
     }
   })();
 
   //at the end of each eval, push cookies
-  this.$onEval(PRIORITY_LAST, update);
+  this.$onEval(PRIORITY_LAST, push);
 
   return cookies;
 
-  function update(){
+
+  /**
+   * Pushes all the cookies from the service to the browser and verifies if all cookies were stored.
+   */
+  function push(){
     var name,
-        browserCookies = $browser.cookies();
+        browserCookies,
+        updated;
 
-    //$cookies -> $browser
-    for(name in cookies) {
-      if (cookies[name] !== browserCookies[name]) {
-        $browser.cookies(name, cookies[name]);
-      }
-    }
-
-    //get what was actually stored in the browser
-    browserCookies = $browser.cookies();
-
-    //$browser -> $cookies
-    for(name in browserCookies) {
+    //delete any cookies deleted in $cookies
+    for (name in lastCookies) {
       if (isUndefined(cookies[name])) {
         $browser.cookies(name, _undefined);
-      } else {
-        cookies[name] = browserCookies[name];
       }
     }
 
-    //drop cookies in $cookies for cookies that $browser or real browser dropped
-    for (name in cookies) {
-      if (isUndefined(browserCookies[name])) {
-        delete cookies[name];
+    //update all cookies updated in $cookies
+    for(name in cookies) {
+      if (cookies[name] !== lastCookies[name]) {
+        $browser.cookies(name, cookies[name]);
+        updated = true;
+      }
+    }
+
+    //verify what was actually stored
+    if (updated){
+      updated = !updated;
+      browserCookies = $browser.cookies();
+
+      for (name in cookies) {
+        if (cookies[name] !== browserCookies[name]) {
+          //delete or reset all cookies that the browser dropped from $cookies
+          if (isUndefined(browserCookies[name])) {
+            delete cookies[name];
+          } else {
+            cookies[name] = browserCookies[name];
+          }
+          updated = true;
+        }
+
+      }
+
+      if (updated) {
+        rootScope.$eval();
       }
     }
   }

test/BrowserSpecs.js

@@ -152,27 +152,39 @@ describe('browser', function(){
       });
 
       it('should log warnings when 4kb per cookie storage limit is reached', function() {
-        var i, longVal = '', cookieString;
+        var i, longVal = '', cookieStr;
 
         for(i=0; i<4092; i++) {
           longVal += '+';
         }
 
-        cookieString = document.cookie;
+        cookieStr = document.cookie;
         browser.cookies('x', longVal); //total size 4094-4096, so it should go through
-        expect(document.cookie).not.toEqual(cookieString);
+        expect(document.cookie).not.toEqual(cookieStr);
         expect(browser.cookies()['x']).toEqual(longVal);
         expect(logs.warn).toEqual([]);
 
-        browser.cookies('x', longVal + 'xxx') //total size 4097-4099, a warning should be logged
-        //browser behavior is undefined, so we test for existance of warning logs only
+        browser.cookies('x', longVal + 'xxx'); //total size 4097-4099, a warning should be logged
         expect(logs.warn).toEqual(
           [[ "Cookie 'x' possibly not set or overflowed because it was too large (4097 > 4096 " +
              "bytes)!" ]]);
+
+        //force browser to dropped a cookie and make sure that the cache is not out of sync
+        browser.cookies('x', 'shortVal');
+        expect(browser.cookies().x).toEqual('shortVal'); //needed to prime the cache
+        cookieStr = document.cookie;
+        browser.cookies('x', longVal + longVal + longVal); //should be too long for all browsers
+
+        if (document.cookie !== cookieStr) {
+          fail("browser didn't drop long cookie when it was expected. make the cookie in this " +
+              "test longer");
+        }
+
+        expect(browser.cookies().x).toEqual('shortVal');
       });
 
       it('should log warnings when 20 cookies per domain storage limit is reached', function() {
-        var i, str;
+        var i, str, cookieStr;
 
         for (i=0; i<20; i++) {
           str = '' + i;
@@ -185,12 +197,18 @@ describe('browser', function(){
         }
         expect(i).toEqual(20);
         expect(logs.warn).toEqual([]);
+        cookieStr = document.cookie;
 
         browser.cookies('one', 'more');
-        //browser behavior is undefined, so we test for existance of warning logs only
         expect(logs.warn).toEqual([]);
-      });
 
+        //if browser dropped a cookie (very likely), make sure that the cache is not out of sync
+        if (document.cookie === cookieStr) {
+          expect(size(browser.cookies())).toEqual(20);
+        } else {
+          expect(size(browser.cookies())).toEqual(21);
+        }
+      });
     });
 
 

test/angular-mocks.js

@@ -75,6 +75,7 @@ function MockBrowser() {
   };
 
   self.cookieHash = {};
+  self.lastCookieHash = {};
 }
 MockBrowser.prototype = {
 
@@ -103,12 +104,17 @@ MockBrowser.prototype = {
       if (value == undefined) {
         delete this.cookieHash[name];
       } else {
-        if (isString(value)) {
+        if (isString(value) &&       //strings only
+            value.length <= 4096) {  //strict cookie storage limits
           this.cookieHash[name] = value;
         }
       }
     } else {
-      return copy(this.cookieHash);
+      if (!equals(this.cookieHash, this.lastCookieHash)) {
+        this.lastCookieHash = copy(this.cookieHash);
+        this.cookieHash = copy(this.cookieHash);
+      }
+      return this.cookieHash;
     }
   }
 

test/servicesSpec.js

@@ -438,6 +438,7 @@ describe("service", function(){
     it('should remove a cookie when a $cookies property is deleted', function() {
       scope.$cookies.oatmealCookie = 'nom nom';
       scope.$eval();
+      scope.$browser.poll();
       expect(scope.$browser.cookies()).
         toEqual({'preexisting': 'oldCookie', 'oatmealCookie':'nom nom'});
 
@@ -446,6 +447,29 @@ describe("service", function(){
 
       expect(scope.$browser.cookies()).toEqual({'preexisting': 'oldCookie'});
     });
+
+
+    it('should drop or reset cookies that browser refused to store', function() {
+      var i, longVal;
+
+      for (i=0; i<5000; i++) {
+        longVal += '*';
+      }
+
+      //drop if no previous value
+      scope.$cookies.longCookie = longVal;
+      scope.$eval();
+      expect(scope.$cookies).toEqual({'preexisting': 'oldCookie'});
+
+
+      //reset if previous value existed
+      scope.$cookies.longCookie = 'shortVal';
+      scope.$eval();
+      expect(scope.$cookies).toEqual({'preexisting': 'oldCookie', 'longCookie': 'shortVal'});
+      scope.$cookies.longCookie = longVal;
+      scope.$eval();
+      expect(scope.$cookies).toEqual({'preexisting': 'oldCookie', 'longCookie': 'shortVal'});
+    });
   });