[webkit-reviews] review granted: [Bug 38269] Web Inspector: Allow editing script resources when resource tracking is enabled. : [Attachment 54579] [PATCH] Proposed change.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 28 10:57:27 PDT 2010
Yury Semikhatsky <yurys at chromium.org> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 38269: Web Inspector: Allow editing script resources when resource tracking
is enabled.
https://bugs.webkit.org/show_bug.cgi?id=38269
Attachment 54579: [PATCH] Proposed change.
https://bugs.webkit.org/attachment.cgi?id=54579&action=review
------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 23a6cdb..71333ec 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,24 @@
> +2010-04-28 Pavel Feldman <pfeldman at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + Web Inspector: Allow editing script resources when resource tracking
is enabled.
> +
> + https://bugs.webkit.org/show_bug.cgi?id=38269
> +
> + * inspector/front-end/ScriptView.js:
> + * inspector/front-end/ScriptsPanel.js:
> + (WebInspector.ScriptsPanel.prototype.canEditScripts):
> + (WebInspector.ScriptsPanel.prototype.editScriptSource):
> + * inspector/front-end/SourceFrame.js:
> + (WebInspector.SourceFrame.prototype.get textModel):
> + * inspector/front-end/SourceView.js:
> + (WebInspector.SourceView):
> + (WebInspector.SourceView.prototype._addBreakpoint):
> + (WebInspector.SourceView.prototype._editLine):
> + (WebInspector.SourceView.prototype._editLineComplete):
> + (WebInspector.SourceView.prototype._sourceIDForLine):
> +
> 2010-04-28 Julien Chaffraix <jchaffraix at webkit.org>
>
> Reviewed by Alexey Proskuryakov.
> diff --git a/WebCore/inspector/front-end/ScriptView.js
b/WebCore/inspector/front-end/ScriptView.js
> index e55a685..cb10889 100644
> --- a/WebCore/inspector/front-end/ScriptView.js
> +++ b/WebCore/inspector/front-end/ScriptView.js
> @@ -77,11 +77,6 @@ WebInspector.ScriptView.prototype = {
> WebInspector.panels.scripts.addBreakpoint(breakpoint);
> },
>
> - _editLine: function(line, newContent)
> - {
> - WebInspector.panels.scripts.editScriptLine(this.script.sourceID,
line, newContent, this._editLineComplete.bind(this));
> - },
> -
> _editLineComplete: function(newBody)
> {
> this.script.source = newBody;
> @@ -107,6 +102,7 @@ WebInspector.ScriptView.prototype = {
> _jumpToSearchResult:
WebInspector.SourceView.prototype._jumpToSearchResult,
> _sourceFrameSetupFinished:
WebInspector.SourceView.prototype._sourceFrameSetupFinished,
> _removeBreakpoint: WebInspector.SourceView.prototype._removeBreakpoint,
> + _editLine: WebInspector.SourceView.prototype._editLine,
> resize: WebInspector.SourceView.prototype.resize
> }
>
> diff --git a/WebCore/inspector/front-end/ScriptsPanel.js
b/WebCore/inspector/front-end/ScriptsPanel.js
> index 6f8b605..096fda0 100644
> --- a/WebCore/inspector/front-end/ScriptsPanel.js
> +++ b/WebCore/inspector/front-end/ScriptsPanel.js
> @@ -359,10 +359,10 @@ WebInspector.ScriptsPanel.prototype = {
>
> canEditScripts: function()
> {
> - return !!InspectorBackend.editScriptLine;
> + return !!InspectorBackend.editScriptSource;
> },
>
> - editScriptLine: function(sourceID, line, newContent, callback)
> + editScriptSource: function(sourceID, newContent, line,
linesCountToShift, callback)
> {
> if (!this.canEditScripts())
> return;
> @@ -376,7 +376,6 @@ WebInspector.ScriptsPanel.prototype = {
> newBreakpoints.push(breakpoint);
> }
>
> - var linesCountToShift = newContent.split("\n").length - 1;
> function mycallback(newBody)
> {
> callback(newBody);
> @@ -388,7 +387,7 @@ WebInspector.ScriptsPanel.prototype = {
> }
> };
> var callbackId = WebInspector.Callback.wrap(mycallback.bind(this))
> - InspectorBackend.editScriptLine(callbackId, sourceID, line,
newContent);
> + InspectorBackend.editScriptSource(callbackId, sourceID, newContent);
> },
>
> selectedCallFrameId: function()
> @@ -1004,4 +1003,4 @@ WebInspector.ScriptsPanel.prototype = {
>
> WebInspector.ScriptsPanel.prototype.__proto__ =
WebInspector.Panel.prototype;
>
> -WebInspector.didEditScriptLine = WebInspector.Callback.processCallback;
> +WebInspector.didEditScriptSource = WebInspector.Callback.processCallback;
> diff --git a/WebCore/inspector/front-end/SourceFrame.js
b/WebCore/inspector/front-end/SourceFrame.js
> index 60fda58..62f9222 100644
> --- a/WebCore/inspector/front-end/SourceFrame.js
> +++ b/WebCore/inspector/front-end/SourceFrame.js
> @@ -152,6 +152,11 @@ WebInspector.SourceFrame.prototype = {
> this._textModel.setText(null, content);
> },
>
> + get textModel()
> + {
> + return this._textModel;
> + },
> +
> highlightLine: function(line)
> {
> if (this._textViewer)
> diff --git a/WebCore/inspector/front-end/SourceView.js
b/WebCore/inspector/front-end/SourceView.js
> index 9fbd161..8aa8bd2 100644
> --- a/WebCore/inspector/front-end/SourceView.js
> +++ b/WebCore/inspector/front-end/SourceView.js
> @@ -32,7 +32,8 @@ WebInspector.SourceView = function(resource)
>
> this.element.addStyleClass("source");
>
> - this.sourceFrame = new WebInspector.SourceFrame(this.contentElement,
this._addBreakpoint.bind(this), this._removeBreakpoint.bind(this));
> + var canEditScripts = WebInspector.panels.scripts.canEditScripts() &&
resource.type === WebInspector.Resource.Type.Script;
> + this.sourceFrame = new WebInspector.SourceFrame(this.contentElement,
this._addBreakpoint.bind(this), this._removeBreakpoint.bind(this),
canEditScripts ? this._editLine.bind(this) : null);
> resource.addEventListener("finished", this._resourceLoadingFinished,
this);
> this._frameNeedsSetup = true;
> }
> @@ -105,17 +106,7 @@ WebInspector.SourceView.prototype = {
>
> _addBreakpoint: function(line)
> {
> - var sourceID = null;
> - var closestStartingLine = 0;
> - var scripts = this.resource.scripts;
> - for (var i = 0; i < scripts.length; ++i) {
> - var script = scripts[i];
> - if (script.startingLine <= line && script.startingLine >=
closestStartingLine) {
> - closestStartingLine = script.startingLine;
> - sourceID = script.sourceID;
> - }
> - }
> -
> + var sourceID = this._sourceIDForLine(line);
> if (WebInspector.panels.scripts) {
> var breakpoint = new WebInspector.Breakpoint(this.resource.url,
line, sourceID);
> WebInspector.panels.scripts.addBreakpoint(breakpoint);
> @@ -128,6 +119,41 @@ WebInspector.SourceView.prototype = {
> WebInspector.panels.scripts.removeBreakpoint(breakpoint);
> },
>
> + _editLine: function(line, newContent)
> + {
> + var lines = [];
> + var textModel = this.sourceFrame.textModel;
> + for (var i = 0; i < textModel.linesCount; ++i) {
> + if (i === line)
> + lines.push(newContent);
> + else
> + lines.push(textModel.line(i));
> + }
> +
> + var linesCountToShift = newContent.split("\n").length - 1;
> +
WebInspector.panels.scripts.editScriptSource(this._sourceIDForLine(line),
lines.join("\n"), line, linesCountToShift, this._editLineComplete.bind(this));
> + },
> +
> + _editLineComplete: function(newBody)
> + {
> + this.sourceFrame.updateContent(newBody);
> + },
> +
> + _sourceIDForLine: function(line)
> + {
> + var sourceID = null;
> + var closestStartingLine = 0;
> + var scripts = this.resource.scripts;
> + for (var i = 0; i < scripts.length; ++i) {
> + var script = scripts[i];
> + if (script.startingLine <= line && script.startingLine >=
closestStartingLine) {
> + closestStartingLine = script.startingLine;
> + sourceID = script.sourceID;
> + }
> + }
> + return sourceID;
> + },
> +
> // The rest of the methods in this prototype need to be generic enough
to work with a ScriptView.
> // The ScriptView prototype pulls these methods into it's prototype to
avoid duplicate code.
>
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> index 83b0488..4410424 100644
> --- a/WebKit/chromium/ChangeLog
> +++ b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,16 @@
> +2010-04-28 Pavel Feldman <pfeldman at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + Web Inspector: Allow editing script resources when resource tracking
is enabled.
> +
> + http://bugs.webkit.org/show_bug.cgi?id=38269
> +
> + * src/js/DebuggerAgent.js:
> + * src/js/InspectorControllerImpl.js:
> + (.devtools.InspectorBackendImpl.prototype.setBreakpoint):
> + (.devtools.InspectorBackendImpl.prototype.editScriptSource):
> +
> 2010-04-28 Yury Semikhatsky <yurys at chromium.org>
>
> Reviewed by Pavel Feldman.
> diff --git a/WebKit/chromium/src/js/DebuggerAgent.js
b/WebKit/chromium/src/js/DebuggerAgent.js
> index 2281ad2..67e54aa 100644
> --- a/WebKit/chromium/src/js/DebuggerAgent.js
> +++ b/WebKit/chromium/src/js/DebuggerAgent.js
> @@ -309,18 +309,11 @@ devtools.DebuggerAgent.prototype.addBreakpoint =
function(sourceId, line, enable
> /**
> * Changes given line of the script.
> */
> -devtools.DebuggerAgent.prototype.editScriptLine = function(sourceId, line,
newContent, callback)
> +devtools.DebuggerAgent.prototype.editScriptSource = function(sourceId,
newContent, callback)
> {
> - var script = this.parsedScripts_[sourceId];
> - if (!script || !script.source)
> - return;
> -
> - var lines = script.source.split("\n");
> - lines[line] = newContent;
> -
> var commandArguments = {
> "script_id": sourceId,
> - "new_source": lines.join("\n")
> + "new_source": newContent
> };
>
> var cmd = new devtools.DebugCommand("changelive", commandArguments);
> diff --git a/WebKit/chromium/src/js/InspectorControllerImpl.js
b/WebKit/chromium/src/js/InspectorControllerImpl.js
> index becc076..aca37a8 100644
> --- a/WebKit/chromium/src/js/InspectorControllerImpl.js
> +++ b/WebKit/chromium/src/js/InspectorControllerImpl.js
> @@ -139,10 +139,10 @@
devtools.InspectorBackendImpl.prototype.removeBreakpoint = function(sourceID,
li
> };
>
>
> -devtools.InspectorBackendImpl.prototype.editScriptLine = function(callID,
sourceID, line, newContent)
> +devtools.InspectorBackendImpl.prototype.editScriptSource = function(callID,
sourceID, newContent)
> {
> - devtools.tools.getDebuggerAgent().editScriptLine(sourceID, line,
newContent, function(newFullBody) {
> - WebInspector.didEditScriptLine(callID, newFullBody);
> + devtools.tools.getDebuggerAgent().editScriptSource(sourceID, newContent,
function(newFullBody) {
> + WebInspector.didEditScriptSource(callID, newFullBody);
> });
> };
>
WebCore/inspector/front-end/SourceView.js:146
+ var scripts = this.resource.scripts;
Is it always defined?
More information about the webkit-reviews
mailing list