Bug 54544 – Web Inspector: show all inlined scripts from single document in the same source frame

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

[Pavel Podivilov]

Web Inspector: show all inlined scripts from single document in the same source frame.

Currently when debugging synchronously executed inlined scripts each script is shown in it's own source frame ("example.html:24").
We should show such scripts in the same source frame "example.html" with <script></script> framing.

[Pavel Podivilov]

Created attachment 82611 [details]
Patch.

[Yury Semikhatsky]

Comment on attachment 82611 [details]
Patch.

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

> Source/WebCore/inspector/front-end/ScriptsPanel.js:264
> +        var sourceName = script.sourceURL || script.sourceID;

You are passing sourceID while _recreateSourceFrame uses it for lookup in _urlToSource... These two types of ids shouldn't be mixed.

> Source/WebCore/inspector/front-end/ScriptsPanel.js:531
> +        var isScript = !script.lineOffset && !script.columnOffset;

Will this code work correctly with JSC when columnOffset is always 0 and lineOffset may well be 0 for inline script.

> Source/WebCore/inspector/front-end/ScriptsPanel.js:1080
> +        var lineNumber = 0, columnNumber = 0;

Each statement should go on a separate line.

[Pavel Podivilov]

Created attachment 82634 [details]
Patch.

[Pavel Podivilov]

(In reply to comment #2)
> (From update of attachment 82611 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=82611&action=review
> 
> > Source/WebCore/inspector/front-end/ScriptsPanel.js:264
> > +        var sourceName = script.sourceURL || script.sourceID;
> 
> You are passing sourceID while _recreateSourceFrame uses it for lookup in _urlToSource... These two types of ids shouldn't be mixed.
> 
sourceID is used as source name for scripts without sourceURL. This is private implementation detail of ScriptsPanel, and it considerably simplifies source frames management.
Renamed _urlToSourceFrame to _sourceNameToSourceFrame to avoid confusion.

> > Source/WebCore/inspector/front-end/ScriptsPanel.js:531
> > +        var isScript = !script.lineOffset && !script.columnOffset;
> 
> Will this code work correctly with JSC when columnOffset is always 0 and lineOffset may well be 0 for inline script.

JSC provides correct columnOffset, as ensured by inspector/debugger/debugger-scripts.html test.

> 
> > Source/WebCore/inspector/front-end/ScriptsPanel.js:1080
> > +        var lineNumber = 0, columnNumber = 0;
> 
> Each statement should go on a separate line.
Could you please clarify? 
The idea was to build source frame content with script tags at the same locations as in actual resource. Please check debug-inlined-scripts test expectations for example.

[Pavel Podivilov]

Created attachment 82635 [details]
Patch.

[Pavel Podivilov]

> Each statement should go on a separate line.
Done.

[Ilya Tikhonovsky]

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

> Source/WebCore/inspector/front-end/ScriptsPanel.js:429
> +        this._sourceNameToFilesSelectOption = {};

Why you create another mapping from sourceName to something instead of a single map with complex objects.

[Pavel Podivilov]

(In reply to comment #7)
> View in context: https://bugs.webkit.org/attachment.cgi?id=82635&action=review
> 
> > Source/WebCore/inspector/front-end/ScriptsPanel.js:429
> > +        this._sourceNameToFilesSelectOption = {};
> 
> Why you create another mapping from sourceName to something instead of a single map with complex objects.

I don't see any benefits in merging these mappings together. They are never modified together, so the code is cleaner when mappings are separated.

[Pavel Podivilov]

Committed r78808: <http://trac.webkit.org/changeset/78808>