From 0fcc986dacb04313e69660afb8c4cf05868539ec Mon Sep 17 00:00:00 2001
From: Erin Campbell <ecampbell@petrode.com>
Date: Thu, 8 Mar 2018 13:52:45 -0700
Subject: [PATCH] Fix memory leak in CanvasImageRenderer when rendering a
 VectorLayer with renderMode: 'image'. Added tests to CanvasImageLayerRenderer
 for this issue.

---
 src/ol/renderer/canvas/ImageLayer.js          | 15 +++++
 .../ol/renderer/canvas/imagelayer.test.js     | 59 +++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/src/ol/renderer/canvas/ImageLayer.js b/src/ol/renderer/canvas/ImageLayer.js
index 2c07a68215..0d70173559 100644
--- a/src/ol/renderer/canvas/ImageLayer.js
+++ b/src/ol/renderer/canvas/ImageLayer.js
@@ -80,6 +80,7 @@ CanvasImageLayerRenderer['create'] = function(mapRenderer, layer) {
       const candidate = /** @type {Object.<string, Function>} */ (candidates[i]);
       if (candidate !== CanvasImageLayerRenderer && candidate['handles'](RendererType.CANVAS, layer)) {
         renderer.setVectorRenderer(candidate['create'](mapRenderer, layer));
+        break;
       }
     }
   }
@@ -87,6 +88,17 @@ CanvasImageLayerRenderer['create'] = function(mapRenderer, layer) {
 };
 
 
+/**
+ * @inheritDoc
+ */
+CanvasImageLayerRenderer.prototype.disposeInternal = function() {
+  if (this.vectorRenderer_) {
+    this.vectorRenderer_.dispose();
+  }
+  IntermediateCanvasRenderer.prototype.disposeInternal.call(this);
+};
+
+
 /**
  * @inheritDoc
  */
@@ -210,6 +222,9 @@ CanvasImageLayerRenderer.prototype.forEachFeatureAtCoordinate = function(coordin
  * @param {ol.renderer.canvas.VectorLayer} renderer Vector renderer.
  */
 CanvasImageLayerRenderer.prototype.setVectorRenderer = function(renderer) {
+  if (this.vectorRenderer_) {
+    this.vectorRenderer_.dispose();
+  }
   this.vectorRenderer_ = renderer;
 };
 export default CanvasImageLayerRenderer;
diff --git a/test/spec/ol/renderer/canvas/imagelayer.test.js b/test/spec/ol/renderer/canvas/imagelayer.test.js
index 049006b5ee..3bb97359d6 100644
--- a/test/spec/ol/renderer/canvas/imagelayer.test.js
+++ b/test/spec/ol/renderer/canvas/imagelayer.test.js
@@ -1,12 +1,42 @@
 import Map from '../../../../../src/ol/Map.js';
 import View from '../../../../../src/ol/View.js';
 import ImageLayer from '../../../../../src/ol/layer/Image.js';
+import VectorLayer from '../../../../../src/ol/layer/Vector.js';
 import Projection from '../../../../../src/ol/proj/Projection.js';
 import Static from '../../../../../src/ol/source/ImageStatic.js';
+import VectorSource from '../../../../../src/ol/source/Vector.js';
+import CanvasImageLayerRenderer from '../../../../../src/ol/renderer/canvas/ImageLayer.js';
+import CanvasVectorLayerRenderer from '../../../../../src/ol/renderer/canvas/VectorLayer.js';
 
 
 describe('ol.renderer.canvas.ImageLayer', function() {
 
+  describe('#dispose()', function() {
+    let layer, imageRenderer, vectorRenderer;
+
+    beforeEach(function() {
+      layer = new VectorLayer({
+        renderMode: 'image',
+        source: new VectorSource()
+      });
+      imageRenderer = new CanvasImageLayerRenderer(layer);
+      vectorRenderer = new CanvasVectorLayerRenderer(layer);
+    });
+
+    afterEach(function() {
+      vectorRenderer.dispose();
+      imageRenderer.dispose();
+      layer.dispose();
+    });
+
+    it('cleans up CanvasVectorRenderer', function() {
+      const spy = sinon.spy(vectorRenderer, 'dispose');
+      imageRenderer.setVectorRenderer(vectorRenderer);
+      imageRenderer.dispose();
+      expect(spy.called).to.be(true);
+    });
+  });
+
   describe('#forEachLayerAtCoordinate', function() {
 
     let map, target, source;
@@ -62,4 +92,33 @@ describe('ol.renderer.canvas.ImageLayer', function() {
     });
   });
 
+  describe('#setVectorRenderer()', function() {
+    let layer, imageRenderer, vectorRenderer1, vectorRenderer2;
+
+    beforeEach(function() {
+      layer = new VectorLayer({
+        renderMode: 'image',
+        source: new VectorSource()
+      });
+      imageRenderer = new CanvasImageLayerRenderer(layer);
+      vectorRenderer1 = new CanvasVectorLayerRenderer(layer);
+      vectorRenderer2 = new CanvasVectorLayerRenderer(layer);
+    });
+
+    afterEach(function() {
+      vectorRenderer1.dispose();
+      vectorRenderer2.dispose();
+      imageRenderer.dispose();
+      layer.dispose();
+    });
+
+    it('cleans up an existing vectorRenderer', function() {
+      const spy = sinon.spy(vectorRenderer1, 'dispose');
+      imageRenderer.setVectorRenderer(vectorRenderer1);
+      expect(spy.called).to.be(false);
+      imageRenderer.setVectorRenderer(vectorRenderer2);
+      expect(spy.called).to.be(true);
+    });
+  });
+
 });