Web Inspector: DOMDebugger: disabling a breakpoint for a specific event listener...
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Apr 2019 23:38:45 +0000 (23:38 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Apr 2019 23:38:45 +0000 (23:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196453
<rdar://problem/49489318>

Reviewed by Joseph Pecoraro.

Allow breakpoints for specific event listeners to be disabled, rather than immeditately
removing/deleting them when they are about to become disabled. This preserves the existing
functionality, but now allows for the tree element to stay in the UI in a disabled state.

* UserInterface/Controllers/DOMManager.js:
(WI.DOMManager):
(WI.DOMManager.prototype.setBreakpointForEventListener):
(WI.DOMManager.prototype.removeBreakpointForEventListener):
(WI.DOMManager.prototype.removeEventListenerBreakpointsForNode): Added.
(WI.DOMManager.prototype._updateEventBreakpoint): Added.
(WI.DOMManager.prototype._handleEventBreakpointDisabledStateChanged): Added.

* UserInterface/Controllers/DOMDebuggerManager.js:
(WI.DOMDebuggerManager.prototype._handleEventBreakpointDisabledStateChanged):

* UserInterface/Views/EventBreakpointTreeElement.js:
(WI.EventBreakpointTreeElement.prototype.populateContextMenu):
(WI.EventBreakpointTreeElement.prototype._toggleBreakpoint):

* UserInterface/Views/ContextMenuUtilities.js:
(WI.appendContextMenuItemsForDOMNodeBreakpoints):

* UserInterface/Views/DOMNodeTreeElement.js:
(WI.DOMNodeTreeElement.prototype.ondelete):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243722 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js
Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js
Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js
Source/WebInspectorUI/UserInterface/Views/DOMNodeTreeElement.js
Source/WebInspectorUI/UserInterface/Views/EventBreakpointTreeElement.js

index 55e6a53..0f0158d 100644 (file)
@@ -1,5 +1,38 @@
 2019-04-01  Devin Rousso  <drousso@apple.com>
 
+        Web Inspector: DOMDebugger: disabling a breakpoint for a specific event listener removes it from the UI
+        https://bugs.webkit.org/show_bug.cgi?id=196453
+        <rdar://problem/49489318>
+
+        Reviewed by Joseph Pecoraro.
+
+        Allow breakpoints for specific event listeners to be disabled, rather than immeditately
+        removing/deleting them when they are about to become disabled. This preserves the existing
+        functionality, but now allows for the tree element to stay in the UI in a disabled state.
+
+        * UserInterface/Controllers/DOMManager.js:
+        (WI.DOMManager):
+        (WI.DOMManager.prototype.setBreakpointForEventListener):
+        (WI.DOMManager.prototype.removeBreakpointForEventListener):
+        (WI.DOMManager.prototype.removeEventListenerBreakpointsForNode): Added.
+        (WI.DOMManager.prototype._updateEventBreakpoint): Added.
+        (WI.DOMManager.prototype._handleEventBreakpointDisabledStateChanged): Added.
+
+        * UserInterface/Controllers/DOMDebuggerManager.js:
+        (WI.DOMDebuggerManager.prototype._handleEventBreakpointDisabledStateChanged):
+
+        * UserInterface/Views/EventBreakpointTreeElement.js:
+        (WI.EventBreakpointTreeElement.prototype.populateContextMenu):
+        (WI.EventBreakpointTreeElement.prototype._toggleBreakpoint):
+
+        * UserInterface/Views/ContextMenuUtilities.js:
+        (WI.appendContextMenuItemsForDOMNodeBreakpoints):
+
+        * UserInterface/Views/DOMNodeTreeElement.js:
+        (WI.DOMNodeTreeElement.prototype.ondelete):
+
+2019-04-01  Devin Rousso  <drousso@apple.com>
+
         Web Inspector: Canvas: auto-record after page load sometimes shows the wrong UI
         https://bugs.webkit.org/show_bug.cgi?id=196320
         <rdar://problem/49356686>
index e71d22c..c5e0548 100644 (file)
@@ -554,6 +554,11 @@ WI.DOMDebuggerManager = class DOMDebuggerManager extends WI.Object
     _handleEventBreakpointDisabledStateChanged(event)
     {
         let breakpoint = event.target;
+
+        // Specific event listener breakpoints are handled by `DOMManager`.
+        if (breakpoint.eventListener)
+            return;
+
         for (let target of WI.targets) {
             if (target.DOMDebuggerAgent)
                 this._updateEventBreakpoint(breakpoint, target);
index dd52c4e..f68d894 100644 (file)
@@ -50,6 +50,8 @@ WI.DOMManager = class DOMManager extends WI.Object
         this._hasRequestedDocument = false;
         this._pendingDocumentRequestCallbacks = null;
 
+        WI.EventBreakpoint.addEventListener(WI.EventBreakpoint.Event.DisabledStateChanged, this._handleEventBreakpointDisabledStateChanged, this);
+
         WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
     }
 
@@ -592,34 +594,46 @@ WI.DOMManager = class DOMManager extends WI.Object
 
     setBreakpointForEventListener(eventListener)
     {
-        let breakpoint = new WI.EventBreakpoint(WI.EventBreakpoint.Type.Listener, eventListener.type, {eventListener});
+        let breakpoint = this._breakpointsForEventListeners.get(eventListener.eventListenerId);
+        if (breakpoint) {
+            console.assert(breakpoint.disabled);
+            breakpoint.disabled = false;
+            return;
+        }
+
+        breakpoint = new WI.EventBreakpoint(WI.EventBreakpoint.Type.Listener, eventListener.type, {eventListener});
+        console.assert(!breakpoint.disabled);
+
         this._breakpointsForEventListeners.set(eventListener.eventListenerId, breakpoint);
 
-        DOMAgent.setBreakpointForEventListener(eventListener.eventListenerId, (error) => {
-            if (error) {
-                console.error(error);
-                return;
-            }
+        for (let target of WI.targets) {
+            if (target.DOMAgent)
+                this._updateEventBreakpoint(breakpoint, target);
+        }
 
-            WI.domDebuggerManager.dispatchEventToListeners(WI.DOMDebuggerManager.Event.EventBreakpointAdded, {breakpoint});
-        });
+        WI.domDebuggerManager.dispatchEventToListeners(WI.DOMDebuggerManager.Event.EventBreakpointAdded, {breakpoint});
     }
 
     removeBreakpointForEventListener(eventListener)
     {
-        let breakpoint = this._breakpointsForEventListeners.get(eventListener.eventListenerId);
+        let breakpoint = this._breakpointsForEventListeners.take(eventListener.eventListenerId);
         console.assert(breakpoint);
 
-        this._breakpointsForEventListeners.delete(eventListener.eventListenerId);
+        for (let target of WI.targets) {
+            if (target.DOMAgent)
+                target.DOMAgent.removeBreakpointForEventListener(eventListener.eventListenerId);
+        }
 
-        DOMAgent.removeBreakpointForEventListener(eventListener.eventListenerId, (error) => {
-            if (error) {
-                console.error(error);
-                return;
-            }
+        WI.domDebuggerManager.dispatchEventToListeners(WI.DOMDebuggerManager.Event.EventBreakpointRemoved, {breakpoint});
+    }
 
-            WI.domDebuggerManager.dispatchEventToListeners(WI.DOMDebuggerManager.Event.EventBreakpointRemoved, {breakpoint});
-        });
+    removeEventListenerBreakpointsForNode(domNode)
+    {
+        for (let breakpoint of Array.from(this._breakpointsForEventListeners.values())) {
+            let eventListener = breakpoint.eventListener;
+            if (eventListener.nodeId === domNode.id)
+                this.removeBreakpointForEventListener(eventListener);
+        }
     }
 
     breakpointForEventListenerId(eventListenerId)
@@ -627,6 +641,8 @@ WI.DOMManager = class DOMManager extends WI.Object
         return this._breakpointsForEventListeners.get(eventListenerId) || null;
     }
 
+    // Private
+
     _buildHighlightConfig(mode = "all")
     {
         let highlightConfig = {showInfo: mode === "all"};
@@ -646,7 +662,30 @@ WI.DOMManager = class DOMManager extends WI.Object
         return highlightConfig;
     }
 
-    // Private
+    _updateEventBreakpoint(breakpoint, target)
+    {
+        let eventListener = breakpoint.eventListener;
+        console.assert(eventListener);
+
+        if (breakpoint.disabled)
+            target.DOMAgent.removeBreakpointForEventListener(eventListener.eventListenerId);
+        else
+            target.DOMAgent.setBreakpointForEventListener(eventListener.eventListenerId);
+    }
+
+    _handleEventBreakpointDisabledStateChanged(event)
+    {
+        let breakpoint = event.target;
+
+        // Non-specific event listener breakpoints are handled by `DOMDebuggerManager`.
+        if (!breakpoint.eventListener)
+            return;
+
+        for (let target of WI.targets) {
+            if (target.DOMAgent)
+                this._updateEventBreakpoint(breakpoint, target);
+        }
+    }
 
     _mainResourceDidChange(event)
     {
index f88d105..e1b126f 100644 (file)
@@ -306,6 +306,7 @@ WI.appendContextMenuItemsForDOMNodeBreakpoints = function(contextMenu, domNode,
 
         contextMenu.appendItem(WI.UIString("Delete Breakpoints"), function() {
             WI.domDebuggerManager.removeDOMBreakpointsForNode(domNode);
+            WI.domManager.removeEventListenerBreakpointsForNode(domNode);
         });
     }
 };
index 54542a4..1f5d171 100644 (file)
@@ -46,11 +46,7 @@ WI.DOMNodeTreeElement = class DOMNodeTreeElement extends WI.GeneralTreeElement
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         WI.domDebuggerManager.removeDOMBreakpointsForNode(this.representedObject);
-
-        for (let treeElement of this.children) {
-            if (treeElement instanceof WI.EventBreakpointTreeElement)
-                treeElement.ondelete();
-        }
+        WI.domManager.removeEventListenerBreakpointsForNode(this.representedObject);
 
         return true;
     }
index b8a1b52..04ddda2 100644 (file)
@@ -110,10 +110,9 @@ WI.EventBreakpointTreeElement = class EventBreakpointTreeElement extends WI.Gene
     populateContextMenu(contextMenu, event)
     {
         let breakpoint = this.representedObject;
-        if (!breakpoint.eventListener) {
-            let label = breakpoint.disabled ? WI.UIString("Enable Breakpoint") : WI.UIString("Disable Breakpoint");
-            contextMenu.appendItem(label, this._toggleBreakpoint.bind(this));
-        }
+
+        let label = breakpoint.disabled ? WI.UIString("Enable Breakpoint") : WI.UIString("Disable Breakpoint");
+        contextMenu.appendItem(label, this._toggleBreakpoint.bind(this));
 
         contextMenu.appendSeparator();
 
@@ -146,11 +145,6 @@ WI.EventBreakpointTreeElement = class EventBreakpointTreeElement extends WI.Gene
 
     _toggleBreakpoint()
     {
-        if (this.representedObject.eventListener) {
-            WI.domManager.removeBreakpointForEventListener(this.representedObject.eventListener);
-            return;
-        }
-
         this.representedObject.disabled = !this.representedObject.disabled;
     }