Web Inspector: Uncaught Exception: Breakpoint at specified location already exists.
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Jul 2019 03:45:28 +0000 (03:45 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Jul 2019 03:45:28 +0000 (03:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197034
<rdar://problem/50049004>

Reviewed by Joseph Pecoraro.

When "adjusting" a `WI.Breakpoint` (e.g. removing and then re-adding with a different
configuration), make sure to only re-add the `WI.Breakpoint` to the `WI.Target` it was just
removed from, rather to all `WI.targets`.

Since we iterate over `WI.targets` in both `WI.DebuggerManager.prototype._setBreakpoint` and
`WI.DebuggerManager.prototype._removeBreakpoint`, we ended up iterating `WI.targets` twice.

Each time the `WI.Breakpoint` is removed from a `WI.Target`, pass the `WI.Target` to the
`callback` given to `WI.DebuggerManager.prototype._removeBreakpoint`, so that the eventual
call to `WI.DebuggerManager.prototype._setBreakpoint` can reuse it as the `specificTarget`,
instead of iterating `WI.targets` (meaning we only iterate it once).

* UserInterface/Controllers/DebuggerManager.js:
(WI.DebuggerManager.prototype.addBreakpoint):
(WI.DebuggerManager.prototype._removeBreakpoint.didRemoveBreakpoint):
(WI.DebuggerManager.prototype._breakpointDisplayLocationDidChange):
(WI.DebuggerManager.prototype._breakpointEditablePropertyDidChange):

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

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js

index 4f95c48..3549d17 100644 (file)
@@ -1,5 +1,31 @@
 2019-07-22  Devin Rousso  <drousso@apple.com>
 
+        Web Inspector: Uncaught Exception: Breakpoint at specified location already exists.
+        https://bugs.webkit.org/show_bug.cgi?id=197034
+        <rdar://problem/50049004>
+
+        Reviewed by Joseph Pecoraro.
+
+        When "adjusting" a `WI.Breakpoint` (e.g. removing and then re-adding with a different
+        configuration), make sure to only re-add the `WI.Breakpoint` to the `WI.Target` it was just
+        removed from, rather to all `WI.targets`.
+
+        Since we iterate over `WI.targets` in both `WI.DebuggerManager.prototype._setBreakpoint` and
+        `WI.DebuggerManager.prototype._removeBreakpoint`, we ended up iterating `WI.targets` twice.
+
+        Each time the `WI.Breakpoint` is removed from a `WI.Target`, pass the `WI.Target` to the
+        `callback` given to `WI.DebuggerManager.prototype._removeBreakpoint`, so that the eventual
+        call to `WI.DebuggerManager.prototype._setBreakpoint` can reuse it as the `specificTarget`,
+        instead of iterating `WI.targets` (meaning we only iterate it once).
+
+        * UserInterface/Controllers/DebuggerManager.js:
+        (WI.DebuggerManager.prototype.addBreakpoint):
+        (WI.DebuggerManager.prototype._removeBreakpoint.didRemoveBreakpoint):
+        (WI.DebuggerManager.prototype._breakpointDisplayLocationDidChange):
+        (WI.DebuggerManager.prototype._breakpointEditablePropertyDidChange):
+
+2019-07-22  Devin Rousso  <drousso@apple.com>
+
         Localization: change fps to FPS
         <rdar://problem/53342508>
 
index 4276799..0ecd601 100644 (file)
@@ -490,10 +490,8 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
 
         this._breakpoints.push(breakpoint);
 
-        if (!breakpoint.disabled) {
-            const specificTarget = undefined;
-            this._setBreakpoint(breakpoint, specificTarget);
-        }
+        if (!breakpoint.disabled)
+            this._setBreakpoint(breakpoint);
 
         if (!this._restoringBreakpoints)
             WI.objectStores.breakpoints.putObject(breakpoint);
@@ -999,10 +997,12 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
         if (!breakpoint.identifier)
             return;
 
-        function didRemoveBreakpoint(error)
+        function didRemoveBreakpoint(target, error)
         {
-            if (error)
-                console.error(error);
+            if (error) {
+                WI.reportInternalError(error);
+                return;
+            }
 
             this._breakpointIdMap.delete(breakpoint.identifier);
 
@@ -1011,16 +1011,16 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
             // Don't reset resolved here since we want to keep disabled breakpoints looking like they
             // are resolved in the user interface. They will get marked as unresolved in reset.
 
-            if (typeof callback === "function")
-                callback();
+            if (callback)
+                callback(target);
         }
 
         if (breakpoint.contentIdentifier) {
             for (let target of WI.targets)
-                target.DebuggerAgent.removeBreakpoint(breakpoint.identifier, didRemoveBreakpoint.bind(this));
+                target.DebuggerAgent.removeBreakpoint(breakpoint.identifier, didRemoveBreakpoint.bind(this, target));
         } else if (breakpoint.scriptIdentifier) {
             let target = breakpoint.target;
-            target.DebuggerAgent.removeBreakpoint(breakpoint.identifier, didRemoveBreakpoint.bind(this));
+            target.DebuggerAgent.removeBreakpoint(breakpoint.identifier, didRemoveBreakpoint.bind(this, target));
         }
     }
 
@@ -1034,10 +1034,10 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
             return;
 
         // Remove the breakpoint with its old id.
-        this._removeBreakpoint(breakpoint, () => {
+        this._removeBreakpoint(breakpoint, (target) => {
             // Add the breakpoint at its new lineNumber and get a new id.
             this._restoringBreakpoints = true;
-            this._setBreakpoint(breakpoint);
+            this._setBreakpoint(breakpoint, target);
             this._restoringBreakpoints = false;
 
             this.dispatchEventToListeners(WI.DebuggerManager.Event.BreakpointMoved, {breakpoint});
@@ -1101,10 +1101,10 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
             return;
 
         // Remove the breakpoint with its old id.
-        this._removeBreakpoint(breakpoint, () => {
+        this._removeBreakpoint(breakpoint, (target) => {
             // Add the breakpoint with its new properties and get a new id.
             this._restoringBreakpoints = true;
-            this._setBreakpoint(breakpoint);
+            this._setBreakpoint(breakpoint, target);
             this._restoringBreakpoints = false;
         });
     }