Bug 34391 – Web Inspector: Introduce DivBasedTextViewer.

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

[Pavel Feldman]

I am trying out a hybrid approach in source rendering. I thought it would be slow, but given that we do no real editing yet, it seems to work just fine. It will anyway be dead slow on debugging gmail scripts (where we create XXK line divs), but other than that it should be Ok.

Some highlights:
- It inherits from TextEditor and conforms to its API
- It re-uses TextEditor's code (and canvas) to render decorations such as program counter, breakpoints and messages
- It uses same TextEditorModel and highlighter
- It uses same code for line number rendering

The difference is that DivBasedTextViewer creates a div-per line and instantly adds all of them to the editor's sheet. This creates a non-highlighted text that has proper scrollers, browser-based selection, etc. We maintain original scroller handlers and hence when user scrolls to a dirty region, paintLines is being called. Instead of painting lines on the canvas, this viewer replaces them with their highlighted versions using same highlighter logic. As a result, when user opens a source view, he gets a nice highlighted text (highlight is only created for visible area). Then if he scrolls to the middle of the large file, highlighter creates markup attributes on the text model in the background (same control flow as canvas-based). When highlighting data for the visible area is ready, it gets 'repainted', i.e. replaced with highlighted spans.

Seems to be much more efficient than what we've had before and allows us to evolve both versions (canvas and span-based) in parallel.

[Pavel Feldman]

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

[Pavel Feldman]

Created attachment 47790 [details]
[PATCH] Same with less typos.

[Timothy Hatcher]

This approch is exactly what I was thinking.

[Timothy Hatcher]

Comment on attachment 47790 [details]
[PATCH] Same with less typos.

> +WebInspector.DivBasedTextViewer = function(textModel, platform)
> +{
> +    WebInspector.TextEditor.call(this, textModel, platform);

I'm not fond of "DivBasedTextViewer" as a class name. What about "NativeTextViewer", "SimpleTextViewer" or "ClassicTextViewer"? Or, not great but, "DOMBasedTextViewer".

> +    useCanvasBasedEditor: true

Should be false?

[Pavel Feldman]

(In reply to comment #4)
> (From update of attachment 47790 [details])
> > +WebInspector.DivBasedTextViewer = function(textModel, platform)
> > +{
> > +    WebInspector.TextEditor.call(this, textModel, platform);
> 
> I'm not fond of "DivBasedTextViewer" as a class name. What about
> "NativeTextViewer", "SimpleTextViewer" or "ClassicTextViewer"? Or, not great
> but, "DOMBasedTextViewer".
> 

Lets try NativeTextViewer.

> > +    useCanvasBasedEditor: true
> 
> Should be false?

Not until I fix todos mentioned in the changelog. But that should not take long.

[Timothy Hatcher]

(In reply to comment #0)
> I am trying out a hybrid approach in source rendering. I thought it would be
> slow, but given that we do no real editing yet, it seems to work just fine. It
> will anyway be dead slow on debugging gmail scripts (where we create XXK line
> divs), but other than that it should be Ok.

Couldn't we also avoid appending all the line divs and insert spacers until those lines need painted?

I justt things like select all would not work then.

That reminds me, I think you need to add the "-webkit-user-select: text" style to the top level element for the viewer.

[Timothy Hatcher]

"I guess things like select all would not work then."

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	A	WebCore/inspector/front-end/NativeTextViewer.js
	M	WebCore/inspector/front-end/Settings.js
	M	WebCore/inspector/front-end/SourceFrame.js
	M	WebCore/inspector/front-end/TextEditor.js
	M	WebCore/inspector/front-end/TextEditorHighlighter.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/inspector/front-end/inspector.html
	M	WebCore/inspector/front-end/textEditor.css
Committed r54110