* SUMMARY WebInspector.Object can dispatch constructor-level events multiple times. As Object.dispatchEventToListeners walks the prototype chain looking for listeners on each constructor, it should check for listeners that are a direct property of the constructor, rather than a simple 'in' test which will pick up properties inherited down the chain. * EXAMPLE Assuming the following class hierarchy: C -> B -> A -> WebInspector.Object and an object Foo listening for events on class A: class Foo() { constructor() { A.addEventListener(A.Event.SomeEvent, this._handleSomeEvent, this); } _handleSomeEvent(event) { ... } } If an object of type C fires A.Event.SomeEvent, Foo._handleSomeEvent will be called three times, once for each constructor in the prototype chain. The expectation is that it will be called just once. * NOTE This has real performance implications in the Inspector UI, as constructor event handlers often perform layout updates. For example, ContentBrowser calls NavigationBar.udpateLayout (which is extremely expensive) in response to WebInspector.ContentView.Event.SelectionPathComponentsDidChange, which it listens for on the ContentView constructor. Selecting a DOM node in the Elements tab currently generates *six* calls to NavigationBar.updateLayout (three for each deselect/select in the DOM tree content view). Fixing this issue will reduce the number to just two.
<rdar://problem/23267238>
Created attachment 264093 [details] [Patch] Proposed Fix
Comment on attachment 264093 [details] [Patch] Proposed Fix Nice find! We might be able to keep this code as-is and just remove the loop where we walk the prototype chain and just do: dispatch(this.constructor); I think that will do the same thing, and just take advantage of JS's inheritance. I'm not sure why I didn't think of that before, unless this wasn't broken until we switched to classes with proper inheritance.
(In reply to comment #3) > Comment on attachment 264093 [details] > [Patch] Proposed Fix > > Nice find! > > We might be able to keep this code as-is and just remove the loop where we > walk the prototype chain and just do: dispatch(this.constructor); That would be a nice simplification. I'll try it out. > I think that will do the same thing, and just take advantage of JS's > inheritance. I'm not sure why I didn't think of that before, unless this > wasn't broken until we switched to classes with proper inheritance. Interesting...
Comment on attachment 264093 [details] [Patch] Proposed Fix Nevermind! That approach wouldn't work, this is the correct fix. My idea wont allow proper bubbling to all the classes. It would only catch event listeners registered on the concrete class of the instance, not the superclasses
Comment on attachment 264093 [details] [Patch] Proposed Fix Clearing flags on attachment: 264093 Committed r191615: <http://trac.webkit.org/changeset/191615>
All reviewed patches have been landed. Closing bug.