From 708624f0c649932d9b1d19e63c0b73b0dd694a1b Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Thu, 26 Jan 2017 11:39:21 -0800 Subject: [PATCH] Revert "fixed memory leak while importing html elements in IE" --- src/CustomElements/observe.js | 23 ++++--------------- src/HTMLImports/Observer.js | 5 +--- src/HTMLImports/importer.js | 4 +--- src/HTMLImports/parser.js | 6 ++--- src/ShadowDOM/wrappers/Element.js | 5 ---- .../html/dynamic-all-imports-detail.html | 13 ++++------- .../html/dynamic-errors-detail.html | 5 ++-- tests/HTMLImports/html/load-404.html | 2 +- tests/WebComponents/html/smoke.html | 11 ++------- 9 files changed, 18 insertions(+), 56 deletions(-) diff --git a/src/CustomElements/observe.js b/src/CustomElements/observe.js index c34ab33b0..c0c742190 100644 --- a/src/CustomElements/observe.js +++ b/src/CustomElements/observe.js @@ -251,10 +251,7 @@ function takeRecords(node) { while (node.parentNode) { node = node.parentNode; } - - // The node is a ShadowRoot, an IE will have a memory leak if you put the observer - // directly on the ShadowRoot, so put it on the head so it does not leak - var observer = node.head.__observer; + var observer = node.__observer; if (observer) { handler(node, observer.takeRecords()); takeMutations(); @@ -263,29 +260,19 @@ function takeRecords(node) { var forEach = Array.prototype.forEach.call.bind(Array.prototype.forEach); + // observe a node tree; bail if it's already being observed. function observe(inRoot) { - - if (inRoot && inRoot.head && inRoot.head.__observer) { + if (inRoot.__observer) { return; } // For each ShadowRoot, we create a new MutationObserver, so the root can be // garbage collected once all references to the `inRoot` node are gone. // Give the handler access to the root so that an 'in document' check can // be done. - - // originally the observer was on the ShadowRoot (inRoot) (single observer); - // this causes a memory leak within IE. To fix this, we must put a an observer - // on both the head and body nodes on the ShadowRoot var observer = new MutationObserver(handler.bind(this, inRoot)); - observer.observe(inRoot.head, {childList: true, subtree: true}); - observer.observe(inRoot.body, {childList: true, subtree: true}); - - // this needs to be on head or it will leak in IE - // IE does not like it when you have non-standard attributes on root dom's, so put - // the observer on the head element - // this is used to check if the observer has been attached already (above) - inRoot.head.__observer = observer; + observer.observe(inRoot, {childList: true, subtree: true}); + inRoot.__observer = observer; } // upgrade an entire document and observe it for elements changes. diff --git a/src/HTMLImports/Observer.js b/src/HTMLImports/Observer.js index ffd6e4283..da8e47d4a 100644 --- a/src/HTMLImports/Observer.js +++ b/src/HTMLImports/Observer.js @@ -40,10 +40,7 @@ Observer.prototype = { }, observe: function(root) { - // IE will leak if you put an observer on a root shadow document - // so observe changes to both the head and body - this.mo.observe(root.head, {childList: true, subtree: true}); - this.mo.observe(root.body, {childList: true, subtree: true}); + this.mo.observe(root, {childList: true, subtree: true}); } }; diff --git a/src/HTMLImports/importer.js b/src/HTMLImports/importer.js index f28586cc5..81bd70352 100644 --- a/src/HTMLImports/importer.js +++ b/src/HTMLImports/importer.js @@ -74,9 +74,7 @@ var importer = { // generate an HTMLDocument from data doc = err ? null : makeDocument(resource, redirectedUrl || url); if (doc) { - // IE will leak if you put the node directly on the ShadowRoot (doc) - // instead appending to ShadowRoot head for reference - doc.head.__importLink = elt; + doc.__importLink = elt; // note, we cannot use MO to detect parsed nodes because // SD polyfill does not report these as mutations. this.bootDocument(doc); diff --git a/src/HTMLImports/parser.js b/src/HTMLImports/parser.js index 5ce8014e2..a52d9decb 100644 --- a/src/HTMLImports/parser.js +++ b/src/HTMLImports/parser.js @@ -161,10 +161,8 @@ var importParser = { rootImportForElement: function(elt) { var n = elt; - // IE will leak if you put the import link directly on the ownerDocument (shadow root) - // so the link is appended on the head element. - while (n.ownerDocument.head.__importLink) { - n = n.ownerDocument.head.__importLink; + while (n.ownerDocument.__importLink) { + n = n.ownerDocument.__importLink; } return n; }, diff --git a/src/ShadowDOM/wrappers/Element.js b/src/ShadowDOM/wrappers/Element.js index 63979e397..0fc646c80 100644 --- a/src/ShadowDOM/wrappers/Element.js +++ b/src/ShadowDOM/wrappers/Element.js @@ -76,11 +76,6 @@ var renderer = scope.getRendererForHost(this); renderer.invalidate(); - // This is needed for IE - if you put elements directly on the root shadow - // IE will leak, create head and body so we can append observers, etc. - newShadowRoot.head = document.createElement('head'); - newShadowRoot.body = document.createElement('body'); - return newShadowRoot; }, diff --git a/tests/HTMLImports/html/dynamic-all-imports-detail.html b/tests/HTMLImports/html/dynamic-all-imports-detail.html index 91e0d1ca7..603e7cf3c 100644 --- a/tests/HTMLImports/html/dynamic-all-imports-detail.html +++ b/tests/HTMLImports/html/dynamic-all-imports-detail.html @@ -46,16 +46,11 @@ test('HTMLImports whenready detail', function(done) { HTMLImports.whenReady(function(detail) { + chai.assert.equal(detail.allImports.length, 2); + chai.assert.equal(detail.loadedImports.length, 2); - var allImports = document.querySelectorAll('link[rel="import"]'); - - chai.assert.equal(detail.allImports.length, allImports.length); - chai.assert.equal(detail.loadedImports.length, allImports.length); - - var importsArray = Array.prototype.slice.call(detail.allImports); - - chai.expect(importsArray.filter(function(el){ return el.href.indexOf('imports/load-1.html') >= 0;})); - chai.expect(importsArray.filter(function(el){ return el.href.indexOf('imports/load-2.html') >= 0;})); + chai.expect(detail.allImports[0].href).to.contain('imports/load-1.html'); + chai.expect(detail.allImports[1].href).to.contain('imports/load-2.html'); done() }); diff --git a/tests/HTMLImports/html/dynamic-errors-detail.html b/tests/HTMLImports/html/dynamic-errors-detail.html index 11bd995fa..d93a0e605 100644 --- a/tests/HTMLImports/html/dynamic-errors-detail.html +++ b/tests/HTMLImports/html/dynamic-errors-detail.html @@ -46,10 +46,9 @@ test('HTMLImports whenready errors', function(done) { HTMLImports.whenReady(function(detail) { - var allImports = document.querySelectorAll('link[rel="import"]'); - chai.assert.equal(detail.allImports.length, allImports.length); + chai.assert.equal(detail.allImports.length, 2); chai.assert.equal(detail.errorImports.length, 1); - chai.assert.equal(detail.loadedImports.length, allImports.length - 1); + chai.assert.equal(detail.loadedImports.length, 1); var errorImport = detail.errorImports[0]; chai.expect(errorImport.href).to.contain('imports/load-does-not-exist.html'); diff --git a/tests/HTMLImports/html/load-404.html b/tests/HTMLImports/html/load-404.html index dfa30c47d..b475da873 100644 --- a/tests/HTMLImports/html/load-404.html +++ b/tests/HTMLImports/html/load-404.html @@ -40,7 +40,7 @@ function check() { clearTimeout(timeout); - chai.assert.equal(document.querySelector('link[href="imports/404-1.html"]').import, null, '404\'d link.import is null'); + chai.assert.equal(document.querySelector('link').import, null, '404\'d link.import is null'); chai.assert.equal(errors, 2, '404\'d generate error event'); done(); } diff --git a/tests/WebComponents/html/smoke.html b/tests/WebComponents/html/smoke.html index 7582dd458..dca9d7671 100644 --- a/tests/WebComponents/html/smoke.html +++ b/tests/WebComponents/html/smoke.html @@ -20,9 +20,7 @@ plain