Bug 68526 – Web Inspector: RawSourceCode could be mapped to several UISourceCodes.

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

[Pavel Podivilov]

Web Inspector: RawSourceCode could be mapped to several UISourceCodes.

Replace SourceMapping.uiSourceCode with SourceMapping.uiSourceCodeList as there could be more then one UISourceCode associated with a RawSourceCode.

[Pavel Podivilov]

Created attachment 108143 [details]
Patch.

Based on patch attached to https://bugs.webkit.org/show_bug.cgi?id=68524.

[Yury Semikhatsky]

Comment on attachment 108143 [details]
Patch.

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

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:134
> +            // FIXME: uiSourceCodeList may contain more then one entry.

What prevents you from iterating though all uiSourceCodeList elements in this change?

> Source/WebCore/inspector/front-end/SourceFile.js:205
> +        return [this._uiSourceCode];

Why not keep this list in a field to avoid creating it on each call?

[Pavel Podivilov]

Created attachment 109025 [details]
Patch

[Pavel Podivilov]

(In reply to comment #2)
> (From update of attachment 108143 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108143&action=review
> 
> > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:134
> > +            // FIXME: uiSourceCodeList may contain more then one entry.
> 
> What prevents you from iterating though all uiSourceCodeList elements in this change?

Removed those fixmes, but added one in ScriptsPanel._uiSourceCodeReplaced to keep patch size small.

> 
> > Source/WebCore/inspector/front-end/SourceFile.js:205
> > +        return [this._uiSourceCode];
> 
> Why not keep this list in a field to avoid creating it on each call?

Done.

[Pavel Podivilov]

Created attachment 109027 [details]
Patch

[Pavel Feldman]

Comment on attachment 109027 [details]
Patch

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

> LayoutTests/inspector/debugger/raw-source-code.html:106
>              var rawSourceCode = createRawSourceCode(script, null);

You probably need a new test for this functionality.

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:462
> +        var uiLocation = this._rawSourceCode.sourceMapping.rawLocationToUILocation(this._callFrame.location);

Again, use rawLocationToUILocation. I don't get your FIXME in the RawSourceCode suggesting the opposite. Are you expecting RawSourceCode to lose this._sourceMapping member?

[Pavel Podivilov]

(In reply to comment #6)
> (From update of attachment 109027 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=109027&action=review
> 
> > LayoutTests/inspector/debugger/raw-source-code.html:106
> >              var rawSourceCode = createRawSourceCode(script, null);
> 
> You probably need a new test for this functionality.

There is no new functionality it this change, it's just a refactoring. Let me put all the pieces together and add integration tests for compiler source maps.

> 
> > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:462
> > +        var uiLocation = this._rawSourceCode.sourceMapping.rawLocationToUILocation(this._callFrame.location);
> 
> Again, use rawLocationToUILocation. I don't get your FIXME in the RawSourceCode suggesting the opposite. Are you expecting RawSourceCode to lose this._sourceMapping member?

FIXME in the RawSourceCode suggests that conversion methods should be a part of SourceMapping interface because sometimes RawSourceCode don't have source mapping right after creation (e.g. formatting is in progress).

[Pavel Feldman]

Comment on attachment 109027 [details]
Patch

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

>>> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:462
>>> +        var uiLocation = this._rawSourceCode.sourceMapping.rawLocationToUILocation(this._callFrame.location);
>> 
>> Again, use rawLocationToUILocation. I don't get your FIXME in the RawSourceCode suggesting the opposite. Are you expecting RawSourceCode to lose this._sourceMapping member?
> 
> FIXME in the RawSourceCode suggests that conversion methods should be a part of SourceMapping interface because sometimes RawSourceCode don't have source mapping right after creation (e.g. formatting is in progress).

Please add FIXME suggesting doing rawSourceCode.toUILocation()

[Pavel Podivilov]

Committed r97063: <http://trac.webkit.org/changeset/97063>