ampproject · dreamofabear · Dec 14, 2016 · Nov 16, 2016 · Nov 16, 2016 · Nov 16, 2016
diff --git a/builtins/amp-img.js b/builtins/amp-img.js
@@ -52,7 +52,7 @@ export class AmpImg extends BaseElement {
       this.srcset_ = srcsetFromElement(this.element);
       this.updateImageSrc_();
     } else if (this.img_ && ATTRIBUTES_TO_PROPAGATE_.indexOf(name) >= 0) {
-      this.propagateAttributes(name, this.img_);
+      this.propagateAttributes(name, this.img_, /* opt_removeMissingAttrs */ true);
     }
   }
 

diff --git a/examples/bind.amp.html b/examples/bind.amp.html
@@ -15,17 +15,17 @@
       background-color: red;
     }
   </style>
+</head>
 
+<body>
   <amp-state id="myState">
     <script type="application/json">
       {
         "myStateKey1": "myStateValue1"
       }
     </script>
-</amp-state>
-</head>
+  </amp-state>
 
-<body>
   <button onclick="AMP.toggleExperiment('amp-bind');window.location.href=window.location.href;">Toggle Experiment</button>
 
   <p [text]="foo">After clicking the button below, this will read 'foo'<p>

diff --git a/extensions/amp-bind/0.1/amp-state.js b/extensions/amp-bind/0.1/amp-state.js
@@ -41,10 +41,10 @@ export class AmpState extends AMP.BaseElement {
   buildCallback() {
     const TAG = this.getName_();
 
+    toggle(this.element, false);
     this.element.setAttribute('aria-hidden', 'true');
 
-    const id = this.element.id;
-    user().assert(id, '%s element must have an id.', TAG);
+    const id = user().assert(this.element.id, '%s element must have an id.', TAG);
 
     let json;
     const children = this.element.children;
@@ -73,12 +73,9 @@ export class AmpState extends AMP.BaseElement {
   }
 
   /** @override */
-  layoutCallback() {
-    // Now that we are rendered, stop rendering the element to reduce
-    // resource consumption.
-    toggle(this.element, false);
-
-    return Promise.resolve();
+  renderOutsideViewport() {
+    // We want the state data to be available wherever it is in the document.
+    return true;
   }
 
   /**

diff --git a/extensions/amp-bind/0.1/bind-impl.js b/extensions/amp-bind/0.1/bind-impl.js
@@ -17,17 +17,20 @@
 import {evaluateBindExpr} from './bind-expr';
 import {getMode} from '../../../src/mode';
 import {isExperimentOn} from '../../../src/experiments';
+import {isFiniteNumber} from '../../../src/types';
 import {dev, user} from '../../../src/log';
 import {vsyncFor} from '../../../src/vsync';
 
 const TAG = 'AMP-BIND';
 
 /**
  * A single binding, e.g. <element [property]="expression"></element>.
+ * `previousResult` is the result of this expression during the last digest.
  * @typedef {{
  *   property: !string,
  *   expression: !string,
- *   element: !Element
+ *   element: !Element,
+ *   previousResult: (BindExpressionResultDef|undefined)
  * }}
  */
 let BindingDef;
@@ -67,13 +70,6 @@ export class Bind {
     /** {!Array<BindingDef>} */
     this.bindings_ = [];
 
-    /**
-     * Expression results during the previous digest cycle, where the i'th
-     * element is the last result of `this.bindings_[i].expression`.
-     * @type {!Array<BindExpressionResultDef>}
-     */
-    this.previousResults_ = [];
-
     /** @const {!Object} */
     this.scope_ = Object.create(null);
 
@@ -86,6 +82,16 @@ export class Bind {
     /** @const {!Function} */
     this.boundMutate_ = this.mutate_.bind(this);
 
+    /**
+     * Keys correspond to valid attribute value types.
+     * @const {!Object<string,boolean>}
+     */
+    this.attributeValueTypes_ = {
+      'string': true,
+      'boolean': true,
+      'number': true,
+    };
+
     this.ampdoc.whenBodyAvailable().then(body => {
       this.bindings_ = this.scanForBindings_(body);
 
@@ -172,7 +178,7 @@ export class Bind {
     const task = {measure: this.boundMeasure_, mutate: this.boundMutate_};
     this.vsync_.run(task, {
       results: [],
-      verifyOnly: opt_verifyOnly,
+      verifyOnly: !!opt_verifyOnly,
     });
   }
 
@@ -199,10 +205,10 @@ export class Bind {
       const result = state.results[i];
 
       // Don't apply mutation if the result hasn't changed.
-      if (this.shallowEquals_(result, this.previousResults_[i])) {
+      if (this.shallowEquals_(result, binding.previousResult)) {
         continue;
       } else {
-        this.previousResults_[i] = result;
+        binding.previousResult = result;
       }
 
       if (state.verifyOnly) {
@@ -269,11 +275,11 @@ export class Bind {
         if (element.classList.contains('-amp-element')) {
           const resources = element.getResources();
           if (property === 'width') {
-            user().assert(typeof attributeValue === 'number',
+            user().assert(isFiniteNumber(attributeValue),
                 'Invalid result for [width]: %s', attributeValue);
             resources./*OK*/changeSize(element, undefined, attributeValue);
           } else if (property === 'height') {
-            user().assert(typeof attributeValue === 'number',
+            user().assert(isFiniteNumber(attributeValue),
                 'Invalid result for [height]: %s', attributeValue);
             resources./*OK*/changeSize(element, attributeValue, undefined);
           }
@@ -319,13 +325,14 @@ export class Bind {
       match = this.compareStringArrays_(initialValue, classes);
     } else {
       const attribute = element.getAttribute(property);
+      initialValue = attribute;
       if (typeof expectedValue === 'boolean') {
-        initialValue = !!attribute;
+        // Boolean attributes return values of either '' or null.
+        match = (expectedValue && initialValue === '')
+            || (!expectedValue && initialValue === null);
       } else {
-        initialValue = attribute;
+        match = (initialValue === expectedValue);
       }
-      // Use abstract equality since boolean attributes return value of ''.
-      match = (initialValue == expectedValue);
     }
 
     if (!match) {
@@ -363,8 +370,8 @@ export class Bind {
    * @return {(string|boolean|number|null)}
    */
   attributeValueOf_(value) {
-    if (typeof value === 'string' || typeof value === 'boolean'
-       || typeof value === 'number') {
+    const type = typeof(value);
+    if (this.attributeValueTypes_[type] !== undefined) {
       return value;
     } else {
       return null;

diff --git a/extensions/amp-bind/0.1/test/test-bind-impl.js b/extensions/amp-bind/0.1/test/test-bind-impl.js
@@ -76,7 +76,7 @@ describes.realWin('amp-bind', {
     });
   });
 
-  it('should verify initial values of bindings in dev mode', () => {
+  it('should verify initial values of string attribute bindings in dev mode', () => {
     env.sandbox.stub(window, 'AMP_MODE', {development: true});
     // Only the initial value for [a] binding does not match.
     createElementWithBinding('[a]="a" [b]="b" b="b"');
@@ -87,6 +87,17 @@ describes.realWin('amp-bind', {
     });
   });
 
+  it('should verify initial values of boolean attribute bindings in dev mode', () => {
+    env.sandbox.stub(window, 'AMP_MODE', {development: true});
+    // Only the initial value for [c] binding does not match.
+    createElementWithBinding(`a [a]="true" [b]="false" c="false" [c]="false"`);
+    const errorStub = env.sandbox.stub(user(), 'error').withArgs('AMP-BIND');
+    return bodyReady(unusedBody => {
+      env.flushVsync();
+      expect(errorStub.callCount).to.equal(1);
+    });
+  });
+
   it('should skip digest if specified in setState()', () => {
     const element = createElementWithBinding('[onePlusOne]="1+1"');
     expect(element.getAttribute('onePlusOne')).to.equal(null);

diff --git a/src/base-element.js b/src/base-element.js
@@ -511,18 +511,22 @@ export class BaseElement {
   /**
    * Utility method that propagates attributes from this element
    * to the given element.
-   * @param  {string|!Array<string>} attributes
-   * @param  {!Element} element
+   * If `opt_removeMissingAttrs` is true, then also removes any specified
+   * attributes that are missing on this element from the target element.
+   * @param {string|!Array<string>} attributes
+   * @param {!Element} element
+   * @param {boolean=} opt_removeMissingAttrs
    * @public @final
    */
-  propagateAttributes(attributes, element) {
+  propagateAttributes(attributes, element, opt_removeMissingAttrs) {
     attributes = isArray(attributes) ? attributes : [attributes];
     for (let i = 0; i < attributes.length; i++) {
       const attr = attributes[i];
-      if (!this.element.hasAttribute(attr)) {
-        continue;
+      if (this.element.hasAttribute(attr)) {
+        element.setAttribute(attr, this.element.getAttribute(attr));
+      } else if (opt_removeMissingAttrs) {
+        element.removeAttribute(attr);
       }
-      element.setAttribute(attr, this.element.getAttribute(attr));
     }
   }
 

diff --git a/src/service/resources-impl.js b/src/service/resources-impl.js
@@ -1353,8 +1353,8 @@ export class Resources {
     }
 
     if (request) {
-      request.newHeight = newHeight || request.newHeight;
-      request.newWidth = newWidth || request.newWidth;
+      request.newHeight = newHeight;
+      request.newWidth = newWidth;
       request.force = force || request.force;
       request.callback = opt_callback;
     } else {