From 192c9d697ed0276428ad5f3a7f544b0edbacfaee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Lemoine?= Date: Mon, 30 Mar 2015 16:29:30 +0200 Subject: [PATCH 1/2] Unregister the viewport listener on setTarget(null) This commit unregisters the viewport resize listener when setTarget(null) is called, and registers it when setTarget(target) is called, and when we don't have that listener registered yet. This prevents memory leaks where references to map objects are retained because of these viewport resize listeners. --- src/ol/map.js | 18 ++++++++++++++++-- test/spec/ol/map.test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/ol/map.js b/src/ol/map.js index aa3c2a11cf..3462b74eda 100644 --- a/src/ol/map.js +++ b/src/ol/map.js @@ -338,13 +338,17 @@ ol.Map = function(options) { this.registerDisposable(this.renderer_); /** + * @type {goog.dom.ViewportSizeMonitor} * @private */ this.viewportSizeMonitor_ = new goog.dom.ViewportSizeMonitor(); this.registerDisposable(this.viewportSizeMonitor_); - goog.events.listen(this.viewportSizeMonitor_, goog.events.EventType.RESIZE, - this.updateSize, false, this); + /** + * @type {goog.events.Key} + * @private + */ + this.viewportResizeListenerKey_ = null; /** * @private @@ -1030,12 +1034,22 @@ ol.Map.prototype.handleTargetChanged_ = function() { if (goog.isNull(targetElement)) { goog.dom.removeNode(this.viewport_); + if (!goog.isNull(this.viewportResizeListenerKey_)) { + goog.events.unlistenByKey(this.viewportResizeListenerKey_); + this.viewportResizeListenerKey_ = null; + } } else { goog.dom.appendChild(targetElement, this.viewport_); var keyboardEventTarget = goog.isNull(this.keyboardEventTarget_) ? targetElement : this.keyboardEventTarget_; this.keyHandler_.attach(keyboardEventTarget); + + if (goog.isNull(this.viewportResizeListenerKey_)) { + this.viewportResizeListenerKey_ = goog.events.listen( + this.viewportSizeMonitor_, goog.events.EventType.RESIZE, + this.updateSize, false, this); + } } this.updateSize(); diff --git a/test/spec/ol/map.test.js b/test/spec/ol/map.test.js index ef6a658606..42b5054be4 100644 --- a/test/spec/ol/map.test.js +++ b/test/spec/ol/map.test.js @@ -187,6 +187,39 @@ describe('ol.Map', function() { }); }); + describe('#setTarget', function() { + var map; + + beforeEach(function() { + map = new ol.Map({ + target: document.createElement('div') + }); + var viewportResizeListeners = map.viewportSizeMonitor_.getListeners( + goog.events.EventType.RESIZE, false); + expect(viewportResizeListeners).to.have.length(1); + }); + + describe('call setTarget with null', function() { + it('unregisters the viewport resize listener', function() { + map.setTarget(null); + var viewportResizeListeners = map.viewportSizeMonitor_.getListeners( + goog.events.EventType.RESIZE, false); + expect(viewportResizeListeners).to.have.length(0); + }); + }); + + describe('call setTarget with an element', function() { + it('registers a viewport resize listener', function() { + map.setTarget(null); + map.setTarget(document.createElement('div')); + var viewportResizeListeners = map.viewportSizeMonitor_.getListeners( + goog.events.EventType.RESIZE, false); + expect(viewportResizeListeners).to.have.length(1); + }); + }); + + }); + describe('create interactions', function() { var options; @@ -244,6 +277,7 @@ describe('ol.Map', function() { goog.require('goog.dispose'); goog.require('goog.dom'); +goog.require('goog.events.EventType'); goog.require('ol.Map'); goog.require('ol.MapEvent'); goog.require('ol.View'); From f1d7e2ec8519008946521e98b52760baf7260d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Lemoine?= Date: Mon, 30 Mar 2015 16:54:09 +0200 Subject: [PATCH 2/2] Unregister tile change listeners When the tile queue loads a tile it registers a "change" listener on that tile. But currently that listener is not unregistered, unless the tile cache containing that tile fills up and "dipose" is called on that tile. This commit makes handleTileChange deregister the "change" listener on the tile, if the tile is now in "loaded" state. --- src/ol/tilequeue.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ol/tilequeue.js b/src/ol/tilequeue.js index 68da9ed5fa..5da58713fc 100644 --- a/src/ol/tilequeue.js +++ b/src/ol/tilequeue.js @@ -76,6 +76,8 @@ ol.TileQueue.prototype.handleTileChange = function(event) { var state = tile.getState(); if (state === ol.TileState.LOADED || state === ol.TileState.ERROR || state === ol.TileState.EMPTY) { + goog.events.unlisten(tile, goog.events.EventType.CHANGE, + this.handleTileChange, false, this); --this.tilesLoading_; this.tileChangeCallback_(); }