Skip to content
This repository was archived by the owner on Mar 13, 2018. It is now read-only.

Commit 812e8a4

Browse files
committed
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
1 parent c007279 commit 812e8a4

File tree

2 files changed

+12
-11
lines changed

2 files changed

+12
-11
lines changed

src/observe.js

+12-7
Original file line numberDiff line numberDiff line change
@@ -535,8 +535,8 @@
535535
addToAll(this);
536536
this.callback_ = callback;
537537
this.target_ = target;
538-
this.state_ = OPENED;
539538
this.connect_();
539+
this.state_ = OPENED;
540540
return this.value_;
541541
},
542542

@@ -545,11 +545,11 @@
545545
return;
546546

547547
removeFromAll(this);
548-
this.state_ = CLOSED;
549548
this.disconnect_();
550549
this.value_ = undefined;
551550
this.callback_ = undefined;
552551
this.target_ = undefined;
552+
this.state_ = CLOSED;
553553
},
554554

555555
deliver: function() {
@@ -906,7 +906,6 @@
906906
if (this.state_ != UNOPENED && this.state_ != RESETTING)
907907
throw Error('Cannot add observers once started.');
908908

909-
observer.open(this.deliver, this);
910909
this.observed_.push(observerSentinel, observer);
911910
},
912911

@@ -939,11 +938,17 @@
939938
check_: function(changeRecords, skipChanges) {
940939
var oldValues;
941940
for (var i = 0; i < this.observed_.length; i += 2) {
942-
var pathOrObserver = this.observed_[i+1];
943941
var object = this.observed_[i];
944-
var value = object === observerSentinel ?
945-
pathOrObserver.discardChanges() :
946-
pathOrObserver.getValueFrom(object)
942+
var path = this.observed_[i+1];
943+
var value;
944+
if (object === observerSentinel) {
945+
var observable = path;
946+
value = this.state_ === UNOPENED ?
947+
observable.open(this.deliver, this) :
948+
observable.discardChanges();
949+
} else {
950+
value = path.getValueFrom(object);
951+
}
947952

948953
if (skipChanges) {
949954
this.value_[i / 2] = value;

tests/test.js

-4
Original file line numberDiff line numberDiff line change
@@ -1174,10 +1174,6 @@ suite('CompoundObserver Tests', function() {
11741174
function setValueFn(value) { return value / 2; }
11751175

11761176
var compound = new CompoundObserver;
1177-
assert.throws(function () {
1178-
compound.addObserver(1);
1179-
});
1180-
11811177
compound.addPath(model, 'a');
11821178
compound.addObserver(new ObserverTransform(new PathObserver(model, 'b'),
11831179
valueFn, setValueFn));

0 commit comments

Comments
 (0)