From 2f28b50281f39bea8d83afa139326d37257cf474 Mon Sep 17 00:00:00 2001 From: "Brian J. Burg" Date: Sun, 8 Dec 2013 23:09:13 -0800 Subject: [PATCH] Probes: fix breakpoint mode inconsistency when creating from popup menu. Fixes #419. This fixes two things: The Breakpoint.mode setter didn't check the validity of the supplied mode. Now, if you try to set auto-continue when no actions are attached, it will instead set the mode to enabled. There was a race between setting the probe breakpoint action and setting the breakpoint's mode to auto-continue. A short timer was added as a temporary workaround until Breakpoints have an API for batching mutations. --- Source/WebInspectorUI/UserInterface/Breakpoint.js | 5 ++++- .../WebInspectorUI/UserInterface/DebuggerManager.js | 3 +++ Source/WebInspectorUI/UserInterface/TextEditor.js | 11 +++++++---- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Source/WebInspectorUI/UserInterface/Breakpoint.js b/Source/WebInspectorUI/UserInterface/Breakpoint.js index 678f282bcbdb2..4b318003943c7 100644 --- a/Source/WebInspectorUI/UserInterface/Breakpoint.js +++ b/Source/WebInspectorUI/UserInterface/Breakpoint.js @@ -146,7 +146,10 @@ WebInspector.Breakpoint.prototype = { if (this._mode === mode) return; - this._mode = mode || WebInspector.Breakpoint.Mode.Enabled; + if (!mode || (mode === WebInspector.Breakpoint.Mode.AutoContinue && !this.actions.length)) + mode = WebInspector.Breakpoint.Mode.Enabled; + + this._mode = mode; this.dispatchEventToListeners(WebInspector.Breakpoint.Event.ModeDidChange); }, diff --git a/Source/WebInspectorUI/UserInterface/DebuggerManager.js b/Source/WebInspectorUI/UserInterface/DebuggerManager.js index a9db002dcad6a..9c7de0128a1e1 100644 --- a/Source/WebInspectorUI/UserInterface/DebuggerManager.js +++ b/Source/WebInspectorUI/UserInterface/DebuggerManager.js @@ -630,6 +630,9 @@ WebInspector.DebuggerManager.prototype = { return; // If the breakpoint exists in the backend, remove it. + // FIXME: several outstanding modifications may race as the first + // one triggers removal of the breakpoint.id, and subsequent + // modifications take the else branch and set the 'new' breakpoint first. if (breakpoint.id) this._removeBreakpoint(breakpoint, setNewBreakpoint.bind(this)); else diff --git a/Source/WebInspectorUI/UserInterface/TextEditor.js b/Source/WebInspectorUI/UserInterface/TextEditor.js index 059b105eb14bd..42fb754e3c42d 100644 --- a/Source/WebInspectorUI/UserInterface/TextEditor.js +++ b/Source/WebInspectorUI/UserInterface/TextEditor.js @@ -1005,12 +1005,15 @@ WebInspector.TextEditor.prototype = { } } - // If we created the breakpoint just for the probe, default to auto-continue. - if (didCreateBreakpoint) - foundBreakpoint.mode = WebInspector.Breakpoint.Mode.AutoContinue; - var newAction = foundBreakpoint.createAction(WebInspector.BreakpointAction.Type.Probe); newAction.data = expression; + + // If we created the breakpoint just for the probe, default to auto-continue (after action is added). + // FIXME: this is a horrible hack to mitigate protocol races on multiple breakpoint mutations. This could + // be fixed by adding lightweight mutation batching to Breakpoint, like CodeMirror does for DOM updates. + if (didCreateBreakpoint) + setTimeout(function() { foundBreakpoint.mode = WebInspector.Breakpoint.Mode.AutoContinue; }, 100); + popover.dismiss(); }