Simplify amp-state/bindReady race fix

extensions/amp-bind/0.1/bind-impl.js

@@ -186,15 +186,11 @@ export class Bind {
           return this.initialize_(root);
         });
 
-    /** @private {?Node} */
-    this.rootNode_ = null;
-
     /** @private {Promise} */
     this.setStatePromise_ = null;
 
     /** @private @const {!../../../src/utils/signals.Signals} */
     this.signals_ = new Signals();
-    this.signals_.whenSignal('READY').then(() => this.onReady_());
 
     // Install debug tools.
     const g = self.AMP;
@@ -234,7 +230,6 @@ export class Bind {
     dev().info(TAG, 'state:', this.state_);
 
     if (opt_skipEval) {
-      this.checkReadiness_();
       return Promise.resolve();
     }
 
@@ -454,8 +449,6 @@ export class Bind {
    * @private
    */
   initialize_(root) {
-    this.rootNode_ = root;
-
     // Disallow URL property bindings in AMP4EMAIL.
     const allowUrlProperties = !this.isAmp4Email_();
     this.validator_ = new BindValidator(allowUrlProperties);
@@ -475,25 +468,23 @@ export class Bind {
       if (getMode().development) {
         return this.evaluate_().then(results => this.verify_(results));
       }
-    }).then(() => this.checkReadiness_());
+    }).then(() => this.checkReadiness_(root));
   }
 
   /**
    * Bind is "ready" when its initialization completes _and_ all <amp-state>
    * elements' local data is parsed and processed (not remote data).
+   * @param {!Node} root
    * @private
    */
-  checkReadiness_() {
-    if (!this.rootNode_) {
-      return;
-    }
-    const unbuilt =
-        this.rootNode_.querySelectorAll('AMP-STATE.i-amphtml-notbuilt');
-    if (unbuilt.length > 0) {
-      return;
+  checkReadiness_(root) {
+    const ampStates = root.querySelectorAll('AMP-STATE');
+    if (ampStates.length > 0) {
+      const whenBuilt = toArray(ampStates).map(el => el.whenBuilt());
+      Promise.all(whenBuilt).then(() => this.onReady_());
+    } else {
+      this.onReady_();
     }
-    // Use a signal to ensure that onReady_() is only invoked once.
-    this.initializePromise_.then(() => this.signals_.signal('READY'));
   }
 
   /**

extensions/amp-bind/0.1/test/integration/test-bind-impl.js

@@ -293,39 +293,26 @@ describe.configure().ifChrome().run('Bind', function() {
     });
 
     it('should send "bindReady" to viewer on init', () => {
-      const signals = bind.signals();
-      sandbox.spy(signals, 'signal');
-
-      expect(signals.signal).to.not.be.called;
       expect(viewer.sendMessage).to.not.be.called;
 
       return onBindReady(env, bind).then(() => {
-        expect(signals.signal).to.be.calledWith('READY');
-        return signals.whenSignal('READY');
-      }).then(() => {
         expect(viewer.sendMessage).to.be.calledOnce;
         expect(viewer.sendMessage).to.be.calledWith('bindReady');
       });
     });
 
     it('should not send "bindReady" until all <amp-state> are built', () => {
       const element = createElement(env, container, '', 'amp-state', true);
-      element.classList.add('i-amphtml-notbuilt');
-
-      const signals = bind.signals();
-      sandbox.spy(signals, 'signal');
-
-      expect(signals.signal).to.not.be.called;
-      expect(viewer.sendMessage).to.not.be.called;
+      let buildAmpState;
+      const builtPromise = new Promise(resolve => {
+        buildAmpState = resolve;
+      });
+      element.whenBuilt = () => builtPromise;
 
       return onBindReady(env, bind).then(() => {
-        expect(signals.signal).to.not.be.called;
-
-        // Simulate <amp-state> buildCallback().
-        element.classList.remove('i-amphtml-notbuilt');
-        bind.setState({}, /* opt_skipEval */ true);
-
-        return signals.whenSignal('READY');
+        expect(viewer.sendMessage).to.not.be.called;
+        buildAmpState();
+        return element.whenBuilt();
       }).then(() => {
         expect(viewer.sendMessage).to.be.calledOnce;
         expect(viewer.sendMessage).to.be.calledWith('bindReady');
@@ -778,7 +765,9 @@ describe.configure().ifChrome().run('Bind', function() {
 
     it('should ignore <amp-state> updates if specified in setState()', () => {
       const element = createElement(env, container, '[src]="foo"', 'amp-state');
+      element.whenBuilt = () => Promise.resolve();
       expect(element.getAttribute('src')).to.be.null;
+
       const promise = onBindReadyAndSetState(env, bind,
           {foo: '/foo'}, /* opt_isAmpStateMutation */ true);
       return promise.then(() => {