fix(Canvas): dispose race condition (#7885)

fabricjs · Apr 19, 2022 · 4b3f9a3 · 4b3f9a3
1 parent 887a745
commit 4b3f9a3
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 26 deletions.
diff --git a/src/canvas.class.js b/src/canvas.class.js
@@ -1273,21 +1273,24 @@
      * @chainable
      */
     dispose: function () {
-      var wrapper = this.wrapperEl;
+      var wrapperEl = this.wrapperEl,
+          lowerCanvasEl = this.lowerCanvasEl,
+          upperCanvasEl = this.upperCanvasEl,
+          cacheCanvasEl = this.cacheCanvasEl;
       this.removeListeners();
-      wrapper.removeChild(this.upperCanvasEl);
-      wrapper.removeChild(this.lowerCanvasEl);
+      this.callSuper('dispose');
+      wrapperEl.removeChild(upperCanvasEl);
+      wrapperEl.removeChild(lowerCanvasEl);
       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);
+      fabric.util.cleanUpJsdomNode(upperCanvasEl);
+      this.upperCanvasEl = undefined;
+      fabric.util.cleanUpJsdomNode(cacheCanvasEl);
+      this.cacheCanvasEl = undefined;
+      if (wrapperEl.parentNode) {
+        wrapperEl.parentNode.replaceChild(lowerCanvasEl, wrapperEl);
       }
       delete this.wrapperEl;
-      fabric.StaticCanvas.prototype.dispose.call(this);
       return this;
     },
 

diff --git a/test/unit/canvas.js b/test/unit/canvas.js
@@ -2135,7 +2135,6 @@
   });
 
   QUnit.test('dispose + set dimensions', function (assert) {
-    var done = assert.async();
     //made local vars to do not dispose the external canvas
     var el = fabric.document.createElement('canvas'),
       parentEl = fabric.document.createElement('div');
@@ -2155,24 +2154,18 @@
 
     var canvas = new fabric.Canvas(el, { enableRetinaScaling: true, renderOnAddRemove: false });
 
-    //  prevent a race condition
-    //  setDimensions requests rendering while disposing which throws an error
-    canvas.on('after:render', () => {
-      assert.equal(canvas._originalCanvasStyle, elStyle, 'saved original canvas style for disposal');
-      assert.notEqual(el.style.cssText, canvas._originalCanvasStyle, 'canvas el style has been changed');
+    canvas.setDimensions({ width: 500, height: 500 });
+    assert.equal(canvas._originalCanvasStyle, elStyle, 'saved original canvas style for disposal');
+    assert.notEqual(el.style.cssText, canvas._originalCanvasStyle, 'canvas el style has been changed');
 
-      canvas.dispose();
-      assert.equal(canvas._originalCanvasStyle, undefined, 'removed original canvas style');
-      assert.equal(el.style.cssText, elStyle, 'restored original canvas style');
-      assert.equal(el.width, 500, 'restored width');
-      assert.equal(el.height, 500, 'restored height');
+    canvas.dispose();
+    assert.equal(canvas._originalCanvasStyle, undefined, 'removed original canvas style');
+    assert.equal(el.style.cssText, elStyle, 'restored original canvas style');
+    assert.equal(el.width, 500, 'restored width');
+    assert.equal(el.height, 500, 'restored height');
 
-      fabric.devicePixelRatio = originalDevicePixelRatio;
-      done();
-    });
+    fabric.devicePixelRatio = originalDevicePixelRatio;
 
-    canvas.setDimensions({ width: 500, height: 500 });
-
   });
 
   // QUnit.test('dispose', function(assert) {