- Revision
- 201019
- Author
- [email protected]
- Date
- 2016-05-17 10:33:20 -0700 (Tue, 17 May 2016)
Log Message
Web Inspector: breakpoints in sourceURL named scripts are not persisted
https://bugs.webkit.org/show_bug.cgi?id=157714
<rdar://problem/26287099>
Reviewed by Joseph Pecoraro.
The Inspector frontend doesn't try to persist a breakpoint that
lacks a URL, even if the breakpoint has a sourceURL. Similarly, for
breakpoints without a URL, the frontend asks the backend to create
the breakpoint for a specific script identifier rather than a
URL-based breakpoint. This prevents breakpoints in injected scripts
from being resolved if the page is reloaded.
The Inspector backend knows how to resolve URL-based breakpoints
by matching against the script's URL or sourceURL, so we just need
to teach the frontend when either is appropriate to use.
This patch adds SourceCode.contentIdentifier, which is roughly
`url || sourceURL` for content that is not emphemeral, such as
console evaluations. Change breakpoint and debugger code to use
`contentIdentifier` rather than `url`, and pass contentIdentifier
to the backend when setting a breakpoint by URL.
* UserInterface/Controllers/DebuggerManager.js:
(WebInspector.DebuggerManager.prototype.breakpointsForSourceCode):
(WebInspector.DebuggerManager.prototype.scriptsForURL):
(WebInspector.DebuggerManager.prototype.get searchableScripts):
(WebInspector.DebuggerManager.prototype.removeBreakpoint):
(WebInspector.DebuggerManager.prototype.reset):
(WebInspector.DebuggerManager.prototype.scriptDidParse):
(WebInspector.DebuggerManager.prototype._setBreakpoint):
(WebInspector.DebuggerManager.prototype._saveBreakpoints):
(WebInspector.DebuggerManager.prototype._associateBreakpointsWithSourceCode):
* UserInterface/Models/Breakpoint.js:
(WebInspector.Breakpoint):
(WebInspector.Breakpoint.prototype.get contentIdentifier):
(WebInspector.Breakpoint.prototype.get info):
(WebInspector.Breakpoint.prototype.saveIdentityToCookie):
(WebInspector.Breakpoint.prototype.get url): Deleted.
Replace uses of Breakpoint.prototype.get url with
contentIdentifier inside the class and at all callsites.
* UserInterface/Models/Script.js:
(WebInspector.Script.prototype.get contentIdentifier):
Added. Use the URL, or the sourceURL unless the script is
ephemeral and only run once, like as a console evaluation.
* UserInterface/Models/SourceCode.js:
(WebInspector.SourceCode.prototype.get url): Added.
All subclasses already override this getter. Add it here so that
the default implementation of contentIdentifier can use it.
(WebInspector.SourceCode.prototype.get contentIdentifier): Added.
* UserInterface/Views/BreakpointTreeElement.js:
(WebInspector.BreakpointTreeElement.prototype.get filterableData):
* UserInterface/Views/ProbeSetDetailsSection.js:
(WebInspector.ProbeSetDetailsSection.prototype._updateLinkElement):
* UserInterface/Views/SourceCodeTextEditor.js:
(WebInspector.SourceCodeTextEditor.prototype._matchesBreakpoint):
* UserInterface/Views/TextResourceContentView.js:
(WebInspector.TextResourceContentView.prototype.get supplementalRepresentedObjects):
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (201018 => 201019)
--- trunk/Source/WebInspectorUI/ChangeLog 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/ChangeLog 2016-05-17 17:33:20 UTC (rev 201019)
@@ -1,3 +1,69 @@
+2016-05-17 Brian Burg <[email protected]>
+
+ Web Inspector: breakpoints in sourceURL named scripts are not persisted
+ https://bugs.webkit.org/show_bug.cgi?id=157714
+ <rdar://problem/26287099>
+
+ Reviewed by Joseph Pecoraro.
+
+ The Inspector frontend doesn't try to persist a breakpoint that
+ lacks a URL, even if the breakpoint has a sourceURL. Similarly, for
+ breakpoints without a URL, the frontend asks the backend to create
+ the breakpoint for a specific script identifier rather than a
+ URL-based breakpoint. This prevents breakpoints in injected scripts
+ from being resolved if the page is reloaded.
+
+ The Inspector backend knows how to resolve URL-based breakpoints
+ by matching against the script's URL or sourceURL, so we just need
+ to teach the frontend when either is appropriate to use.
+
+ This patch adds SourceCode.contentIdentifier, which is roughly
+ `url || sourceURL` for content that is not emphemeral, such as
+ console evaluations. Change breakpoint and debugger code to use
+ `contentIdentifier` rather than `url`, and pass contentIdentifier
+ to the backend when setting a breakpoint by URL.
+
+ * UserInterface/Controllers/DebuggerManager.js:
+ (WebInspector.DebuggerManager.prototype.breakpointsForSourceCode):
+ (WebInspector.DebuggerManager.prototype.scriptsForURL):
+ (WebInspector.DebuggerManager.prototype.get searchableScripts):
+ (WebInspector.DebuggerManager.prototype.removeBreakpoint):
+ (WebInspector.DebuggerManager.prototype.reset):
+ (WebInspector.DebuggerManager.prototype.scriptDidParse):
+ (WebInspector.DebuggerManager.prototype._setBreakpoint):
+ (WebInspector.DebuggerManager.prototype._saveBreakpoints):
+ (WebInspector.DebuggerManager.prototype._associateBreakpointsWithSourceCode):
+
+ * UserInterface/Models/Breakpoint.js:
+ (WebInspector.Breakpoint):
+ (WebInspector.Breakpoint.prototype.get contentIdentifier):
+ (WebInspector.Breakpoint.prototype.get info):
+ (WebInspector.Breakpoint.prototype.saveIdentityToCookie):
+ (WebInspector.Breakpoint.prototype.get url): Deleted.
+ Replace uses of Breakpoint.prototype.get url with
+ contentIdentifier inside the class and at all callsites.
+
+ * UserInterface/Models/Script.js:
+ (WebInspector.Script.prototype.get contentIdentifier):
+ Added. Use the URL, or the sourceURL unless the script is
+ ephemeral and only run once, like as a console evaluation.
+
+ * UserInterface/Models/SourceCode.js:
+ (WebInspector.SourceCode.prototype.get url): Added.
+ All subclasses already override this getter. Add it here so that
+ the default implementation of contentIdentifier can use it.
+
+ (WebInspector.SourceCode.prototype.get contentIdentifier): Added.
+
+ * UserInterface/Views/BreakpointTreeElement.js:
+ (WebInspector.BreakpointTreeElement.prototype.get filterableData):
+ * UserInterface/Views/ProbeSetDetailsSection.js:
+ (WebInspector.ProbeSetDetailsSection.prototype._updateLinkElement):
+ * UserInterface/Views/SourceCodeTextEditor.js:
+ (WebInspector.SourceCodeTextEditor.prototype._matchesBreakpoint):
+ * UserInterface/Views/TextResourceContentView.js:
+ (WebInspector.TextResourceContentView.prototype.get supplementalRepresentedObjects):
+
2016-05-16 Joseph Pecoraro <[email protected]>
Uncaught Exception: TypeError: null is not an object (evaluating 'event.data.pathComponent.domTreeElement')
Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js (201018 => 201019)
--- trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js 2016-05-17 17:33:20 UTC (rev 201019)
@@ -53,7 +53,7 @@
this._allUncaughtExceptionsBreakpoint = new WebInspector.Breakpoint(specialBreakpointLocation, !this._allUncaughtExceptionsBreakpointEnabledSetting.value);
this._breakpoints = [];
- this._breakpointURLMap = new Map;
+ this._breakpointContentIdentifierMap = new Map;
this._breakpointScriptIdentifierMap = new Map;
this._breakpointIdMap = new Map;
@@ -65,7 +65,7 @@
this._internalWebKitScripts = [];
this._scriptIdMap = new Map;
- this._scriptURLMap = new Map;
+ this._scriptContentIdentifierMap = new Map;
this._breakpointsSetting = new WebInspector.Setting("breakpoints", []);
this._breakpointsEnabledSetting = new WebInspector.Setting("breakpoints-enabled", true);
@@ -288,10 +288,10 @@
});
}
- let urlBreakpoints = this._breakpointURLMap.get(sourceCode.url);
- if (urlBreakpoints) {
- this._associateBreakpointsWithSourceCode(urlBreakpoints, sourceCode);
- return urlBreakpoints;
+ let contentIdentifierBreakpoints = this._breakpointContentIdentifierMap.get(sourceCode.contentIdentifier);
+ if (contentIdentifierBreakpoints) {
+ this._associateBreakpointsWithSourceCode(contentIdentifierBreakpoints, sourceCode);
+ return contentIdentifierBreakpoints;
}
if (sourceCode instanceof WebInspector.Script) {
@@ -318,7 +318,7 @@
scriptsForURL(url)
{
// FIXME: This may not be safe. A Resource's URL may differ from a Script's URL.
- return this._scriptURLMap.get(url) || [];
+ return this._scriptContentIdentifierMap.get(url) || [];
}
continueToLocation(scriptIdentifier, lineNumber, columnNumber)
@@ -328,7 +328,7 @@
get searchableScripts()
{
- return this.knownNonResourceScripts.filter((script) => !!script.url);
+ return this.knownNonResourceScripts.filter((script) => !!script.contentIdentifier);
}
get knownNonResourceScripts()
@@ -379,13 +379,13 @@
if (!breakpoint)
return;
- if (breakpoint.url) {
- let urlBreakpoints = this._breakpointURLMap.get(breakpoint.url);
- if (!urlBreakpoints) {
- urlBreakpoints = [];
- this._breakpointURLMap.set(breakpoint.url, urlBreakpoints);
+ if (breakpoint.contentIdentifier) {
+ let contentIdentifierBreakpoints = this._breakpointContentIdentifierMap.get(breakpoint.contentIdentifier);
+ if (!contentIdentifierBreakpoints) {
+ contentIdentifierBreakpoints = [];
+ this._breakpointContentIdentifierMap.set(breakpoint.contentIdentifier, contentIdentifierBreakpoints);
}
- urlBreakpoints.push(breakpoint);
+ contentIdentifierBreakpoints.push(breakpoint);
}
if (breakpoint.scriptIdentifier) {
@@ -427,12 +427,12 @@
if (breakpoint.identifier)
this._removeBreakpoint(breakpoint);
- if (breakpoint.url) {
- let urlBreakpoints = this._breakpointURLMap.get(breakpoint.url);
- if (urlBreakpoints) {
- urlBreakpoints.remove(breakpoint);
- if (!urlBreakpoints.length)
- this._breakpointURLMap.delete(breakpoint.url);
+ if (breakpoint.contentIdentifier) {
+ let contentIdentifierBreakpoints = this._breakpointContentIdentifierMap.get(breakpoint.contentIdentifier);
+ if (contentIdentifierBreakpoints) {
+ contentIdentifierBreakpoints.remove(breakpoint);
+ if (!contentIdentifierBreakpoints.length)
+ this._breakpointContentIdentifierMap.delete(breakpoint.contentIdentifier);
}
}
@@ -487,7 +487,7 @@
this._internalWebKitScripts = [];
this._scriptIdMap.clear();
- this._scriptURLMap.clear();
+ this._scriptContentIdentifierMap.clear();
this._ignoreBreakpointDisplayLocationDidChangeEvent = true;
@@ -591,11 +591,11 @@
this._scriptIdMap.set(scriptIdentifier, script);
- if (script.url) {
- let scripts = this._scriptURLMap.get(script.url);
+ if (script.contentIdentifier) {
+ let scripts = this._scriptContentIdentifierMap.get(script.contentIdentifier);
if (!scripts) {
scripts = [];
- this._scriptURLMap.set(script.url, scripts);
+ this._scriptContentIdentifierMap.set(script.contentIdentifier, scripts);
}
scripts.push(script);
}
@@ -778,10 +778,10 @@
// COMPATIBILITY (iOS 7): iOS 7 and earlier, DebuggerAgent.setBreakpoint* took a "condition" string argument.
// This has been replaced with an "options" BreakpointOptions object.
- if (breakpoint.url) {
+ if (breakpoint.contentIdentifier) {
DebuggerAgent.setBreakpointByUrl.invoke({
lineNumber: breakpoint.sourceCodeLocation.lineNumber,
- url: breakpoint.url,
+ url: breakpoint.contentIdentifier,
urlRegex: undefined,
columnNumber: breakpoint.sourceCodeLocation.columnNumber,
condition: breakpoint.condition,
@@ -949,7 +949,7 @@
if (this._restoringBreakpoints)
return;
- let breakpointsToSave = this._breakpoints.filter((breakpoint) => !!breakpoint.url);
+ let breakpointsToSave = this._breakpoints.filter((breakpoint) => !!breakpoint.contentIdentifier);
let serializedBreakpoints = breakpointsToSave.map((breakpoint) => breakpoint.info);
this._breakpointsSetting.value = serializedBreakpoints;
}
@@ -963,7 +963,7 @@
if (breakpoint.sourceCodeLocation.sourceCode === null)
breakpoint.sourceCodeLocation.sourceCode = sourceCode;
// SourceCodes can be unequal if the SourceCodeLocation is associated with a Script and we are looking at the Resource.
- console.assert(breakpoint.sourceCodeLocation.sourceCode === sourceCode || breakpoint.sourceCodeLocation.sourceCode.url ="" sourceCode.url);
+ console.assert(breakpoint.sourceCodeLocation.sourceCode === sourceCode || breakpoint.sourceCodeLocation.sourceCode.contentIdentifier === sourceCode.contentIdentifier);
}
this._ignoreBreakpointDisplayLocationDidChangeEvent = false;
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/Breakpoint.js (201018 => 201019)
--- trunk/Source/WebInspectorUI/UserInterface/Models/Breakpoint.js 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/Breakpoint.js 2016-05-17 17:33:20 UTC (rev 201019)
@@ -31,11 +31,12 @@
if (sourceCodeLocationOrInfo instanceof WebInspector.SourceCodeLocation) {
var sourceCode = sourceCodeLocationOrInfo.sourceCode;
- var url = "" ? sourceCode.url : null;
+ var contentIdentifier = sourceCode ? sourceCode.contentIdentifier : null;
var scriptIdentifier = sourceCode instanceof WebInspector.Script ? sourceCode.id : null;
var location = sourceCodeLocationOrInfo;
} else if (sourceCodeLocationOrInfo && typeof sourceCodeLocationOrInfo === "object") {
- var url = ""
+ // The 'url' fallback is for transitioning from older frontends and should be removed.
+ var contentIdentifier = sourceCodeLocationOrInfo.contentIdentifier || sourceCodeLocationOrInfo.url;
var lineNumber = sourceCodeLocationOrInfo.lineNumber || 0;
var columnNumber = sourceCodeLocationOrInfo.columnNumber || 0;
var location = new WebInspector.SourceCodeLocation(null, lineNumber, columnNumber);
@@ -50,7 +51,7 @@
console.error("Unexpected type passed to WebInspector.Breakpoint", sourceCodeLocationOrInfo);
this._id = null;
- this._url = url || null;
+ this._contentIdentifier = contentIdentifier || null;
this._scriptIdentifier = scriptIdentifier || null;
this._disabled = disabled || false;
this._condition = condition || "";
@@ -76,9 +77,9 @@
this._id = id || null;
}
- get url()
+ get contentIdentifier()
{
- return this._url;
+ return this._contentIdentifier;
}
get scriptIdentifier()
@@ -196,7 +197,7 @@
{
// The id, scriptIdentifier and resolved state are tied to the current session, so don't include them for serialization.
return {
- url: this._url,
+ contentIdentifier: this._contentIdentifier,
lineNumber: this._sourceCodeLocation.lineNumber,
columnNumber: this._sourceCodeLocation.columnNumber,
disabled: this._disabled,
@@ -299,7 +300,7 @@
saveIdentityToCookie(cookie)
{
- cookie[WebInspector.Breakpoint.URLCookieKey] = this.url;
+ cookie[WebInspector.Breakpoint.ContentIdentifierCookieKey] = this.contentIdentifier;
cookie[WebInspector.Breakpoint.LineNumberCookieKey] = this.sourceCodeLocation.lineNumber;
cookie[WebInspector.Breakpoint.ColumnNumberCookieKey] = this.sourceCodeLocation.columnNumber;
}
@@ -340,7 +341,7 @@
WebInspector.Breakpoint.DefaultBreakpointActionType = WebInspector.BreakpointAction.Type.Log;
WebInspector.Breakpoint.TypeIdentifier = "breakpoint";
-WebInspector.Breakpoint.URLCookieKey = "breakpoint-url";
+WebInspector.Breakpoint.ContentIdentifierCookieKey = "breakpoint-content-identifier";
WebInspector.Breakpoint.LineNumberCookieKey = "breakpoint-line-number";
WebInspector.Breakpoint.ColumnNumberCookieKey = "breakpoint-column-number";
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/Script.js (201018 => 201019)
--- trunk/Source/WebInspectorUI/UserInterface/Models/Script.js 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/Script.js 2016-05-17 17:33:20 UTC (rev 201019)
@@ -90,6 +90,28 @@
return this._url;
}
+ get contentIdentifier()
+ {
+ if (this._url)
+ return this._url;
+
+ if (!this._sourceURL)
+ return null;
+
+ // Since reused content identifiers can cause breakpoints
+ // to show up in completely unrelated files, sourceURLs should
+ // be unique where possible. The checks below exclude cases
+ // where sourceURLs are intentionally reused and we would never
+ // expect a breakpoint to be persisted across sessions.
+ if (isWebInspectorConsoleEvaluationScript(this._sourceURL))
+ return null;
+
+ if (isWebInspectorInternalScript(this._sourceURL))
+ return null;
+
+ return this._sourceURL;
+ }
+
get sourceURL()
{
return this._sourceURL;
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/SourceCode.js (201018 => 201019)
--- trunk/Source/WebInspectorUI/UserInterface/Models/SourceCode.js 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/SourceCode.js 2016-05-17 17:33:20 UTC (rev 201019)
@@ -76,6 +76,20 @@
return this._currentRevision.content;
}
+ get url()
+ {
+ // To be overridden by subclasses.
+ }
+
+ get contentIdentifier()
+ {
+ // A contentIdentifier is roughly `url || sourceURL` for cases where
+ // the content is consistent between sessions and not ephemeral.
+
+ // Can be overridden by subclasses if better behavior is possible.
+ return this.url;
+ }
+
get sourceMaps()
{
return this._sourceMaps || [];
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.js (201018 => 201019)
--- trunk/Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.js 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.js 2016-05-17 17:33:20 UTC (rev 201019)
@@ -72,7 +72,7 @@
get filterableData()
{
- return {text: [this.breakpoint.url]};
+ return {text: [this.breakpoint.contentIdentifier]};
}
ondelete()
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ProbeSetDetailsSection.js (201018 => 201019)
--- trunk/Source/WebInspectorUI/UserInterface/Views/ProbeSetDetailsSection.js 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ProbeSetDetailsSection.js 2016-05-17 17:33:20 UTC (rev 201019)
@@ -96,7 +96,7 @@
console.assert(!breakpoint.resolved);
var location = breakpoint.sourceCodeLocation;
- titleElement = WebInspector.linkifyLocation(breakpoint.url, location.displayLineNumber, location.displayColumnNumber);
+ titleElement = WebInspector.linkifyLocation(breakpoint.contentIdentifier, location.displayLineNumber, location.displayColumnNumber);
}
titleElement.classList.add(WebInspector.ProbeSetDetailsSection.DontFloatLinkStyleClassName);
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js (201018 => 201019)
--- trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js 2016-05-17 17:33:20 UTC (rev 201019)
@@ -788,9 +788,9 @@
if (this._sourceCode instanceof WebInspector.SourceMapResource)
return breakpoint.sourceCodeLocation.displaySourceCode === this._sourceCode;
if (this._sourceCode instanceof WebInspector.Resource)
- return breakpoint.url ="" this._sourceCode.url;
+ return breakpoint.contentIdentifier === this._sourceCode.contentIdentifier;
if (this._sourceCode instanceof WebInspector.Script)
- return breakpoint.url ="" this._sourceCode.url || breakpoint.scriptIdentifier === this._sourceCode.id;
+ return breakpoint.contentIdentifier === this._sourceCode.contentIdentifier || breakpoint.scriptIdentifier === this._sourceCode.id;
return false;
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/TextResourceContentView.js (201018 => 201019)
--- trunk/Source/WebInspectorUI/UserInterface/Views/TextResourceContentView.js 2016-05-17 17:07:19 UTC (rev 201018)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/TextResourceContentView.js 2016-05-17 17:33:20 UTC (rev 201019)
@@ -78,7 +78,7 @@
get supplementalRepresentedObjects()
{
var objects = WebInspector.probeManager.probeSets.filter(function(probeSet) {
- return this._resource.url ="" probeSet.breakpoint.url;
+ return this._resource.contentIdentifier === probeSet.breakpoint.contentIdentifier;
}, this);
// If the SourceCodeTextEditor has an executionLineNumber, we can assume
