Fix CompoundObserver for Object.observe when all deps are Observers

BUG= Review URL: https://codereview.appspot.com/38350045
googlearchive · Dec 12, 2013 · 04285a4 · 04285a4
1 parent 1515c4a
commit 04285a4
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 2 deletions.
diff --git a/src/observe.js b/src/observe.js
@@ -415,6 +415,9 @@
         return;
 
       if (hasObserve) {
+        if (this.deliverDeps_)
+          this.deliverDeps_();
+
         this.testingResults = testingResults;
         Object.deliverChangeRecords(this.boundInternalCallback_);
         this.testingResults = undefined;
@@ -762,6 +765,8 @@
 
     this.observed_ = [];
     this.value_ = [];
+    this.hasObservers_ = false;
+    this.depsChanged_ = undefined;
   }
 
   var observerSentinel = {};
@@ -785,13 +790,37 @@
       if (!isObservable(observer))
         throw Error('Object must be observable');
 
-      observer.open(this.deliver, this);
+      this.hasObservers_ = true;
+
+      observer.open(this.observerChanged_, this);
       var value = observer.value;
 
       this.observed_.push(observerSentinel, observer);
       this.value_.push(value);
     },
 
+    deliverDeps_: function() {
+      if (!this.hasObservers_)
+        return;
+
+      for (var i = 0; i < this.observed_.length; i = i + 2) {
+        if (this.observed_[i] === observerSentinel)
+          this.observed_[i + 1].deliver();
+      }
+    },
+
+    observerChanged_: function() {
+      if (!hasObserve)
+        return this.deliver();
+
+      if (!this.depsChanged_) {
+        this.depsChanged_ = {};
+        Object.observe(this.depsChanged_, this.boundInternalCallback_);
+      }
+
+      this.depsChanged_.changed = !this.depsChanged_.changed;
+    },
+
     getValues_: function(sync) {
       if (this.observedSet_)
         this.observedSet_.reset();
@@ -838,15 +867,21 @@
     },
 
     close: function() {
-      if (this.observed_) {
+      if (this.hasObservers_) {
         for (var i = 0; i < this.observed_.length; i += 2) {
           if (this.observed_[i] === observerSentinel)
             this.observed_[i + 1].close();
         }
+
         this.observed_ = undefined;
         this.value_ = undefined;
       }
 
+      if (this.depsChanged_) {
+        Object.unobserve(this.depsChanged_, this.boundInternalCallback_);
+        this.depsChanged_ = undefined;
+      }
+
       Observer.prototype.close.call(this);
     }
   });

diff --git a/tests/test.js b/tests/test.js
@@ -951,6 +951,34 @@ suite('CompoundObserver Tests', function() {
     observer.close();
   });
 
+  test('All Observers', function() {
+    function ident(value) { return value; }
+
+    var model = { a: 1, b: 2, c: 3 };
+
+    observer = new CompoundObserver();
+    observer.addObserver(new PathObserver(model, 'a'));
+    observer.addObserver(new PathObserver(model, 'b'));
+    observer.addObserver(new PathObserver(model, Path.get('c')));
+    observer.open(callback);
+
+    var observerCallbackArg = [model, Path.get('a'),
+                               model, Path.get('b'),
+                               model, Path.get('c')];
+    model.a = -10;
+    model.b = 20;
+    model.c = 30;
+    assertCompoundPathChanges([-10, 20, 30], [1, 2, 3],
+                              observerCallbackArg);
+
+    model.a = 'a';
+    model.c = 'c';
+    assertCompoundPathChanges(['a', 20, 'c'], [-10,, 30],
+                              observerCallbackArg);
+
+    observer.close();
+  });
+
   test('Degenerate Values', function() {
     var model = {};
     observer = new CompoundObserver();