Bug 187613 – Web Inspector: Basic blocks highlighting should use line/column locations instead of offsets

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

Basic blocks highlighting should use line/column locations instead of offsets.

BasicBlockAnnotator uses start and end offsets from the backend to calculate execution ranges to highlight. Once CodeMirror returns to auto-detecting line endings (see https://webkit.org/b/186453), these ranges won't correspond to the same ranges in CodeMirror.

This isn't a simple matter of using line/column locations from the AST instead of offsets. This may require sending line/column locations from the backend, in which case compatibility with older backends would be an issue.

[Radar WebKit Bug Importer]

<rdar://problem/42131808>

[Matt Baker]

Created attachment 344955 [details]
Patch

[Matt Baker]

Comment on attachment 344955 [details]
Patch

Found a small issue. Fixing.

[Matt Baker]

Created attachment 344959 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 344959 [details]
Patch

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

r=me if the formatted case continues to behave as we expect (which I suspect it will given

> Source/WebInspectorUI/UserInterface/Controllers/BasicBlockAnnotator.js:62
> +        let lineEndings = [];
> +        let lineEndingLengths = [];
> +        let pattern = /\r\n?|\n/g;
> +        let match = pattern.exec(content);

This reminds me that we have `String.prototype.lineEndings` in FormatterUtilities.js, which only uses "\n" and not "\r\n". Lets double check that things behave as expected in a pretty printed resource.

My test for that is typically:
1. Inspect http://bogojoker.com/shell/
2. Resources > easySlider.min.js
3. Set a breakpoint on Line 56:21 (the first line inside $next.click callback
4. Click the down arrow on the page
5. Step through code (step all around the code include the calling code which is jquery and ensure ranges are correct)

Shouldn't matter for the majority of cases (\r\n and \n) end in \n. It seems we might only have differences in \r only eliminated content.

[Matt Baker]

Created attachment 344986 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 344986 [details]
Patch for landing

Clearing flags on attachment: 344986

Committed r233820: <https://trac.webkit.org/changeset/233820>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.