Bug 56559 - Web Inspector: Scrolling and navigation is not smooth on a script with many long lines
Summary: Web Inspector: Scrolling and navigation is not smooth on a script with many l...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-17 06:49 PDT by Andrey Adaikin
Modified: 2011-03-23 03:30 PDT (History)
11 users (show)

See Also:


Attachments
Patch (8.51 KB, patch)
2011-03-17 07:01 PDT, Andrey Adaikin
no flags Details | Formatted Diff | Diff
Patch (8.59 KB, patch)
2011-03-17 07:52 PDT, Andrey Adaikin
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Adaikin 2011-03-17 06:49:46 PDT
Now we highlight all lines in the visible area synchronously. It takes noticeable time on source files that have many long lines (for example, obfuscated JavaScript code of GMail). Thus, scrolling and navigation via keys is not smooth on such cases. We should paint lines asynchronously.

Tested on a file with 2K lines, 1M of bytes.
Comment 1 Andrey Adaikin 2011-03-17 07:01:16 PDT
Created attachment 86053 [details]
Patch
Comment 2 Andrey Adaikin 2011-03-17 07:52:04 PDT
Created attachment 86055 [details]
Patch
Comment 3 Andrey Adaikin 2011-03-17 07:53:51 PDT
FYI. Diff between the first and the second patches:


--- a/Source/WebCore/inspector/front-end/TextViewer.js
+++ b/Source/WebCore/inspector/front-end/TextViewer.js
@@ -844,9 +844,9 @@ WebInspector.TextEditorMainPanel.prototype = {
         if (!this._scheduledPaintLines)
             return;
 
-        if (this._dirtyLines) {
-            // Reschedule the timer.
-            this._paintScheduledLinesTimer = setTimeout(this._paintScheduledLines.bind(this), 10);
+        // Reschedule the timer if we still can not paint the lines, or the user is scrolling.
+        if (this._dirtyLines || this._repaintAllTimer) {
+            this._paintScheduledLinesTimer = setTimeout(this._paintScheduledLines.bind(this), 50);
             return;
         }
Comment 4 Yury Semikhatsky 2011-03-22 07:27:39 PDT
Comment on attachment 86055 [details]
Patch

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

> Source/WebCore/inspector/front-end/TextViewer.js:838
> +    _paintScheduledLines: function(opt_skipRestoreSelection)

We don't use opt_ prefix in the inspector code.

> Source/WebCore/inspector/front-end/TextViewer.js:899
> +        if (this._dirtyLines || this._scheduledPaintLines || this._paintLinesOperationsCredit < 0) {

You should check for this._paintLinesOperationsCredit < 0 at line 891 after painting next line and if the credit is exceeded schedule painting of all rest lines in the chunk.
Comment 5 Yury Semikhatsky 2011-03-22 07:28:10 PDT
Comment on attachment 86055 [details]
Patch

Please address the comments before landing.
Comment 6 Andrey Adaikin 2011-03-23 02:23:50 PDT
Comment on attachment 86055 [details]
Patch

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

>> Source/WebCore/inspector/front-end/TextViewer.js:838
>> +    _paintScheduledLines: function(opt_skipRestoreSelection)
> 
> We don't use opt_ prefix in the inspector code.

done

>> Source/WebCore/inspector/front-end/TextViewer.js:899
>> +        if (this._dirtyLines || this._scheduledPaintLines || this._paintLinesOperationsCredit < 0) {
> 
> You should check for this._paintLinesOperationsCredit < 0 at line 891 after painting next line and if the credit is exceeded schedule painting of all rest lines in the chunk.

done
Comment 7 Pavel Podivilov 2011-03-23 03:30:31 PDT
Committed r81760: <http://trac.webkit.org/changeset/81760>