From bc7109928ba8984aa441f709217bfd7481dce5cc Mon Sep 17 00:00:00 2001
From: Kevin Schaaf <kschaaf@google.com>
Date: Mon, 22 Jun 2015 19:39:29 -0700
Subject: [PATCH 1/2] Coerce undefined to default when binding. Fixes #1742.

---
 src/standard/configure.html         | 22 +++++++++++++++-------
 src/standard/effectBuilder.html     |  4 ++++
 test/unit/notify-path-elements.html | 17 ++++++++++-------
 test/unit/notify-path.html          | 13 +++++++++++++
 4 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/src/standard/configure.html b/src/standard/configure.html
index 467ce3be36..ddc9f8b00e 100644
--- a/src/standard/configure.html
+++ b/src/standard/configure.html
@@ -91,18 +91,26 @@
 
     _configureProperties: function(properties, config) {
       for (var i in properties) {
-        var c = properties[i];
-        if (c.value !== undefined) {
-          var value = c.value;
-          if (typeof value == 'function') {
-            // pass existing config values (this._config) to value function
-            value = value.call(this, this._config);
-          }
+        var value = this._defaultValueFromInfo(properties[i]);
+        if (value !== undefined) {
           config[i] = value;
         }
       }
     },
 
+    _defaultValueFromInfo: function(info) {
+      var value = info && info.value;
+      if (typeof value == 'function') {
+        // pass existing config values (this._config) to value function
+        value = value.call(this, this._config);
+      }
+      return value;
+    },
+
+    _defaultValueForProp: function(prop) {
+      return this._defaultValueFromInfo(this.getPropertyInfo(prop));
+    },
+
     _mixinConfigure: function(a, b) {
       for (var prop in b) {
         if (!this.getPropertyInfo(prop).readOnly) {
diff --git a/src/standard/effectBuilder.html b/src/standard/effectBuilder.html
index cf34c1feb2..65fa70da5e 100644
--- a/src/standard/effectBuilder.html
+++ b/src/standard/effectBuilder.html
@@ -263,6 +263,10 @@
             (node.localName == 'input' && property == 'value')) {
           value = value == undefined ? '' : value;
         }
+        // coerce undefined back to default
+        if (value === undefined && node._defaultValueForProp) {
+          value = node._defaultValueForProp(property);
+        }
         // TODO(kschaaf): Ideally we'd use `fromAbove: true`, but this
         // breaks read-only properties
         // this._setProperty(property, value, true, node);
diff --git a/test/unit/notify-path-elements.html b/test/unit/notify-path-elements.html
index 2d386d01d5..76769a33a2 100644
--- a/test/unit/notify-path-elements.html
+++ b/test/unit/notify-path-elements.html
@@ -5,6 +5,10 @@
       notifyingValue: {
         type: Number,
         notify: true
+      },
+      valueWithDefault: {
+        type: Number,
+        value: 99
       }
     }
   });
@@ -50,7 +54,7 @@
       },
       objValueChanged: function(value) {
         this.observerCounts.objValueChanged++;
-        assert.equal(this.obj.value, value);
+        assert.equal(this.obj && this.obj.value, value);
       },
     });
   </script>
@@ -95,7 +99,7 @@
       },
       objValueChanged: function(value) {
         this.observerCounts.objValueChanged++;
-        assert.equal(this.obj.value, value);
+        assert.equal(this.obj && this.obj.value, value);
       },
     });
   </script>
@@ -103,7 +107,7 @@
 
 <dom-module id="x-stuff">
   <template>
-    <x-basic id="basic" notifying-value="{{nested.obj.value}}" attrvalue$="{{nested.obj.value}}"></x-basic>
+    <x-basic id="basic" notifying-value="{{nested.obj.value}}" value-with-default="{{nested.obj.value}}" attrvalue$="{{nested.obj.value}}"></x-basic>
     <x-compose id="compose" obj="{{nested.obj}}"></x-compose>
     <x-forward id="forward" obj="{{nested.obj}}"></x-forward>
     <div id="boundChild" computed-from-paths="{{computeFromPaths(a, nested.b, nested.obj.c)}}" array-length="{{data.length}}"></div>
@@ -157,11 +161,11 @@
       },
       nestedObjChanged: function(value) {
         this.observerCounts.nestedObjChanged++;
-        assert.equal(this.nested.obj, value);
+        assert.equal(this.nested && this.nested.obj, value);
       },
       nestedObjSubpathChanged: function(change) {
         this.observerCounts.nestedObjSubpathChanged++;
-        assert.equal(change.base, this.nested.obj);
+        assert.equal(change.base, this.nested && this.nested.obj);
         if (this.expectedNestedObjSubpath) {
           assert.equal(change.path, this.expectedNestedObjSubpath);
           assert.equal(change.value, this.expectedNestedObjValue);
@@ -175,12 +179,11 @@
         this.observerCounts.multipleChanged++;
         assert.equal(a, 'a');
         assert.equal(b, 'b');
-        assert.equal(nestedObjChange.base, this.nested.obj);
+        assert.equal(nestedObjChange.base, this.nested && this.nested.obj);
         if (this.expectedNestedObjSubpath) {
           assert.equal(nestedObjChange.path, this.expectedNestedObjSubpath);
           assert.equal(nestedObjChange.value, this.expectedNestedObjValue);
         }
-        assert.equal(nestedObjChange.base, this.nested.obj);
       },
       computeFromPaths: function(a, b, c) {
         assert.equal(a, this.a, 'computeFromNested `a` arg incorrect');
diff --git a/test/unit/notify-path.html b/test/unit/notify-path.html
index 2a0ac95a3e..b215e99960 100644
--- a/test/unit/notify-path.html
+++ b/test/unit/notify-path.html
@@ -75,6 +75,19 @@
     assert.equal(el.$.forward.$.compose.$.basic2.getAttribute('attrvalue'), '42');
   });
 
+  test('reset to default on null', function() {
+    // Setup
+    var nested = {
+      obj: {
+        value: 42
+      }
+    };
+    el.nested = nested;
+    assert.equal(el.$.basic.valueWithDefault, 42);
+    el.nested = null;
+    assert.equal(el.$.basic.valueWithDefault, 99);
+  });
+
   test('notification from basic element property change', function() {
     // Setup
     var nested = {

From ca2272ef2cc060808ee492bef106a1bac8de5b0d Mon Sep 17 00:00:00 2001
From: Kevin Schaaf <kschaaf@google.com>
Date: Tue, 23 Jun 2015 18:49:29 -0700
Subject: [PATCH 2/2] Use resetProperty API, and optimize default config
 lookup.

---
 src/lib/bind/accessors.html     |  4 +--
 src/standard/configure.html     | 63 +++++++++++++++++----------------
 src/standard/effectBuilder.html | 39 +++++++++++---------
 3 files changed, 57 insertions(+), 49 deletions(-)

diff --git a/src/lib/bind/accessors.html b/src/lib/bind/accessors.html
index 5793c2ca24..24cfdf4149 100644
--- a/src/lib/bind/accessors.html
+++ b/src/lib/bind/accessors.html
@@ -73,11 +73,11 @@
       // TODO(kschaaf): downward bindings (e.g. _applyEffectValue) should also
       // use non-notifying setters but right now that would require looking
       // up readOnly property config in the hot-path
-      _setProperty: function(property, value, quiet, node) {
+      _setProperty: function(property, value, fromAbove, node) {
         node = node || this;
         var effects = node._propertyEffects && node._propertyEffects[property];
         if (effects) {
-          node._propertySetter(property, value, effects, quiet);
+          node._propertySetter(property, value, effects, fromAbove);
         } else {
           node[property] = value;
         }
diff --git a/src/standard/configure.html b/src/standard/configure.html
index ddc9f8b00e..47155dd7b2 100644
--- a/src/standard/configure.html
+++ b/src/standard/configure.html
@@ -45,18 +45,20 @@
 
     // storage for configuration
     _setupConfigure: function(initialConfig) {
-      this._config = initialConfig || {};
+      this._inputConfig = initialConfig || {};
+      this._defaultConfig = {};
+      this._config = null;
       this._handlers = [];
     },
 
-    // static attributes are deserialized into _config
+    // static attributes are deserialized into _inputConfig
     _marshalAttributes: function() {
-      this._takeAttributesToModel(this._config);
+      this._takeAttributesToModel(this._inputConfig);
     },
 
-    // at configure time values are stored in _config
+    // at configure time values are stored in _inputConfig
     _configValue: function(name, value) {
-      this._config[name] = value;
+      this._inputConfig[name] = value;
     },
 
     // Override polymer-mini thunk
@@ -71,44 +73,43 @@
       // e.g. hand template content stored in notes to children as part of
       // configure flow so templates have their content at ready time
       this._configureAnnotationReferences();
-      // save copy of configuration that came from above
-      this._aboveConfig = this.mixin({}, this._config);
-      // get individual default values from property configs
-      var config = {};
-      // mixed-in behaviors
+      // get individual default values from `value` in property configs
       this.behaviors.forEach(function(b) {
-        this._configureProperties(b.properties, config);
+        // mixed-in behaviors
+        this._configureProperties(b.properties, this._defaultConfig);
       }, this);
       // prototypical behavior
-      this._configureProperties(this.properties, config);
-      // override local configuration with configuration from above
-      this._mixinConfigure(config, this._aboveConfig);
+      this._configureProperties(this.properties, this._defaultConfig);
+      // override default configuration with configuration from above
       // this is the new _config, which are the final values to be applied
-      this._config = config;
+      this._config = Object.create(this._defaultConfig);
+      this._mixinConfigure(this._config, this._inputConfig);
       // pass configuration data to bindings
       this._distributeConfig(this._config);
     },
 
     _configureProperties: function(properties, config) {
       for (var i in properties) {
-        var value = this._defaultValueFromInfo(properties[i]);
-        if (value !== undefined) {
+        var c = properties[i];
+        if (c.value !== undefined) {
+          var value = c.value;
+          if (typeof value == 'function') {
+            // pass existing config values (this._config) to value function
+            value = value.call(this, this._config);
+          }
           config[i] = value;
         }
       }
     },
 
-    _defaultValueFromInfo: function(info) {
-      var value = info && info.value;
-      if (typeof value == 'function') {
-        // pass existing config values (this._config) to value function
-        value = value.call(this, this._config);
-      }
-      return value;
-    },
-
-    _defaultValueForProp: function(prop) {
-      return this._defaultValueFromInfo(this.getPropertyInfo(prop));
+    /**
+     * Resets a property to its default if it exists, otherwise `undefined`.
+     *
+     * @method resetProperty
+     * @param {string} property Property to reset.
+     */
+    resetProperty: function(property) {
+      this[property] = this._defaultConfig[property];
     },
 
     _mixinConfigure: function(a, b) {
@@ -146,21 +147,21 @@
     _afterClientsReady: function() {
       // process static effects, e.g. computations that have only literal arguments
       this._executeStaticEffects();
-      this._applyConfig(this._config, this._aboveConfig);
+      this._applyConfig(this._config, this._inputConfig);
       this._flushHandlers();
     },
 
     // NOTE: values are already propagated to children via
     // _distributeConfig so propagation triggered by effects here is
     // redundant, but safe due to dirty checking
-    _applyConfig: function(config, aboveConfig) {
+    _applyConfig: function(config, inputConfig) {
       for (var n in config) {
         // Don't stomp on values that may have been set by other side effects
         if (this[n] === undefined) {
           // Call _propertySet for any properties with accessors, which will
           // initialize read-only properties also; set quietly if value was
           // configured from above, as opposed to default
-          this._setProperty(n, config[n], n in aboveConfig);
+          this._setProperty(n, config[n], n in inputConfig);
         }
       }
     },
diff --git a/src/standard/effectBuilder.html b/src/standard/effectBuilder.html
index 65fa70da5e..8dc2acfb5e 100644
--- a/src/standard/effectBuilder.html
+++ b/src/standard/effectBuilder.html
@@ -253,24 +253,31 @@
       if (info.kind == 'attribute') {
         this.serializeValueToAttribute(value, property, node);
       } else {
-        // TODO(sorvell): consider pre-processing the following two string
-        // comparisons in the hot path so this can be a boolean check
-        if (property === 'className') {
-          value = this._scopeElementClass(node, value);
-        }
-        // Some browsers serialize `undefined` to `"undefined"`
-        if (property === 'textContent' ||
-            (node.localName == 'input' && property == 'value')) {
-          value = value == undefined ? '' : value;
-        }
         // coerce undefined back to default
-        if (value === undefined && node._defaultValueForProp) {
-          value = node._defaultValueForProp(property);
+        // TODO(kschaaf): coersion could be done in one of three places:
+        // 1. property setter (any propety set to undefined bounces to default)
+        // 2. all bindings (when binding, coerce undefined to the node's default)
+        // 3. only path bindings (where undefined most likely & problematic)
+        // we choose (3) because it is the most limited change
+        if (value === undefined && (info.model != property) &&
+          node.resetProperty) {
+          node.resetProperty(property);
+        } else {
+          // TODO(sorvell): consider pre-processing the following two string
+          // comparisons in the hot path so this can be a boolean check
+          if (property === 'className') {
+            value = this._scopeElementClass(node, value);
+          }
+          // Some browsers serialize `undefined` to `"undefined"`
+          if (property === 'textContent' ||
+              (node.localName == 'input' && property == 'value')) {
+            value = value == undefined ? '' : value;
+          }
+          // TODO(kschaaf): Ideally we'd use `fromAbove: true`, but this
+          // breaks read-only properties
+          // this._setPropertyQuiet(property, value, node);
+          node[property] = value;
         }
-        // TODO(kschaaf): Ideally we'd use `fromAbove: true`, but this
-        // breaks read-only properties
-        // this._setProperty(property, value, true, node);
-        return node[property] = value;
       }
     },