Skip to content
This repository has been archived by the owner on Sep 20, 2019. It is now read-only.

Commit

Permalink
Merge pull request #665 from webcomponents/revert-644-541-memory-leak…
Browse files Browse the repository at this point in the history
…-ie11

Revert "fixed memory leak while importing html elements in IE"
  • Loading branch information
kevinpschaaf authored Jan 26, 2017
2 parents 0d6b57b + 708624f commit 0d285e7
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 56 deletions.
23 changes: 5 additions & 18 deletions src/CustomElements/observe.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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.
Expand Down
5 changes: 1 addition & 4 deletions src/HTMLImports/Observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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});
}

};
Expand Down
4 changes: 1 addition & 3 deletions src/HTMLImports/importer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions src/HTMLImports/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
Expand Down
5 changes: 0 additions & 5 deletions src/ShadowDOM/wrappers/Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},

Expand Down
13 changes: 4 additions & 9 deletions tests/HTMLImports/html/dynamic-all-imports-detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -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()
});
Expand Down
5 changes: 2 additions & 3 deletions tests/HTMLImports/html/dynamic-errors-detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion tests/HTMLImports/html/load-404.html
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
11 changes: 2 additions & 9 deletions tests/WebComponents/html/smoke.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
<x-foo>plain</x-foo>
<button is="x-baz">plain</button>
<script>
var isNativeShadowDomSupported;
test('smoke', function(done) {
isNativeShadowDomSupported = !!unwrap(document.createElement('div')).createShadowRoot;
var p = Object.create(HTMLElement.prototype);
p.createdCallback = function() {
this.textContent = 'custom!';
Expand Down Expand Up @@ -59,13 +57,8 @@
done();
}
var ob = new MutationObserver(handler);
if( isNativeShadowDomSupported ) {
ob.observe(root, {childList: true, subtree: true});
root.appendChild(xzot);
} else {
ob.observe(root.body, {childList: true, subtree: true});
root.body.appendChild(xzot);
}
ob.observe(root, {childList: true, subtree: true});
root.appendChild(xzot);
});
</script>
</body>
Expand Down

0 comments on commit 0d285e7

Please sign in to comment.