Better resource cleaning on canvas dispose (fabricjs#5142)

* test-leak * better than nothing * avoid breaking tests * avoid breaking tests
thiagocunha · Aug 5, 2018 · fb44937 · fb44937
1 parent 507388f
commit fb44937
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 8 deletions.
diff --git a/src/canvas.class.js b/src/canvas.class.js
@@ -1577,10 +1577,12 @@
       this.removeListeners();
       wrapper.removeChild(this.upperCanvasEl);
       wrapper.removeChild(this.lowerCanvasEl);
-      this.upperCanvasEl = null;
-      this.cacheCanvasEl = null;
       this.contextCache = null;
       this.contextTop = null;
+      ['upperCanvasEl', 'cacheCanvasEl'].forEach((function(element) {
+        fabric.util.cleanUpJsdomNode(this[element]);
+        this[element] = undefined;
+      }).bind(this));
       if (wrapper.parentNode) {
         wrapper.parentNode.replaceChild(this.lowerCanvasEl, this.wrapperEl);
       }

diff --git a/src/shapes/image.class.js b/src/shapes/image.class.js
@@ -184,9 +184,11 @@
         backend.evictCachesForKey(this.cacheKey);
         backend.evictCachesForKey(this.cacheKey + '_filtered');
       }
-      this._originalElement = undefined;
-      this._element = undefined;
-      this._filteredEl = undefined;
+      this._cacheContext = undefined;
+      ['_originalElement', '_element', '_filteredEl', '_cacheCanvas'].forEach((function(element) {
+        fabric.util.cleanUpJsdomNode(this[element]);
+        this[element] = undefined;
+      }).bind(this));
     },
 
     /**

diff --git a/src/static_canvas.class.js b/src/static_canvas.class.js
@@ -1673,11 +1673,18 @@
         object.dispose && object.dispose();
       });
       this._objects = [];
+      if (this.backgroundImage && this.backgroundImage.dispose) {
+        this.backgroundImage.dispose();
+      }
       this.backgroundImage = null;
+      if (this.overlayImage && this.overlayImage.dispose) {
+        this.overlayImage.dispose();
+      }
       this.overlayImage = null;
       this._iTextInstances = null;
-      this.lowerCanvasEl = null;
       this.contextContainer = null;
+      fabric.util.cleanUpJsdomNode(this.lowerCanvasEl);
+      this.lowerCanvasEl = undefined;
       return this;
     },
 

diff --git a/src/util/dom_misc.js b/src/util/dom_misc.js
@@ -292,6 +292,21 @@
     return impl._canvas || impl._image;
   };
 
+  function cleanUpJsdomNode(element) {
+    if (!fabric.isLikelyNode) {
+      return;
+    }
+    var impl = fabric.jsdomImplForWrapper(element);
+    if (impl) {
+      impl._image = null;
+      impl._canvas = null;
+      // unsure if necessary
+      impl._currentSrc = null;
+      impl._attributes = null;
+      impl._classList = null;
+    }
+  }
+
   fabric.util.getById = getById;
   fabric.util.toArray = toArray;
   fabric.util.makeElement = makeElement;
@@ -301,5 +316,6 @@
   fabric.util.getElementOffset = getElementOffset;
   fabric.util.getElementStyle = getElementStyle;
   fabric.util.getNodeCanvas = getNodeCanvas;
+  fabric.util.cleanUpJsdomNode = cleanUpJsdomNode;
 
 })();
diff --git a/test/visual/resize_filter.js b/test/visual/resize_filter.js
@@ -46,9 +46,11 @@
     }
     var img = fabric.document.createElement('img');
     img.onload = function() {
+      img.onload = null;
       callback(img);
     };
     img.onerror = function(err) {
+      img.onerror = null;
       console.log('Image loading errored', err);
     };
     img.src = filename;
@@ -79,8 +81,8 @@
       image.scaleToWidth(canvas.width / zoom);
       canvas.add(image);
       canvas.renderAll();
-      image.dispose();
       callback(canvas.lowerCanvasEl);
+      image.dispose();
     });
   }
 
@@ -103,8 +105,8 @@
       image.scaleToWidth(canvas.width);
       canvas.add(image);
       canvas.renderAll();
-      image.dispose();
       callback(canvas.lowerCanvasEl);
+      image.dispose();
     });
   }