Merge pull request #3296 from Polymer/fix-3295

Fixes #3295. Only cache a false-y result for an element's owner shady…
Polymer · Jan 19, 2016 · 9cd6b79 · 9cd6b79
2 parents 848dbb9 + 0e74810
commit 9cd6b79
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 13 deletions.
diff --git a/src/lib/dom-api-shady.html b/src/lib/dom-api-shady.html
@@ -170,8 +170,8 @@
       if (!node) {
         return;
       }
-      if (node._ownerShadyRoot === undefined) {
-        var root;
+      var root = node._ownerShadyRoot;
+      if (root === undefined) {
         if (node._isShadyRoot) {
           root = node;
         } else {
@@ -183,9 +183,16 @@
            root = null;
           }
         }
-        node._ownerShadyRoot = root;
+        // memo-ize result for performance but only memo-ize a false-y
+        // result if node is in the document. This avoids a problem where a root
+        // can be cached while an element is inside a fragment. 
+        // If this happens and we cache the result, the value can become stale
+        // because for perf we avoid processing the subtree of added fragments.
+        if (root || document.documentElement.contains(node)) {
+          node._ownerShadyRoot = root;
+        }
       }
-      return node._ownerShadyRoot;
+      return root;
     },
 
     _maybeDistribute: function(node) {
@@ -345,7 +352,13 @@
 
     // TODO(sorvell): consider doing native QSA and filtering results.
     querySelector: function(selector) {
-      return this.querySelectorAll(selector)[0];
+      // match selector and halt on first result.
+      var result = this._query(function(n) {
+        return DomApi.matchesSelector.call(n, selector);
+      }, this.node, function(n) {
+        return Boolean(n);
+      })[0];
+      return result || null;
     },
 
     querySelectorAll: function(selector) {

diff --git a/src/lib/dom-api.html b/src/lib/dom-api.html
@@ -139,26 +139,34 @@
       // NOTE: `_query` is used primarily for ShadyDOM's querySelector impl,
       // but it's also generally useful to recurse through the element tree
       // and is used by Polymer's styling system. 
-      _query: function(matcher, node) {
+      _query: function(matcher, node, halter) {
         node = node || this.node;
         var list = [];
-        this._queryElements(TreeApi.Logical.getChildNodes(node), matcher, list);
+        this._queryElements(TreeApi.Logical.getChildNodes(node), matcher, 
+          halter, list);
         return list;
       },
 
-      _queryElements: function(elements, matcher, list) {
+      _queryElements: function(elements, matcher, halter, list) {
         for (var i=0, l=elements.length, c; (i<l) && (c=elements[i]); i++) {
           if (c.nodeType === Node.ELEMENT_NODE) {
-            this._queryElement(c, matcher, list);
+            if (this._queryElement(c, matcher, halter, list)) {
+              return true;
+            }
           }
         }
       },
 
-      _queryElement: function(node, matcher, list) {
-        if (matcher(node)) {
+      _queryElement: function(node, matcher, halter, list) {
+        var result = matcher(node);
+        if (result) {
           list.push(node);
         }
-        this._queryElements(TreeApi.Logical.getChildNodes(node), matcher, list);
+        if (halter && halter(result)) {
+          return result;
+        }
+        this._queryElements(TreeApi.Logical.getChildNodes(node), matcher, 
+          halter, list);
       }
 
     };

diff --git a/test/smoke/owner-root.html b/test/smoke/owner-root.html
@@ -0,0 +1,59 @@
+<!doctype html>
+<html>
+<head>
+
+  <title>early owner-root</title>
+
+  <meta charset="utf-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+  <script src="../../../webcomponentsjs/webcomponents-lite.js"></script>
+  <link rel="import" href="../../polymer.html">
+
+</head>
+<body>
+
+
+  <dom-module id="x-owner-root">
+    <template>where am I?</template>
+    <script>
+      Polymer({
+        is: 'x-owner-root',
+
+        properties: {
+          nug: {type: String, observer: '_nugChanged'}
+        },
+
+        _nugChanged: function() {
+          console.log(this.localName, Polymer.dom(this).getOwnerRoot());
+        }
+
+
+      });
+    </script>
+  </dom-module>
+
+  <dom-module id="x-test">
+    <template>
+      <template is="dom-if" if="{{foo}}">
+        <x-owner-root nug="foo"></x-owner-root>
+      </template>
+    </template>
+    <script>
+      Polymer({
+        is: 'x-test',
+        attached: function() {
+          this.foo = true;
+          Polymer.dom.flush();
+          var e = Polymer.dom(this.root).querySelector('x-owner-root');
+          var r = e && Polymer.dom(e).getOwnerRoot();
+          console.log(e, r, r === this.root);
+        }
+      });
+    </script>
+  </dom-module>
+
+  <x-test></x-test>
+
+</body>
+</html>
diff --git a/test/unit/polymer-dom.js b/test/unit/polymer-dom.js
@@ -11,7 +11,7 @@ suite('Polymer.dom', function() {
     var projected = Polymer.dom(testElement.root).querySelector('#projected');
     assert.equal(projected.textContent, 'projected');
     var p2 = Polymer.dom(testElement).querySelector('#projected');
-    assert.notOk(p2);
+    assert.isNull(p2);
     var rere = Polymer.dom(testElement.root).querySelector('x-rereproject');
     assert.equal(rere.is, 'x-rereproject');
     var re = Polymer.dom(rere.root).querySelector('x-reproject');
@@ -1207,6 +1207,24 @@ suite('Polymer.dom non-distributed elements', function() {
     assert.equal(Polymer.dom(test).getOwnerRoot(), c1.root, 'getOwnerRoot incorrect for child added to element in root');
   });
 
+  test('getOwnerRoot when out of tree and adding subtree', function() {
+    var container = document.createDocumentFragment();
+    var test = document.createElement('div');
+    container.appendChild(test);
+    assert.notOk(Polymer.dom(test).getOwnerRoot(), 'getOwnerRoot incorrect when not in root');
+    var c1 = document.createElement('x-compose');
+    var project = c1.$.project;
+    Polymer.dom(project).appendChild(container);
+    Polymer.dom.flush();
+    assert.equal(Polymer.dom(test).getOwnerRoot(), c1.root, 'getOwnerRoot incorrect for child added to element in root');
+    Polymer.dom(project).removeChild(test);
+    Polymer.dom.flush();
+    assert.notOk(Polymer.dom(test).getOwnerRoot(), 'getOwnerRoot incorrect for child moved from a root to no root');
+    Polymer.dom(project).appendChild(test);
+    Polymer.dom.flush();
+    assert.equal(Polymer.dom(test).getOwnerRoot(), c1.root, 'getOwnerRoot incorrect for child added to element in root');
+  });
+
   test('getOwnerRoot, subtree', function() {
     var test = document.createElement('div');
     var testChild = document.createElement('div');