Changeset 243722 in webkit

Timestamp:

Apr 1, 2019, 4:38:45 PM (7 years ago)

Author:

Devin Rousso

Message:

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):

Location:

trunk/Source/WebInspectorUI

Files:

• ChangeLog (modified) (1 diff)
• UserInterface/Controllers/DOMDebuggerManager.js (modified) (1 diff)
• UserInterface/Controllers/DOMManager.js (modified) (4 diffs)
• UserInterface/Views/ContextMenuUtilities.js (modified) (1 diff)
• UserInterface/Views/DOMNodeTreeElement.js (modified) (1 diff)
• UserInterface/Views/EventBreakpointTreeElement.js (modified) (2 diffs)

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js

@@ -555 +555 @@
     {
         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)

trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js

@@ -51 +51 @@
         this._pendingDocumentRequestCallbacks = null;
 
+        WI.EventBreakpoint.addEventListener(WI.EventBreakpoint.Event.DisabledStateChanged, this._handleEventBreakpointDisabledStateChanged, this);
+
         WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
     }
@@ -593 +595 @@
     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;
-            }
-
-            WI.domDebuggerManager.dispatchEventToListeners(WI.DOMDebuggerManager.Event.EventBreakpointAdded, {breakpoint});
-        });
+        for (let target of WI.targets) {
+            if (target.DOMAgent)
+                this._updateEventBreakpoint(breakpoint, target);
+        }
+
+        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);
-
-        DOMAgent.removeBreakpointForEventListener(eventListener.eventListenerId, (error) => {
-            if (error) {
-                console.error(error);
-                return;
-            }
-
-            WI.domDebuggerManager.dispatchEventToListeners(WI.DOMDebuggerManager.Event.EventBreakpointRemoved, {breakpoint});
-        });
+        for (let target of WI.targets) {
+            if (target.DOMAgent)
+                target.DOMAgent.removeBreakpointForEventListener(eventListener.eventListenerId);
+        }
+
+        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);
+        }
     }
 
@@ -627 +641 @@
         return this._breakpointsForEventListeners.get(eventListenerId) || null;
     }
+
+    // Private
 
     _buildHighlightConfig(mode = "all")
@@ -647 +663 @@
     }
 
-    // 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)

trunk/Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js

@@ -307 +307 @@
         contextMenu.appendItem(WI.UIString("Delete Breakpoints"), function() {
             WI.domDebuggerManager.removeDOMBreakpointsForNode(domNode);
+            WI.domManager.removeEventListenerBreakpointsForNode(domNode);
         });
     }

trunk/Source/WebInspectorUI/UserInterface/Views/DOMNodeTreeElement.js

@@ -47 +47 @@
 
         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;

trunk/Source/WebInspectorUI/UserInterface/Views/EventBreakpointTreeElement.js

@@ -111 +111 @@
     {
         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();
@@ -147 +146 @@
     _toggleBreakpoint()
     {
-        if (this.representedObject.eventListener) {
-            WI.domManager.removeBreakpointForEventListener(this.representedObject.eventListener);
-            return;
-        }
-
         this.representedObject.disabled = !this.representedObject.disabled;
     }