DevTools: Use a Map for modelListeners in TargetManager

third_party/WebKit/Source/devtools/front_end/sdk/TargetManager.js

 /*
  * Copyright 2014 The Chromium Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 /**
  * @constructor
  * @extends {WebInspector.Object}
  */
 WebInspector.TargetManager = function()
 {
     WebInspector.Object.call(this);
     /** @type {!Array.<!WebInspector.Target>} */
     this._targets = [];
     /** @type {!Array.<!WebInspector.TargetManager.Observer>} */
     this._observers = [];
     this._observerCapabiliesMaskSymbol = Symbol("observerCapabilitiesMask");
-    /** @type {!Object.<string, !Array.<{modelClass: !Function, thisObject: (!Object|undefined), listener: function(!WebInspector.Event)}>>} */
-    this._modelListeners = {};
+    /** @type {!Map<symbol, !Array<{modelClass: !Function, thisObject: (!Object|undefined), listener: function(!WebInspector.Event)}>>} */

[lushnikov, 2016/08/26 19:34:45]

technically, this should be (symbol|string)

[dgozman, 2016/08/26 20:52:49]

I hope we don't have strings anymore! Let's change to symbol everywhere.

[alph, 2016/08/26 21:00:33]

I'll keep it just symbol and make sure there are no strings in a separate patch.

+    this._modelListeners = new Map();
     this._isSuspended = false;
 }
 
 /** @enum {symbol} */
 WebInspector.TargetManager.Events = {
     InspectedURLChanged: Symbol("InspectedURLChanged"),
     MainFrameNavigated: Symbol("MainFrameNavigated"),
     Load: Symbol("Load"),
     PageReloadRequested: Symbol("PageReloadRequested"),
     WillReloadPage: Symbol("WillReloadPage"),
@@ skipping 77 matching lines @@
 
         var resourceTreeModel = WebInspector.ResourceTreeModel.fromTarget(this._targets[0]);
         if (!resourceTreeModel)
             return;
 
         resourceTreeModel.reloadPage(bypassCache, injectedScript);
     },
 
     /**
      * @param {!Function} modelClass
-     * @param {string} eventType
+     * @param {symbol} eventType

[lushnikov, 2016/08/26 19:34:45]

(symbol|string) as well

[alph, 2016/08/26 21:00:34]

ditto

      * @param {function(!WebInspector.Event)} listener
      * @param {!Object=} thisObject
      */
     addModelListener: function(modelClass, eventType, listener, thisObject)
     {
         for (var i = 0; i < this._targets.length; ++i) {
             var model = this._targets[i].model(modelClass);
             if (model)
                 model.addEventListener(eventType, listener, thisObject);
         }
-        if (!this._modelListeners[eventType])
-            this._modelListeners[eventType] = [];
-        this._modelListeners[eventType].push({ modelClass: modelClass, thisObject: thisObject, listener: listener });
+        if (!this._modelListeners.has(eventType))
+            this._modelListeners.set(eventType, []);
+        this._modelListeners.get(eventType).push({ modelClass: modelClass, thisObject: thisObject, listener: listener });
     },
 
     /**
      * @param {!Function} modelClass
-     * @param {string} eventType
+     * @param {symbol} eventType

[lushnikov, 2016/08/26 19:34:45]

ditto

[alph, 2016/08/26 21:00:34]

ditto

      * @param {function(!WebInspector.Event)} listener
      * @param {!Object=} thisObject
      */
     removeModelListener: function(modelClass, eventType, listener, thisObject)
     {
-        if (!this._modelListeners[eventType])
+        if (!this._modelListeners.has(eventType))
             return;
 
         for (var i = 0; i < this._targets.length; ++i) {
             var model = this._targets[i].model(modelClass);
             if (model)
                 model.removeEventListener(eventType, listener, thisObject);
         }
 
-        var listeners = this._modelListeners[eventType];
+        var listeners = this._modelListeners.get(eventType);
         for (var i = 0; i < listeners.length; ++i) {
             if (listeners[i].modelClass === modelClass && listeners[i].listener === listener && listeners[i].thisObject === thisObject)
                 listeners.splice(i--, 1);
         }
         if (!listeners.length)
-            delete this._modelListeners[eventType];
+            this._modelListeners.delete(eventType);
     },
 
     /**
      * @param {!WebInspector.TargetManager.Observer} targetObserver
      * @param {number=} capabilitiesMask
      */
     observeTargets: function(targetObserver, capabilitiesMask)
     {
         if (this._observerCapabiliesMaskSymbol in targetObserver)
             throw new Error("Observer can only be registered once");
@@ skipping 82 matching lines @@
                 setupRedispatch.call(this, WebInspector.ResourceTreeModel.Events.MainFrameNavigated, WebInspector.TargetManager.Events.MainFrameNavigated),
                 setupRedispatch.call(this, WebInspector.ResourceTreeModel.Events.Load, WebInspector.TargetManager.Events.Load),
                 setupRedispatch.call(this, WebInspector.ResourceTreeModel.Events.PageReloadRequested, WebInspector.TargetManager.Events.PageReloadRequested),
                 setupRedispatch.call(this, WebInspector.ResourceTreeModel.Events.WillReloadPage, WebInspector.TargetManager.Events.WillReloadPage)
             ];
         }
         var copy = this._observersForTarget(target);
         for (var i = 0; i < copy.length; ++i)
             copy[i].targetAdded(target);
 
-        for (var eventType in this._modelListeners) {
-            var listeners = this._modelListeners[eventType];
+        for (var pair of this._modelListeners) {

[lushnikov, 2016/08/26 19:34:45]

nit: this probably never compiles - there are no tuples in closure

[alph, 2016/08/26 21:00:34]

yeah. it seems to treat them as having value type.

+            var listeners = pair[1];
             for (var i = 0; i < listeners.length; ++i) {
                 var model = target.model(listeners[i].modelClass);
                 if (model)
-                    model.addEventListener(eventType, listeners[i].listener, listeners[i].thisObject);
+                    model.addEventListener(pair[0], listeners[i].listener, listeners[i].thisObject);
             }
         }
 
         /**
          * @param {!WebInspector.ResourceTreeModel.Events} sourceEvent
          * @param {!WebInspector.TargetManager.Events} targetEvent
          * @return {!WebInspector.EventTarget.EventDescriptor}
          * @this {WebInspector.TargetManager}
          */
         function setupRedispatch(sourceEvent, targetEvent)
@@ skipping 10 matching lines @@
         this._targets.remove(target);
         var resourceTreeModel = WebInspector.ResourceTreeModel.fromTarget(target);
         var treeModelListeners = resourceTreeModel && resourceTreeModel[WebInspector.TargetManager._listenersSymbol];
         if (treeModelListeners)
             WebInspector.EventTarget.removeEventListeners(treeModelListeners);
 
         var copy = this._observersForTarget(target);
         for (var i = 0; i < copy.length; ++i)
             copy[i].targetRemoved(target);
 
-        for (var eventType in this._modelListeners) {
-            var listeners = this._modelListeners[eventType];
+        for (var pair of this._modelListeners) {
+            var listeners = pair[1];
             for (var i = 0; i < listeners.length; ++i) {
                 var model = target.model(listeners[i].modelClass);
                 if (model)
-                    model.removeEventListener(eventType, listeners[i].listener, listeners[i].thisObject);
+                    model.removeEventListener(pair[0], listeners[i].listener, listeners[i].thisObject);
             }
         }
     },
 
     /**
      * @param {number=} capabilitiesMask
      * @return {!Array.<!WebInspector.Target>}
      */
     targets: function(capabilitiesMask)
     {
@@ skipping 64 matching lines @@
     /**
      * @param {!WebInspector.Target} target
      */
     targetRemoved: function(target) { },
 }
 
 /**
  * @type {!WebInspector.TargetManager}
  */
 WebInspector.targetManager = new WebInspector.TargetManager();