Merge pull request #4124 from blasten/import-href-listener

Fix #4123: Memory leak when using `importHref`
Polymer · Nov 2, 2016 · f24dea1 · f24dea1
2 parents 4be2e44 + 29de005
commit f24dea1
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 10 deletions.
diff --git a/src/standard/utils.html b/src/standard/utils.html
@@ -375,20 +375,22 @@
       var cached = list[link.href];
       var imprt = cached || link;
       var self = this;
+      var loadListener = function(e) {
+        e.target.__firedLoad = true;
+        e.target.removeEventListener('load', loadListener);
+        e.target.removeEventListener('error', errorListener);
+        return onload.call(self, e);
+      };
+      var errorListener = function(e) {
+        e.target.__firedError = true;
+        e.target.removeEventListener('load', loadListener);
+        e.target.removeEventListener('error', errorListener);
+        return onerror.call(self, e);
+      };
       if (onload) {
-        var loadListener = function(e) {
-          e.target.__firedLoad = true;
-          e.target.removeEventListener('load', loadListener);
-          return onload.call(self, e);
-        }
         imprt.addEventListener('load', loadListener);
       }
       if (onerror) {
-        var errorListener = function(e) {
-          e.target.__firedError = true;
-          e.target.removeEventListener('error', errorListener);
-          return onerror.call(self, e);
-        }
         imprt.addEventListener('error', errorListener);
       }
       // if already loaded/erroed, fire 'fake' load/error event

diff --git a/test/unit/dynamic-import.html b/test/unit/dynamic-import.html
@@ -65,6 +65,40 @@
         }, null, true);
       });
 
+      test('importHref does not leak event listeners on load', function(done) {
+        var errorSpy = sinon.spy();
+        var loadSpy = sinon.spy(function(e) {
+          var target = e.target;
+          target.dispatchEvent(new Event('error'));
+          assert.isFalse(errorSpy.called, 'doesn\'t trigger the error event listener');
+          setTimeout(function() {
+            loadSpy.reset();
+            target.dispatchEvent(new Event('load'));
+            assert.isFalse(loadSpy.called, 'doesn\'t trigger the load event listener');
+            done();
+          });
+        });
+
+        Polymer.Base.importHref('dynamic-imports/async-import.html', loadSpy, errorSpy, true);
+      });
+
+      test('importHref does not leak event listeners on error', function(done) {
+        var loadSpy = sinon.spy();
+        var errorSpy = sinon.spy(function(e) {
+          var target = e.target;
+          target.dispatchEvent(new Event('load'));
+          assert.isFalse(loadSpy.called, 'doesn\'t trigger the load event listener');
+          setTimeout(function() {
+            errorSpy.reset();
+            target.dispatchEvent(new Event('error'));
+            assert.isFalse(errorSpy.called, 'doesn\'t trigger the error event listener');
+            done();
+          });
+        });
+
+        Polymer.Base.importHref('dynamic-imports/async-import-invalid.html', loadSpy, errorSpy, true);
+      });
+
     });
 
   });