From 192c9d697ed0276428ad5f3a7f544b0edbacfaee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89ric=20Lemoine?= <eric.lemoine@gmail.com>
Date: Mon, 30 Mar 2015 16:29:30 +0200
Subject: [PATCH] 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');