Title: [171784] trunk/Source/WebInspectorUI
- Revision
- 171784
- Author
- b...@cs.washington.edu
- Date
- 2014-07-29 20:48:54 -0700 (Tue, 29 Jul 2014)
Log Message
Web Inspector: breakpoints are always speculatively resolved when restored from local storage
https://bugs.webkit.org/show_bug.cgi?id=135396
Reviewed by Timothy Hatcher.
A longstanding quirk/optimization in the frontend is that we immediately set a breakpoint
as resolved if the breakpoint was successfully set in the backend. This ensures that clicking in
the gutter immediately produces a resolved breakpoint with only one round-trip.
However, not all breakpoints should be speculatively resolved, because the corresponding resource
may not be loaded yet. This situation causes problems for code that assumes a resolved breakpoint
also has a valid sourceCodeLocation.sourceCode.
* UserInterface/Controllers/DebuggerManager.js:
(WebInspector.DebuggerManager.restoreBreakpointsSoon): Don't leak the variable to global scope.
(WebInspector.DebuggerManager):
(WebInspector.DebuggerManager.prototype.speculativelyResolveBreakpoint):
(WebInspector.DebuggerManager.prototype.addBreakpoint): Speculatively resolve here if requested
using the success callback passed to _setBreakpoint.
(WebInspector.DebuggerManager.prototype.didSetBreakpoint): Emit simulated
Debugger.breakpointResolved events since they are only sent by the backend when a script is parsed.
(WebInspector.DebuggerManager.prototype._setBreakpoint):
* UserInterface/Views/SourceCodeTextEditor.js:
(WebInspector.SourceCodeTextEditor.prototype.textEditorBreakpointAdded): Request speculative resolve.
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (171783 => 171784)
--- trunk/Source/WebInspectorUI/ChangeLog 2014-07-30 00:30:52 UTC (rev 171783)
+++ trunk/Source/WebInspectorUI/ChangeLog 2014-07-30 03:48:54 UTC (rev 171784)
@@ -1,3 +1,32 @@
+2014-07-29 Brian J. Burg <b...@cs.washington.edu>
+
+ Web Inspector: breakpoints are always speculatively resolved when restored from local storage
+ https://bugs.webkit.org/show_bug.cgi?id=135396
+
+ Reviewed by Timothy Hatcher.
+
+ A longstanding quirk/optimization in the frontend is that we immediately set a breakpoint
+ as resolved if the breakpoint was successfully set in the backend. This ensures that clicking in
+ the gutter immediately produces a resolved breakpoint with only one round-trip.
+
+ However, not all breakpoints should be speculatively resolved, because the corresponding resource
+ may not be loaded yet. This situation causes problems for code that assumes a resolved breakpoint
+ also has a valid sourceCodeLocation.sourceCode.
+
+ * UserInterface/Controllers/DebuggerManager.js:
+ (WebInspector.DebuggerManager.restoreBreakpointsSoon): Don't leak the variable to global scope.
+ (WebInspector.DebuggerManager):
+ (WebInspector.DebuggerManager.prototype.speculativelyResolveBreakpoint):
+ (WebInspector.DebuggerManager.prototype.addBreakpoint): Speculatively resolve here if requested
+ using the success callback passed to _setBreakpoint.
+
+ (WebInspector.DebuggerManager.prototype.didSetBreakpoint): Emit simulated
+ Debugger.breakpointResolved events since they are only sent by the backend when a script is parsed.
+
+ (WebInspector.DebuggerManager.prototype._setBreakpoint):
+ * UserInterface/Views/SourceCodeTextEditor.js:
+ (WebInspector.SourceCodeTextEditor.prototype.textEditorBreakpointAdded): Request speculative resolve.
+
2014-07-29 Joseph Pecoraro <pecor...@apple.com>
Web Inspector: Eliminate more forced layouts during timeline recordings
Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js (171783 => 171784)
--- trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js 2014-07-30 00:30:52 UTC (rev 171783)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js 2014-07-30 03:48:54 UTC (rev 171784)
@@ -67,7 +67,7 @@
this._updateBreakOnExceptionsState();
function restoreBreakpointsSoon() {
- for (cookie of this._breakpointsSetting.value)
+ for (var cookie of this._breakpointsSetting.value)
this.addBreakpoint(new WebInspector.Breakpoint(cookie));
}
@@ -227,7 +227,7 @@
DebuggerAgent.continueToLocation({scriptId: scriptIdentifier, lineNumber: lineNumber, columnNumber: columnNumber});
},
- addBreakpoint: function(breakpoint, skipEventDispatch)
+ addBreakpoint: function(breakpoint, skipEventDispatch, shouldSpeculativelyResolve)
{
console.assert(breakpoint instanceof WebInspector.Breakpoint, "Bad argument to DebuggerManger.addBreakpoint: ", breakpoint);
if (!breakpoint)
@@ -249,8 +249,12 @@
this._breakpoints.push(breakpoint);
+ function speculativelyResolveBreakpoint(breakpoint) {
+ breakpoint.resolved = true;
+ }
+
if (!breakpoint.disabled)
- this._setBreakpoint(breakpoint);
+ this._setBreakpoint(breakpoint, shouldSpeculativelyResolve ? speculativelyResolveBreakpoint.bind(null, breakpoint) : null);
if (!skipEventDispatch)
this.dispatchEventToListeners(WebInspector.DebuggerManager.Event.BreakpointAdded, {breakpoint: breakpoint});
@@ -524,7 +528,7 @@
// a multi-step process for the user that can be confusing.
this.breakpointsEnabled = true;
- function didSetBreakpoint(error, breakpointIdentifier)
+ function didSetBreakpoint(error, breakpointIdentifier, locations)
{
if (error)
return;
@@ -532,8 +536,14 @@
this._breakpointIdMap[breakpointIdentifier] = breakpoint;
breakpoint.identifier = breakpointIdentifier;
- breakpoint.resolved = true;
+ // Debugger.setBreakpoint returns a single location.
+ if (!(locations instanceof Array))
+ locations = [locations];
+
+ for (var location of locations)
+ this.breakpointResolved(breakpointIdentifier, location);
+
if (typeof callback === "function")
callback();
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js (171783 => 171784)
--- trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js 2014-07-30 00:30:52 UTC (rev 171783)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js 2014-07-30 03:48:54 UTC (rev 171784)
@@ -839,7 +839,10 @@
this._addBreakpointWithEditorLineInfo(breakpoint, lineInfo);
this._ignoreBreakpointAddedBreakpoint = breakpoint;
- WebInspector.debuggerManager.addBreakpoint(breakpoint);
+
+ var shouldSkipEventDispatch = false;
+ var shouldSpeculativelyResolveBreakpoint = true;
+ WebInspector.debuggerManager.addBreakpoint(breakpoint, shouldSkipEventDispatch, shouldSpeculativelyResolveBreakpoint);
delete this._ignoreBreakpointAddedBreakpoint;
// Return the more accurate location and breakpoint info.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
