Bug 34816 – Web Inspector: split source code into chunks in order to improve text viewer performance on large files.

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

[Pavel Feldman]

This change brings back SourceFrame's canvas+style magic, restores line wrapping and makes things a lot like they used to be.
It removes TextEditor for now and renames NativeTextViewer into TextViewer. TextViewer is no longer derived from TextEditor.
This TextViewer is still based on TextEditorModel, no iframes are involved.

Instead of creating div per line, TextViewer splits source code into 50 line chunks. Upon scroll event, visible chunks are sharded into lines and individual lines are highlighted. Whenever highlighted region gets outside of the visible area, highlight spans are thrown away and region is replaced with the original plain text chunk.

Complex stuff:
- Whenever there is a need to manipulate individual lines (add message bubble / set breakpoint / reveal / etc.), individual chunks for such lines are created.
- There is also an implicit machinery that is maintaining selection when it goes beyond the visible area.
- Search occurrences are implemented as artificial spans interweaving highlighting markup. We can now show all and highlight current (not yet implemented).

Outstanding problems / TODOs:
- Selection does not grow upwards due to range normalization
- Highlight animation is missing (sticky yellow is used)
- Repaint logic is very simple. Every time it restores origianal plain text chunks first and then splits visible ones apart. It should be optimized in a way that existing markup / line rows are reused.

[Pavel Feldman]

Created attachment 48523 [details]
[PATCH] Proposed change.

[WebKit Review Bot]

Attachment 48523 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Skipping input 'WebCore/inspector/front-end/textEditor.css': Can't open for reading
WebCore/inspector/front-end/textViewer.css:79:  Line contains tab character.  [whitespace/tab] [5]
WebCore/inspector/front-end/textViewer.css:80:  Line contains tab character.  [whitespace/tab] [5]
WebCore/inspector/front-end/textViewer.css:124:  Line contains tab character.  [whitespace/tab] [5]
WebCore/inspector/front-end/textViewer.css:125:  Line contains tab character.  [whitespace/tab] [5]
WebCore/inspector/front-end/textViewer.css:133:  Line contains tab character.  [whitespace/tab] [5]
Skipping input 'WebCore/inspector/front-end/TextEditor.js': Can't open for reading
Skipping input 'WebCore/inspector/front-end/NativeTextViewer.js': Can't open for reading
Total errors found: 5


If any of these errors are false positives, please file a bug against check-webkit-style.

[Pavel Feldman]

Created attachment 48525 [details]
[PATCH] Proposed change (same with style fixes).

[Timothy Hatcher]

Comment on attachment 48525 [details]
[PATCH] Proposed change (same with style fixes).

> +    setCoalescingUpdate: function(enabled)

I think two functions would be better. Like beginUpdates and endUpdates.

> +            if (offset >= currentOffset && offset < currentOffset + rowHeight) {
> +                return row.chunkNumber;
> +            }

No braces.


> +        for (var i = 0; i < chunkNumber && i < this._textChunks.length; ++i) {
> +            lineNumber += this._textChunks[i].linesCount;
> +        }

No braces.

> +        var firstVisibleChunk = this._chunkForOffset(Math.max(scrollTop - 10, 0));

What is the 10 magic number? Make it a named constant.


> +        for (var i = digits; i < totalDigits; ++i) {
> +            number += " ";
> +        }

Braces.


> +Node.prototype.traverseNextTextNode = function(stayWithin, ignoreClass)

ignoreClass isn't used.


I am a little concerned about the \u200B\n code. It will cause the coppied text to contain unicode an that can make some editors complain.

[Timothy Hatcher]

Comment on attachment 48525 [details]
[PATCH] Proposed change (same with style fixes).

With the style tweaks we talked about on irc for the line gutter and the zero-width space removal.

[Pavel Feldman]

(In reply to comment #4)
> (From update of attachment 48525 [details])
> > +    setCoalescingUpdate: function(enabled)
> 
> I think two functions would be better. Like beginUpdates and endUpdates.
> 

Done.

> No braces.
> 

Done.

> 
> No braces.

Done.

> 
> 
> What is the 10 magic number? Make it a named constant.

Done.
> 
> 
> Braces.
> 

Done.

> 
> > +Node.prototype.traverseNextTextNode = function(stayWithin, ignoreClass)
> 
> ignoreClass isn't used.

Done.

> 
> I am a little concerned about the \u200B\n code. It will cause the coppied text
> to contain unicode an that can make some editors complain.

I removed \u200B.

(In reply to comment #5)
> (From update of attachment 48525 [details])
> With the style tweaks we talked about on irc for the line gutter and the
> zero-width space removal.

Removed the space, added gutter gap.

[Pavel Feldman]

Variation of patch landed. (I extracted a TextChunk class for better clarity and optimized repaint).

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	D	WebCore/inspector/front-end/NativeTextViewer.js
	D	WebCore/inspector/front-end/TextEditor.js
	D	WebCore/inspector/front-end/textEditor.css
	M	WebCore/ChangeLog
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/inspector/front-end/ScriptView.js
	M	WebCore/inspector/front-end/Settings.js
	M	WebCore/inspector/front-end/SourceFrame.js
	M	WebCore/inspector/front-end/SourceView.js
	A	WebCore/inspector/front-end/TextViewer.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/inspector/front-end/inspector.html
	A	WebCore/inspector/front-end/textViewer.css
	M	WebCore/inspector/front-end/utilities.js
Committed r54657