✨ [Observable] Make observable safe to remove handler while firing.

src/core/data-structures/observable.js

@@ -1,4 +1,4 @@
-import {removeItem} from '#core/types/array';
+import {remove, removeItem} from '#core/types/array';
 
 /**
  * This class helps to manage observers. Observers can be added, removed or
@@ -12,6 +12,12 @@ export class Observable {
   constructor() {
     /** @type {?Array<function(TYPE=):void>} */
     this.handlers_ = null;
+
+    /** @type {!Set<function(TYPE=):void>} */
+    this.handlersToRemove_ = new Set();
+
+    /** @type {number} */
+    this.iteratingDepth_ = 0;
   }
 
   /**
@@ -31,13 +37,18 @@ export class Observable {
 
   /**
    * Removes the observer from this instance.
+   * Can be called in a handler fired.
    * @param {function(TYPE=):void} handler Observer's instance.
    */
   remove(handler) {
     if (!this.handlers_) {
       return;
     }
-    removeItem(this.handlers_, handler);
+    if (this.iteratingDepth_ > 0) {
+      this.handlersToRemove_.add(handler);
+    } else {
+      removeItem(this.handlers_, handler);
+    }
   }
 
   /**
@@ -58,9 +69,15 @@ export class Observable {
     if (!this.handlers_) {
       return;
     }
+    this.iteratingDepth_++;
     for (const handler of this.handlers_) {
       handler(opt_event);
     }
+    this.iteratingDepth_--;
+    if (this.handlersToRemove_.size && this.iteratingDepth_ == 0) {
+      remove(this.handlers_, (handler) => this.handlersToRemove_.has(handler));
+      this.handlersToRemove_.clear();
+    }
   }
 
   /**

test/unit/core/data-structures/test-observable.js

@@ -44,4 +44,48 @@ describes.sandboxed('data structures - Observable', {}, () => {
     expect(observer1Called).to.equal(1);
     expect(observer2Called).to.equal(2);
   });
+
+  it('remove while firing', () => {
+    let observer1Called = 0;
+    const observer1 = () => {
+      observer1Called++;
+    };
+    observable.add(observer1);
+    const remove = observable.add(() => {
+      remove();
+    });
+    observable.add(observer1);
+
+    observable.fire();
+
+    expect(observer1Called).to.equal(2);
+  });
+
+  it('remove while firing on multiple depths', () => {
+    // First depth of iteration will remove the handler, should not stop second depth.
+    let observer1Called = 0;
+    let iterations = 0;
+    const observer1 = () => {
+      observer1Called++;
+    };
+    const secondRemove = observable.add(observer1);
+    observable.add(() => {
+      iterations++;
+      if (iterations == 1) {
+        secondRemove();
+        observable.fire();
+      }
+    });
+
+    // At this point handlers are: [increaseByOne, removePreviousHandler]
+    // When firing the first time, handler should not be removed until the end.
+    observable.fire();
+    expect(observer1Called).to.equal(2);
+    expect(iterations).to.equal(2);
+
+    // When firing again, handler should have been removed.
+    observable.fire();
+    expect(observer1Called).to.equal(2);
+    expect(iterations).to.equal(3);
+  });
 });