add counter + test, better comments

packages/@ember/-internals/meta/lib/meta.ts

@@ -14,6 +14,7 @@ export interface MetaCounters {
   deleteCalls: number;
   metaCalls: number;
   metaInstantiated: number;
+  reopensAfterFlatten: number;
 }
 
 let counters: MetaCounters | undefined;
@@ -25,6 +26,7 @@ if (DEBUG) {
     deleteCalls: 0,
     metaCalls: 0,
     metaInstantiated: 0,
+    reopensAfterFlatten: 0,
   };
 }
 
@@ -42,7 +44,7 @@ const enum MetaFlags {
   META_DESTROYED = 1 << 2,
 }
 
-export const enum ListenerKind {
+const enum ListenerKind {
   ADD = 0,
   ONCE = 1,
   REMOVE = 2,
@@ -91,7 +93,7 @@ export class Meta {
   _listeners: Listener[] | undefined;
   _listenersVersion = 1;
   _inheritedEnd = 0;
-  _inheritedVersion = 0;
+  _flattenedVersion = 0;
 
   // DEBUG
   _values: any | undefined;
@@ -540,50 +542,90 @@ export class Meta {
     }
   }
 
-  // check if we our meta is owned by a prototype
-  // if so, our listeners are inheritable so check if we have
-  // cached our flattened listeners, if so then clear the inherited listeners
-  // and bump the global version count
+  /**
+    Flattening is based on a global revision counter. If the revision has
+    bumped it means that somewhere up the inheritance chain something has
+    changed, so we need to reflatten everything. This can only happen if:
+
+    1. A meta has been flattened (listener has been called)
+    2. The meta is a prototype meta with children who have inherited its
+       listeners
+    3. A new listener is subsequently added to the meta (e.g. via `.reopen()`)
+
+    This is a very rare occurence, so while the counter is global it shouldn't
+    be updated very often in practice.
+  */
+  private _shouldFlatten() {
+    return this._flattenedVersion < currentListenerVersion;
+  }
+
+  private _isFlattened() {
+    // A meta is flattened _only_ if the saved version is equal to the current
+    // version. Otherwise, it will flatten again the next time
+    // `flattenedListeners` is called, so there is no reason to bump the global
+    // version again.
+    return this._flattenedVersion === currentListenerVersion;
+  }
+
+  private _setFlattened() {
+    return this._flattenedVersion = currentListenerVersion;
+  }
+
   private writableListeners(): Listener[] {
     let listeners = this._listeners;
 
     if (listeners === undefined) {
       listeners = this._listeners = [] as Listener[];
     }
 
-    if (this.source === this.proto && this._inheritedVersion > 0) {
+    // Check if the meta is owned by a prototype. If so, our listeners are
+    // inheritable so check the meta has been flattened. If it has, children
+    // have inherited its listeners, so bump the global version counter to
+    // invalidate.
+    if (this.source === this.proto && this._isFlattened()) {
+      if (DEBUG) {
+        counters!.reopensAfterFlatten++;
+      }
+
       currentListenerVersion++;
     }
 
     return listeners;
   }
 
-  protected flattenedListeners(): Listener[] | undefined {
-    let parent = this.parent;
+  private flattenedListeners(): Listener[] | undefined {
+    if (this._shouldFlatten()) {
+      let parent = this.parent;
 
-    if (parent !== null && this._inheritedVersion < currentListenerVersion) {
-      // compute
-      let parentListeners = parent.flattenedListeners();
+      if (parent !== null) {
+        // compute
+        let parentListeners = parent.flattenedListeners();
 
-      if (parentListeners !== undefined) {
-        let listeners = this._listeners;
+        if (parentListeners !== undefined) {
+          let listeners = this._listeners;
 
-        if (listeners === undefined) {
-          listeners = this._listeners = [] as Listener[];
-        }
+          if (listeners === undefined) {
+            listeners = this._listeners = [] as Listener[];
+          }
+
+          if (this._inheritedEnd > 0) {
+            listeners.splice(0, this._inheritedEnd);
+            this._inheritedEnd = 0;
+          }
 
-        for (let i = 0; i < parentListeners.length; i++) {
-          let listener = parentListeners[i];
-          let index = indexOfListener(listeners, listener.event, listener.target, listener.method);
+          for (let i = 0; i < parentListeners.length; i++) {
+            let listener = parentListeners[i];
+            let index = indexOfListener(listeners, listener.event, listener.target, listener.method);
 
-          if (index === -1) {
-            listeners.unshift(listener);
-            this._inheritedEnd++;
+            if (index === -1) {
+              listeners.unshift(listener);
+              this._inheritedEnd++;
+            }
           }
         }
       }
 
-      this._inheritedVersion = currentListenerVersion;
+      this._setFlattened();
     }
 
     return this._listeners;
@@ -598,6 +640,8 @@ export class Meta {
       for (let index = 0; index < listeners.length; index++) {
         let listener = listeners[index];
 
+        // REMOVE and REMOVE_ALL listeners are placeholders that tell us not to
+        // inherit, so they never match. Only ADD and ONCE can match.
         if (
           listener.event === eventName &&
           (listener.kind === ListenerKind.ADD || listener.kind === ListenerKind.ONCE)

packages/@ember/-internals/meta/tests/meta_test.js

@@ -1,5 +1,5 @@
 import { AbstractTestCase, moduleFor } from 'internal-test-helpers';
-import { meta } from '..';
+import { meta, counters } from '..';
 
 moduleFor(
   'Ember.meta',
@@ -76,6 +76,59 @@ moduleFor(
       assert.equal(matching[0], t);
     }
 
+    ['@test meta.listeners reopen after flatten'](assert) {
+      // Ensure counter is zeroed
+      counters.reopensAfterFlatten = 0;
+
+      class Class1 {}
+      let class1Meta = meta(Class1.prototype);
+      class1Meta.addToListeners('hello', null, 'm', 0);
+
+      let instance1 = new Class1();
+      let m1 = meta(instance1);
+
+      class Class2 {}
+      let class2Meta = meta(Class2.prototype);
+      class2Meta.addToListeners('hello', null, 'm', 0);
+
+      let instance2 = new Class2();
+      let m2 = meta(instance2);
+
+      m1.matchingListeners('hello');
+      m2.matchingListeners('hello');
+
+      assert.equal(counters.reopensAfterFlatten, 0, 'no reopen calls yet');
+
+      m1.addToListeners('world', null, 'm', 0);
+      m2.addToListeners('world', null, 'm', 0);
+      m1.matchingListeners('world');
+      m2.matchingListeners('world');
+
+      assert.equal(
+        counters.reopensAfterFlatten,
+        0,
+        'no reopen calls after mutating leaf listeners'
+      );
+
+      class1Meta.removeFromListeners('hello', null, 'm');
+      class2Meta.removeFromListeners('hello', null, 'm');
+      m1.matchingListeners('hello');
+      m2.matchingListeners('hello');
+
+      assert.equal(counters.reopensAfterFlatten, 1, 'one reopen call after mutating parents');
+
+      class1Meta.addToListeners('hello', null, 'm', 0);
+      m1.matchingListeners('hello');
+      class2Meta.addToListeners('hello', null, 'm', 0);
+      m2.matchingListeners('hello');
+
+      assert.equal(
+        counters.reopensAfterFlatten,
+        2,
+        'one reopen call after mutating parents and flattening out of order'
+      );
+    }
+
     ['@test meta.writeWatching issues useful error after destroy']() {
       let target = {
         toString() {

yarn.lock

@@ -112,6 +112,10 @@
   version "0.0.38"
   resolved "https://registry.yarnpkg.com/@types/estree/-/estree-0.0.38.tgz#c1be40aa933723c608820a99a373a16d215a1ca2"
 
+"@types/node@^10.5.5":
+  version "10.9.3"
+  resolved "https://registry.yarnpkg.com/@types/node/-/node-10.9.3.tgz#85f288502503ade0b3bfc049fe1777b05d0327d5"
+
 "@types/node@^9.6.0":
   version "9.6.0"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-9.6.0.tgz#d3480ee666df9784b1001a1872a2f6ccefb6c2d7"
@@ -6401,9 +6405,11 @@ route-recognizer@^0.3.3:
   version "0.3.3"
   resolved "https://registry.yarnpkg.com/route-recognizer/-/route-recognizer-0.3.3.tgz#1d365e27fa6995e091675f7dc940a8c00353bd29"
 
-router_js@2.0.0-beta.4:
-  version "2.0.0-beta.4"
-  resolved "https://registry.yarnpkg.com/router_js/-/router_js-2.0.0-beta.4.tgz#b61821c9f6da70e3c6114a09ef8135b389b7d0d0"
+router_js@^3.0.0:
+  version "3.0.0"
+  resolved "https://registry.yarnpkg.com/router_js/-/router_js-3.0.0.tgz#fcbc928bfc918d9bf1ca6fc02632f2a7cf62e0c1"
+  dependencies:
+    "@types/node" "^10.5.5"
 
 rsvp@3.0.14:
   version "3.0.14"