From 812e8a499dbea6bcb830378c6250e755df32264f Mon Sep 17 00:00:00 2001
From: Rafael Weinstein <rafaelw@chromium.org>
Date: Tue, 8 Apr 2014 12:32:52 -0700
Subject: [PATCH] Avoid accessing observable value twice during setup of
 CompoundObserver

This was affecting polymer startup. The basic problem is that Observer.open must read the underlying value, and was called during addObserver, only shortly after to have check_ call discardChanges() -- which read the underlying value again.

The solution is to delay opening the Observable until the CompoundObserver is opened

R=arv
BUG=

Review URL: https://codereview.appspot.com/85570043
---
 src/observe.js | 19 ++++++++++++-------
 tests/test.js  |  4 ----
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/observe.js b/src/observe.js
index 4cad7cc..b1d134b 100644
--- a/src/observe.js
+++ b/src/observe.js
@@ -535,8 +535,8 @@
       addToAll(this);
       this.callback_ = callback;
       this.target_ = target;
-      this.state_ = OPENED;
       this.connect_();
+      this.state_ = OPENED;
       return this.value_;
     },
 
@@ -545,11 +545,11 @@
         return;
 
       removeFromAll(this);
-      this.state_ = CLOSED;
       this.disconnect_();
       this.value_ = undefined;
       this.callback_ = undefined;
       this.target_ = undefined;
+      this.state_ = CLOSED;
     },
 
     deliver: function() {
@@ -906,7 +906,6 @@
       if (this.state_ != UNOPENED && this.state_ != RESETTING)
         throw Error('Cannot add observers once started.');
 
-      observer.open(this.deliver, this);
       this.observed_.push(observerSentinel, observer);
     },
 
@@ -939,11 +938,17 @@
     check_: function(changeRecords, skipChanges) {
       var oldValues;
       for (var i = 0; i < this.observed_.length; i += 2) {
-        var pathOrObserver = this.observed_[i+1];
         var object = this.observed_[i];
-        var value = object === observerSentinel ?
-            pathOrObserver.discardChanges() :
-            pathOrObserver.getValueFrom(object)
+        var path = this.observed_[i+1];
+        var value;
+        if (object === observerSentinel) {
+          var observable = path;
+          value = this.state_ === UNOPENED ?
+              observable.open(this.deliver, this) :
+              observable.discardChanges();
+        } else {
+          value = path.getValueFrom(object);
+        }
 
         if (skipChanges) {
           this.value_[i / 2] = value;
diff --git a/tests/test.js b/tests/test.js
index 05fd1f4..7601631 100644
--- a/tests/test.js
+++ b/tests/test.js
@@ -1174,10 +1174,6 @@ suite('CompoundObserver Tests', function() {
     function setValueFn(value) { return value / 2; }
 
     var compound = new CompoundObserver;
-    assert.throws(function () {
-      compound.addObserver(1);
-    });
-
     compound.addPath(model, 'a');
     compound.addObserver(new ObserverTransform(new PathObserver(model, 'b'),
                                                valueFn, setValueFn));