fix: dispose performance

core/block.ts

@@ -313,44 +313,45 @@ export class Block implements IASTNodeLocation, IDeletable {
    * @suppress {checkTypes}
    */
   dispose(healStack: boolean) {
-    if (this.isDeadOrDying()) {
-      return;
-    }
-    // Terminate onchange event calls.
+    if (this.isDeadOrDying()) return;
+
+    // Dispose of this change listener before unplugging.
+    // Technically not necessary due to the event firing delay.
+    // But future-proofing.
     if (this.onchangeWrapper_) {
       this.workspace.removeChangeListener(this.onchangeWrapper_);
     }
 
     this.unplug(healStack);
     if (eventUtils.isEnabled()) {
+      // Constructing the delete event is costly. Only perform if necessary.
       eventUtils.fire(new (eventUtils.get(eventUtils.BLOCK_DELETE))(this));
     }
-    eventUtils.disable();
+    this.workspace.removeTopBlock(this);
+    this.disposeInternal();
+  }
+
+  /**
+   * Disposes of this block without doing things required by the top block.
+   * E.g. does not fire events, unplug the block, etc.
+   */
+  protected disposeInternal() {
+    if (this.isDeadOrDying()) return;
+
+    if (this.onchangeWrapper_) {
+      this.workspace.removeChangeListener(this.onchangeWrapper_);
+    }
 
+    eventUtils.disable();
     try {
-      // This block is now at the top of the workspace.
-      // Remove this block from the workspace's list of top-most blocks.
-      this.workspace.removeTopBlock(this);
       this.workspace.removeTypedBlock(this);
-      // Remove from block database.
       this.workspace.removeBlockById(this.id);
       this.disposing = true;
 
-      // First, dispose of all my children.
-      for (let i = this.childBlocks_.length - 1; i >= 0; i--) {
-        this.childBlocks_[i].dispose(false);
-      }
-      // Then dispose of myself.
-      // Dispose of all inputs and their fields.
-      for (let i = 0, input; input = this.inputList[i]; i++) {
-        input.dispose();
-      }
+      this.childBlocks_.forEach((c) => c.disposeInternal());
+      this.inputList.forEach((i) => i.dispose());
       this.inputList.length = 0;
-      // Dispose of any remaining connections (next/previous/output).
-      const connections = this.getConnections_(true);
-      for (let i = 0, connection; connection = connections[i]; i++) {
-        connection.dispose();
-      }
+      this.getConnections_(true).forEach((c) => c.dispose());
     } finally {
       eventUtils.enable();
       if (typeof this.destroy === 'function') {

core/block_animations.ts

@@ -39,6 +39,9 @@ let wobblingBlock: BlockSvg|null = null;
  * @internal
  */
 export function disposeUiEffect(block: BlockSvg) {
+  // Disposing is going to take so long the animation won't play anyway.
+  if (block.getDescendants(false).length > 100) return;
+
   const workspace = block.workspace;
   const svgGroup = block.getSvgRoot();
   workspace.getAudioManager().play('delete');

core/block_svg.ts

@@ -768,55 +768,39 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg,
    * @suppress {checkTypes}
    */
   override dispose(healStack?: boolean, animate?: boolean) {
-    if (this.isDeadOrDying()) {
-      return;
-    }
+    if (this.isDeadOrDying()) return;
+
     Tooltip.dispose();
-    Tooltip.unbindMouseEvents(this.pathObject.svgPath);
-    dom.startTextWidthCache();
-    // Save the block's workspace temporarily so we can resize the
-    // contents once the block is disposed.
-    const blockWorkspace = this.workspace;
-    // If this block is being dragged, unlink the mouse events.
-    if (common.getSelected() === this) {
-      this.unselect();
-      this.workspace.cancelCurrentGesture();
-    }
-    // If this block has a context menu open, close it.
-    if (ContextMenu.getCurrentBlock() === this) {
-      ContextMenu.hide();
-    }
+    ContextMenu.hide();
 
     if (animate && this.rendered) {
       this.unplug(healStack);
       blockAnimations.disposeUiEffect(this);
     }
-    // Stop rerendering.
-    this.rendered = false;
 
-    // Clear pending warnings.
-    for (const n of this.warningTextDb.values()) {
-      clearTimeout(n);
-    }
-    this.warningTextDb.clear();
+    super.dispose(!!healStack);
+    dom.removeNode(this.svgGroup_);
+  }
 
-    const icons = this.getIcons();
-    for (let i = 0; i < icons.length; i++) {
-      icons[i].dispose();
-    }
+  /**
+   * Disposes of this block without doing things required by the top block.
+   * E.g. does trigger UI effects, remove nodes, etc.
+   */
+  override disposeInternal() {
+    if (this.isDeadOrDying()) return;
+    super.disposeInternal();
+
+    this.rendered = false;
 
-    // Just deleting this block from the DOM would result in a memory leak as
-    // well as corruption of the connection database.  Therefore we must
-    // methodically step through the blocks and carefully disassemble them.
     if (common.getSelected() === this) {
-      common.setSelected(null);
+      this.unselect();
+      this.workspace.cancelCurrentGesture();
     }
 
-    super.dispose(!!healStack);
+    [...this.warningTextDb.values()].forEach((n) => clearTimeout(n));
+    this.warningTextDb.clear();
 
-    dom.removeNode(this.svgGroup_);
-    blockWorkspace.resizeContents();
-    dom.stopTextWidthCache();
+    this.getIcons().forEach((i) => i.dispose());
   }
 
   /**

core/connection.ts

@@ -150,7 +150,7 @@ export class Connection implements IASTNodeLocationWithBlock {
       this.setShadowStateInternal_();
 
       const targetBlock = this.targetBlock();
-      if (targetBlock) {
+      if (targetBlock && !targetBlock.isDeadOrDying()) {
         // Disconnect the attached normal block.
         targetBlock.unplug();
       }
@@ -544,6 +544,7 @@ export class Connection implements IASTNodeLocationWithBlock {
       }
     } else if (target.isShadow()) {
       target.dispose(false);
+      if (this.getSourceBlock().isDeadOrDying()) return;
       this.respawnShadow_();
       if (this.targetBlock() && this.targetBlock()!.isShadow()) {
         this.serializeShadow_(this.targetBlock());

core/field.ts

@@ -489,14 +489,11 @@ export abstract class Field<T = any> implements IASTNodeLocationSvg,
   dispose() {
     dropDownDiv.hideIfOwner(this);
     WidgetDiv.hideIfOwner(this);
-    Tooltip.unbindMouseEvents(this.getClickTarget_());
 
-    if (this.mouseDownWrapper_) {
-      browserEvents.unbind(this.mouseDownWrapper_);
+    if (!this.getSourceBlock()?.isDeadOrDying()) {
+      dom.removeNode(this.fieldGroup_);
     }
 
-    dom.removeNode(this.fieldGroup_);
-
     this.disposed = true;
   }
 

core/icon.ts

@@ -79,8 +79,10 @@ export abstract class Icon {
 
   /** Dispose of this icon. */
   dispose() {
-    dom.removeNode(this.iconGroup_);  // Dispose of and unlink the icon.
-    this.setVisible(false);           // Dispose of and unlink the bubble.
+    if (!this.getBlock().isDeadOrDying()) {
+      dom.removeNode(this.iconGroup_);
+    }
+    this.setVisible(false);  // Dispose of and unlink the bubble.
   }
 
   /** Add or remove the UI indicating if this icon may be clicked or not. */

core/tooltip.ts

@@ -276,6 +276,7 @@ function onMouseOut(_e: PointerEvent) {
     hide();
   }, 1);
   clearTimeout(showPid);
+  showPid = 0;
 }
 
 /**
@@ -342,6 +343,7 @@ export function hide() {
   }
   if (showPid) {
     clearTimeout(showPid);
+    showPid = 0;
   }
 }
 

tests/mocha/connection_checker_test.js

@@ -386,7 +386,7 @@ suite('Connection checker', function() {
       });
 
       test('Connect to unconnected unmovable block', function() {
-        // Delete blockA.
+        this.blockB.previousConnection.disconnect();
         this.blockA.dispose();
 
         // Try to connect blockC above blockB.
@@ -436,7 +436,7 @@ suite('Connection checker', function() {
       });
 
       test('Connect to unconnected unmovable block', function() {
-        // Delete blockA
+        this.blockB.outputConnection.disconnect();
         this.blockA.dispose();
 
         // Try to connect C's input to B's output. Allowed because B is now unconnected.

tests/mocha/event_block_delete_test.js

@@ -6,13 +6,12 @@
 
 goog.declareModuleId('Blockly.test.eventBlockDelete');
 
-import * as eventUtils from '../../build/src/core/events/utils.js';
 import {defineRowBlock} from './test_helpers/block_definitions.js';
 import {sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown.js';
 
 suite('Block Delete Event', function() {
   setup(function() {
-    sharedTestSetup.call(this);
+    this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock;
     defineRowBlock();
     this.workspace = new Blockly.Workspace();
   });
@@ -21,6 +20,23 @@ suite('Block Delete Event', function() {
     sharedTestTeardown.call(this);
   });
 
+  suite('Receiving', function() {
+    test('blocks do not receive their own delete events', function() {
+      Blockly.Blocks['test'] = {
+        onchange: function(e) { },
+      };
+      // Need to stub the definition, because the property on the definition is
+      // what gets registered as an event listener.
+      const spy = sinon.spy(Blockly.Blocks['test'], 'onchange');
+      const testBlock = this.workspace.newBlock('test');
+
+      testBlock.dispose();
+      this.clock.runAll();
+
+      chai.assert.isFalse(spy.called);
+    });
+  });
+
   suite('Serialization', function() {
     test('events round-trip through JSON', function() {
       const block = this.workspace.newBlock('row_block', 'block_id');

tests/mocha/serializer_test.js

@@ -1866,7 +1866,7 @@ const runSerializerTestSuite = (serializer, deserializer, testSuite) => {
 
   suiteCall(testSuite.title, function() {
     setup(function() {
-      sharedTestSetup.call(this);
+      sharedTestSetup.call(this, {fireEventsNow: false});
       this.workspace = new Blockly.Workspace();
     });