Bug 86099 – Web Inspector: pass source mapping into UISourceCode's constructor; move formatting outside mapping.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

This changes moves formatting out of the source mapping machinery.

[Pavel Feldman]

Created attachment 141169 [details]
Patch

[Pavel Podivilov]

Comment on attachment 141169 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141169&action=review

> Source/WebCore/inspector/front-end/BreakpointManager.js:70
> +            if (breakpoint._breakpointSourceCodeStorageId !== sourceFileId)

_sourceCodeStorageId?

> Source/WebCore/inspector/front-end/BreakpointManager.js:347
> +        var rawLocation = this._primaryUILocation.uiSourceCode.uiLocationToRawLocation(this._primaryUILocation.lineNumber, 0);

Looks like a UILocation's method.

> Source/WebCore/inspector/front-end/CallStackSidebarPane.js:150
> +            this._statusMessageElement.appendChild(status);

Shouldn't you remove this._statusMessageElement's children?

> Source/WebCore/inspector/front-end/JavaScriptSource.js:51
> +            WebInspector.debuggerPresentationModel.breakpointManager.restoreBreakpoints(this);

Maybe move this call to ScriptsPanel?

> Source/WebCore/inspector/front-end/JavaScriptSource.js:145
> +                this._togglingFormatter = true;

Maybe include this info in event?

> Source/WebCore/inspector/front-end/RawSourceCode.js:118
> +        return WebInspector.debuggerModel.createRawLocationByURL(this.url, lineNumber, columnNumber);

Wouldn't that broke breakpoints in anonymous scripts?

> Source/WebCore/inspector/front-end/ResourceScriptMapping.js:131
>          var rawSourceCode = /** @type {WebInspector.RawSourceCode} */ event.target;

Not used.

[Pavel Feldman]

Comment on attachment 141169 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141169&action=review

>> Source/WebCore/inspector/front-end/BreakpointManager.js:70
>> +            if (breakpoint._breakpointSourceCodeStorageId !== sourceFileId)
> 
> _sourceCodeStorageId?

Done.

>> Source/WebCore/inspector/front-end/BreakpointManager.js:347
>> +        var rawLocation = this._primaryUILocation.uiSourceCode.uiLocationToRawLocation(this._primaryUILocation.lineNumber, 0);
> 
> Looks like a UILocation's method.

Done.

>> Source/WebCore/inspector/front-end/CallStackSidebarPane.js:150
>> +            this._statusMessageElement.appendChild(status);
> 
> Shouldn't you remove this._statusMessageElement's children?

Good catch. Done.

>> Source/WebCore/inspector/front-end/JavaScriptSource.js:51
>> +            WebInspector.debuggerPresentationModel.breakpointManager.restoreBreakpoints(this);
> 
> Maybe move this call to ScriptsPanel?

I would need to copy it in the tests then.

>> Source/WebCore/inspector/front-end/JavaScriptSource.js:145
>> +                this._togglingFormatter = true;
> 
> Maybe include this info in event?

I'll re-implement this piece as we agreed offline.

>> Source/WebCore/inspector/front-end/RawSourceCode.js:118
>> +        return WebInspector.debuggerModel.createRawLocationByURL(this.url, lineNumber, columnNumber);
> 
> Wouldn't that broke breakpoints in anonymous scripts?

I don't think it does. I'll double check.

>> Source/WebCore/inspector/front-end/ResourceScriptMapping.js:131
>>          var rawSourceCode = /** @type {WebInspector.RawSourceCode} */ event.target;
> 
> Not used.

It is!

[Pavel Feldman]

> 
> I don't think it does. I'll double check.

It does. Fixed.

[Pavel Feldman]

Created attachment 141401 [details]
Patch

[Pavel Podivilov]

looks good!

[Vsevolod Vlasov]

Comment on attachment 141401 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141401&action=review

> Source/WebCore/inspector/front-end/ScriptsPanel.js:520
> +        this._editorContainer.uiSourceCodeAdded(uiLocation.uiSourceCode);

Please keep the old behaviour as discussed offline.

[Pavel Feldman]

Committed r116775: <http://trac.webkit.org/changeset/116775>

[Csaba Osztrogonác]

(In reply to comment #8)
> Committed r116775: <http://trac.webkit.org/changeset/116775>

It made a test fail on Qt:
--- /ramdisk/qt-linux-64-release/build/layout-test-results/inspector/debugger/script-formatter-breakpoints-expected.txt 
+++ /ramdisk/qt-linux-64-release/build/layout-test-results/inspector/debugger/script-formatter-breakpoints-actual.txt 
@@ -14,10 +14,14 @@
 
     script-formatter-breakpoints.html:18
     var f = 0;
+    script-formatter-breakpoints.html:25
+    var sourceFrame;
 Breakpoint sidebar pane while paused in raw
 
     script-formatter-breakpoints.html:11
     var f=0;
+    script-formatter-breakpoints.html:18
+    var sourceFrame;
 Script execution resumed.
 Debugger was disabled.
 

Could you check it, please?

[Csaba Osztrogonác]

New bug report filed on it - https://bugs.webkit.org/show_bug.cgi?id=86337