watchCollection: Return correct oldCollection argument to listener

src/ng/rootScope.js

@@ -416,79 +416,98 @@ function $RootScopeProvider(){
         var self = this;
         var oldValue;
         var newValue;
-        var changeDetected = 0;
+        var changeFlipFlop = 0;
         var objGetter = $parse(obj);
         var internalArray = [];
         var internalObject = {};
-        var oldLength = 0;
+        var internalLength = 0;
+
+        // Holds simple value or reference to internalArray or internalObject.
+        // The special initial value is used to ensure that the listener is called
+        // when the watch is established and that oldValue = newValue.
+        var internalValue = initWatchVal;
 
         function $watchCollectionWatch() {
+          var newLength, key, i, changeDetected;
+
           newValue = objGetter(self);
-          var newLength, key;
+          oldValue = internalValue;
+          changeDetected = 0;
 
           if (!isObject(newValue)) {
-            if (oldValue !== newValue) {
-              oldValue = newValue;
+            if (internalValue !== newValue) {
+              internalValue = newValue;
               changeDetected++;
             }
           } else if (isArrayLike(newValue)) {
-            if (oldValue !== internalArray) {
-              // we are transitioning from something which was not an array into array.
-              oldValue = internalArray;
-              oldLength = oldValue.length = 0;
-              changeDetected++;
-            }
-
             newLength = newValue.length;
-
-            if (oldLength !== newLength) {
-              // if lengths do not match we need to trigger change notification
+            if (internalValue !== internalArray) {
+              // we are transitioning from something which was not an array into array.
               changeDetected++;
-              oldValue.length = oldLength = newLength;
-            }
-            // copy the items to oldValue and look for changes.
-            for (var i = 0; i < newLength; i++) {
-              if (oldValue[i] !== newValue[i]) {
+            } else {
+              if (internalLength !== newLength) {
+                // if lengths do not match we need to trigger change notification
                 changeDetected++;
-                oldValue[i] = newValue[i];
+              } else {
+                // look for item changes
+                for (i = 0; i < newLength; i++) {
+                  if (internalValue[i] !== newValue[i]) {
+                    changeDetected++;
+                    break;
+                  }
+                }
+              }
+            }
+            if (changeDetected) {
+              // copy the items to array cache
+              internalValue = internalArray = [];
+              internalValue.length = internalLength = newLength;
+              for (i = 0; i < newLength; i++) {
+                internalValue[i] = newValue[i];
               }
             }
           } else {
-            if (oldValue !== internalObject) {
-              // we are transitioning from something which was not an object into object.
-              oldValue = internalObject = {};
-              oldLength = 0;
+            if (internalValue !== internalObject) {
+              // we are transitioning from something which was not an object into object
               changeDetected++;
-            }
-            // copy the items to oldValue and look for changes.
-            newLength = 0;
-            for (key in newValue) {
-              if (newValue.hasOwnProperty(key)) {
-                newLength++;
-                if (oldValue.hasOwnProperty(key)) {
-                  if (oldValue[key] !== newValue[key]) {
+            } else {
+              // look for item changes
+              newLength = 0;
+              for (key in newValue) {
+                if (newValue.hasOwnProperty(key)) {
+                  newLength++;
+                  if (! (internalValue.hasOwnProperty(key) &&
+                         internalValue[key] === newValue[key])) {
                     changeDetected++;
-                    oldValue[key] = newValue[key];
+                    break;
                   }
-                } else {
-                  oldLength++;
-                  oldValue[key] = newValue[key];
-                  changeDetected++;
                 }
               }
+              if (internalLength !== newLength) {
+                changeDetected++;
+              }
             }
-            if (oldLength > newLength) {
-              // we used to have more keys, need to find them and destroy them.
-              changeDetected++;
-              for(key in oldValue) {
-                if (oldValue.hasOwnProperty(key) && !newValue.hasOwnProperty(key)) {
-                  oldLength--;
-                  delete oldValue[key];
+            if (changeDetected) {
+              // copy the items to object cache
+              internalValue = internalObject = {};
+              internalLength = 0;
+              for (key in newValue) {
+                if (newValue.hasOwnProperty(key)) {
+                  internalLength++;
+                  internalValue[key] = newValue[key];
                 }
               }
             }
           }
-          return changeDetected;
+
+          if (changeDetected) {
+            changeFlipFlop = 1 - changeFlipFlop;
+            if (oldValue === initWatchVal) {
+              oldValue = newValue;
+            }
+          }
+
+          return changeFlipFlop;
         }
 
         function $watchCollectionAction() {

test/ng/rootScopeSpec.js

@@ -454,29 +454,44 @@ describe('Scope', function() {
       }));
     });
 
-
     describe('$watchCollection', function() {
-      var log, $rootScope, deregister;
+        var log, $rootScope, deregister, prevval, firstWatch, oldValueCorrect;
+
+        beforeEach(inject(function (_$rootScope_) {
+            log = [];
+            $rootScope = _$rootScope_;
+            firstWatch = true;
+            oldValueCorrect = true;
+            deregister = $rootScope.$watchCollection('obj', function listener(obj, oldobj) {
+                log.push(toJson(obj));
+                if (firstWatch) {
+                    firstWatch = false;
+                    if (toJson(obj) !== toJson(oldobj)) {
+                        oldValueCorrect = false;
+                    }
+                } else {
+                    if (toJson(oldobj) !== prevval) {
+                        oldValueCorrect = false;
+                    }
+                }
+                prevval = toJson(obj);
+            });
+        }));
 
-      beforeEach(inject(function(_$rootScope_) {
-        log = [];
-        $rootScope = _$rootScope_;
-        deregister = $rootScope.$watchCollection('obj', function logger(obj) {
-          log.push(toJson(obj));
+        afterEach(function () {
+            deregister();
         });
-      }));
 
-
-      it('should not trigger if nothing change', inject(function($rootScope) {
+      it('should not trigger if nothing change', function() {
         $rootScope.$digest();
         expect(log).toEqual([undefined]);
 
         $rootScope.$digest();
         expect(log).toEqual([undefined]);
-      }));
+      });
 
 
-      it('should allow deregistration', inject(function($rootScope) {
+      it('should allow deregistration', function() {
         $rootScope.obj = [];
         $rootScope.$digest();
 
@@ -487,7 +502,7 @@ describe('Scope', function() {
 
         $rootScope.$digest();
         expect(log).toEqual(['[]']);
-      }));
+      });
 
 
       describe('array', function() {
@@ -511,6 +526,9 @@ describe('Scope', function() {
           $rootScope.obj = undefined;
           $rootScope.$digest();
           expect(log).toEqual(['"test"', '[]', '{}', '[]', undefined]);
+
+          expect(oldValueCorrect).toBe(true);
+
         });
 
 
@@ -554,6 +572,9 @@ describe('Scope', function() {
           log = [];
           $rootScope.$digest();
           expect(log).toEqual([ '[{},[]]' ]);
+
+          expect(oldValueCorrect).toBe(true);
+
         });
 
         it('should watch array-like objects like arrays', function () {
@@ -571,6 +592,9 @@ describe('Scope', function() {
           $rootScope.arrayLikeObject =  document.getElementsByTagName('a');
           $rootScope.$digest();
           expect(arrayLikelog).toEqual(['x', 'y']);
+
+          expect(oldValueCorrect).toBe(true);
+
         });
       });
 
@@ -627,6 +651,9 @@ describe('Scope', function() {
           log = [];
           $rootScope.$digest();
           expect(log).toEqual([ '{"b":[],"c":"B"}' ]);
+
+          expect(oldValueCorrect).toBe(true);
+
         });
       });
     });