Web Inspector: speedup highlight regex API in DefaultTextEditor
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Jan 2013 19:10:03 +0000 (19:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Jan 2013 19:10:03 +0000 (19:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=107238

Patch by Andrey Lushnikov <lushnikov@chromium.org> on 2013-01-23
Reviewed by Pavel Feldman.

Source/WebCore:

Move overlay highlight measurement (highlight regex API) from DefaultTextEditor._paintLine
method to the DefaultTextEditor._paintLines method which allows to relayout dom only
once. This is a significant improvement to the current state of the
art which does relayout on each regex occurence.
In addition, use "left" css attribute instead of "margin-left": this
way it will be possible to avoid an unnecessary relayouting during
appending overlay highlight.

No new tests: no change in behaviour.

* inspector/front-end/DefaultTextEditor.js:
(WebInspector.TextEditorMainPanel.prototype.markAndRevealRange):
(WebInspector.TextEditorMainPanel.prototype._paintLines):
(WebInspector.TextEditorMainPanel.prototype._measureRegexHighlight):
(WebInspector.TextEditorMainPanel.prototype._measureSpans):
(WebInspector.TextEditorMainPanel.prototype._appendOverlayHighlight):
(WebInspector.TextEditorMainPanel.prototype._paintLine):
(WebInspector.TextEditorMainPanel.ElementMetrics):
(WebInspector.TextEditorMainPanel.LineOverlayHighlight):
(WebInspector.TextEditorMainChunk.prototype.expand):

LayoutTests:

Corrected layout test expectations to correspond to refactoring.

* inspector/editor/text-editor-highlight-regexp-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@140556 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/editor/text-editor-highlight-regexp-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/inspector/front-end/DefaultTextEditor.js

index 9c015a9..5393f83 100644 (file)
@@ -1,3 +1,14 @@
+2013-01-23  Andrey Lushnikov  <lushnikov@chromium.org>
+
+        Web Inspector: speedup highlight regex API in DefaultTextEditor
+        https://bugs.webkit.org/show_bug.cgi?id=107238
+
+        Reviewed by Pavel Feldman.
+
+        Corrected layout test expectations to correspond to refactoring.
+
+        * inspector/editor/text-editor-highlight-regexp-expected.txt:
+
 2013-01-23  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [GTK][WK2] Unflag tests related to layoutTestController.setViewModeMediaFeature
index 4690319..f8ba5fd 100644 (file)
@@ -17,11 +17,11 @@ Add highlight for regexp /foo_./
 
 <div class="inner-container" tabindex="0">
 <div class="webkit-line-content">foo();</div>
-<div class="webkit-line-content">foo_1; foor; foo_;<span class="some-css-class text-editor-overlay-highlight" style="margin-left: -127px; width: 37px;">&nbsp;</span><span class="some-css-class text-editor-overlay-highlight" style="margin-left: -36px; width: 37px;">&nbsp;</span></div>
+<div class="webkit-line-content">foo_1; foor; foo_;<span class="some-css-class text-editor-overlay-highlight" style="left: -1px; width: 37px;"> </span><span class="some-css-class text-editor-overlay-highlight" style="left: 90px; width: 37px;"> </span></div>
 <div class="webkit-line-content">foo_</div>
 <div class="webkit-line-content">global_foo</div>
 <div class="webkit-line-content">global_foo2</div>
-<div class="webkit-line-content">some_other_foo_X<span class="some-css-class text-editor-overlay-highlight" style="margin-left: -36px; width: 37px;">&nbsp;</span></div></div>
+<div class="webkit-line-content">some_other_foo_X<span class="some-css-class text-editor-overlay-highlight" style="left: 76px; width: 37px;"> </span></div></div>
 
 Remove highlight for regexp /foo_./
 
index e4ee6f5..6529b85 100644 (file)
@@ -1,3 +1,31 @@
+2013-01-23  Andrey Lushnikov  <lushnikov@chromium.org>
+
+        Web Inspector: speedup highlight regex API in DefaultTextEditor
+        https://bugs.webkit.org/show_bug.cgi?id=107238
+
+        Reviewed by Pavel Feldman.
+
+        Move overlay highlight measurement (highlight regex API) from DefaultTextEditor._paintLine
+        method to the DefaultTextEditor._paintLines method which allows to relayout dom only
+        once. This is a significant improvement to the current state of the
+        art which does relayout on each regex occurence.
+        In addition, use "left" css attribute instead of "margin-left": this
+        way it will be possible to avoid an unnecessary relayouting during
+        appending overlay highlight.
+
+        No new tests: no change in behaviour.
+
+        * inspector/front-end/DefaultTextEditor.js:
+        (WebInspector.TextEditorMainPanel.prototype.markAndRevealRange):
+        (WebInspector.TextEditorMainPanel.prototype._paintLines):
+        (WebInspector.TextEditorMainPanel.prototype._measureRegexHighlight):
+        (WebInspector.TextEditorMainPanel.prototype._measureSpans):
+        (WebInspector.TextEditorMainPanel.prototype._appendOverlayHighlight):
+        (WebInspector.TextEditorMainPanel.prototype._paintLine):
+        (WebInspector.TextEditorMainPanel.ElementMetrics):
+        (WebInspector.TextEditorMainPanel.LineOverlayHighlight):
+        (WebInspector.TextEditorMainChunk.prototype.expand):
+
 2013-01-23  Ojan Vafai  <ojan@chromium.org>
 
         Assert that computePreferredLogicalWidths never calls setNeedsLayout
index 66c1683..6baca8d 100644 (file)
@@ -1566,7 +1566,7 @@ WebInspector.TextEditorMainPanel.prototype = {
             this._rangeToMark = range;
             this.revealLine(range.startLine);
             var chunk = this.makeLineAChunk(range.startLine);
-            this._paintLine(chunk.element);
+            this._paintLines(chunk.startLine, chunk.startLine + 1);
             if (this._markedRangeElement)
                 this._markedRangeElement.scrollIntoViewIfNeeded();
         }
@@ -1793,6 +1793,7 @@ WebInspector.TextEditorMainPanel.prototype = {
     {
         var chunk;
         var selection;
+        var lineRows = [];
         for (var lineNumber = fromLine; lineNumber < toLine; ++lineNumber) {
             if (!chunk || lineNumber < chunk.startLine || lineNumber >= chunk.startLine + chunk.linesCount)
                 chunk = this.chunkForLine(lineNumber);
@@ -1801,9 +1802,21 @@ WebInspector.TextEditorMainPanel.prototype = {
                 continue;
             if (restoreSelection && !selection)
                 selection = this.selection();
-            this._paintLine(lineRow);
+            lineRows.push(lineRow);
         }
 
+        var highlight = {};
+        this.beginDomUpdates();
+        for(var regexString in this._highlightRegexs) {
+            var regexHighlightDescriptor = this._highlightRegexs[regexString];
+            this._measureRegexHighlight(highlight, lineRows, regexHighlightDescriptor.regex, regexHighlightDescriptor.cssClass);
+        }
+
+        for(var i = 0; i < lineRows.length; ++i)
+            this._paintLine(lineRows[i], highlight[lineRows[i].lineNumber]);
+
+        this.endDomUpdates();
+
         if (restoreSelection)
             this._restoreSelection(selection);
     },
@@ -1827,48 +1840,67 @@ WebInspector.TextEditorMainPanel.prototype = {
     },
 
     /**
-     * @param {Element} lineRow
-     * @param {string} line
+     * @param {Object.<number, Array.<WebInspector.TextEditorMainPanel.LineOverlayHighlight>>} highlight
+     * @param {Array.<Element>} lineRows
      * @param {RegExp} regex
-     * @return {Array.<{left: number, width: number}>}
+     * @param {string} cssClass
      */
-    _measureRegex: function(lineRow, line, regex)
+    _measureRegexHighlight: function(highlight, lineRows, regex, cssClass)
     {
-        var ranges = this._findRegexOccurrences(line, regex);
-        if (ranges.length === 0)
-            return [];
+        var rowsToMeasure = [];
+        for(var i = 0; i < lineRows.length; ++i) {
+            var lineRow = lineRows[i];
+            var line = this._textModel.line(lineRow.lineNumber);
+            var ranges = this._findRegexOccurrences(line, regex);
+            if (ranges.length === 0)
+                continue;
 
-        this._renderRanges(lineRow, line, ranges);
-        var spans = lineRow.getElementsByTagName("span");
-        if (WebInspector.debugDefaultTextEditor)
-            console.assert(spans.length === ranges.length, "Ranges number: " + ranges.length + " !== spans number: " + spans.length);
+            this._renderRanges(lineRow, line, ranges);
+            rowsToMeasure.push(lineRow);
+        }
+
+        for(var i = 0; i < rowsToMeasure.length; ++i) {
+            var lineRow = rowsToMeasure[i];
+            var lineNumber = lineRow.lineNumber;
+            var metrics = this._measureSpans(lineRow);
+
+            if (!highlight[lineNumber])
+                highlight[lineNumber] = [];
 
+            highlight[lineNumber].push(new WebInspector.TextEditorMainPanel.LineOverlayHighlight(metrics, cssClass));
+        }
+    },
+
+    /**
+     * @param {Element} lineRow
+     * @return {Array.<WebInspector.TextEditorMainPanel.ElementMetrics>}
+     */
+    _measureSpans: function(lineRow)
+    {
+        var spans = lineRow.getElementsByTagName("span");
         var metrics = [];
-        for(var i = 0; i < ranges.length; ++i)
-            metrics.push({
-                left: spans[i].offsetLeft,
-                width: spans[i].offsetWidth
-            });
+        for(var i = 0; i < spans.length; ++i)
+            metrics.push(new WebInspector.TextEditorMainPanel.ElementMetrics(spans[i]));
         return metrics;
     },
 
     /**
      * @param {Element} lineRow
-     * @param {Array.<{offsetLeft: number, offsetTop: number, offsetWidth: number, offsetHeight: number}>} metrics
-     * @param {string} cssClass
+     * @param {WebInspector.TextEditorMainPanel.LineOverlayHighlight} highlight
      */
-    _appendOverlayHighlight: function(lineRow, metrics, cssClass)
+    _appendOverlayHighlight: function(lineRow, highlight)
     {
         const extraWidth = 1;
+        var metrics = highlight.metrics;
+        var cssClass = highlight.cssClass;
         for(var i = 0; i < metrics.length; ++i) {
-            var highlight = document.createElement("span");
-            highlight.addStyleClass(cssClass);
-            lineRow.appendChild(highlight);
-
-            highlight.style.marginLeft = (metrics[i].left - highlight.offsetLeft - extraWidth) + "px";
-            highlight.style.width = (metrics[i].width + extraWidth * 2) + "px";
-            highlight.innerHTML = "&nbsp;";
-            highlight.addStyleClass("text-editor-overlay-highlight");
+            var highlightSpan = document.createElement("span");
+            highlightSpan.addStyleClass(cssClass);
+            highlightSpan.style.left = (metrics[i].left - extraWidth) + "px";
+            highlightSpan.style.width = (metrics[i].width + extraWidth * 2) + "px";
+            highlightSpan.textContent = " ";
+            highlightSpan.addStyleClass("text-editor-overlay-highlight");
+            lineRow.appendChild(highlightSpan);
         }
     },
 
@@ -1914,32 +1946,25 @@ WebInspector.TextEditorMainPanel.prototype = {
 
     /**
      * @param {Element} lineRow
+     * @param {Array.<WebInspector.TextEditorMainPanel.LineOverlayHighlight>} overlayHighlight
      */
-    _paintLine: function(lineRow)
+    _paintLine: function(lineRow, overlayHighlight)
     {
         var lineNumber = lineRow.lineNumber;
 
         this.beginDomUpdates();
         try {
-            var highlight = this._textModel.getAttribute(lineNumber, "highlight");
-            if (!highlight)
+            var syntaxHighlight = this._textModel.getAttribute(lineNumber, "highlight");
+            if (!syntaxHighlight)
                 return;
 
             var line = this._textModel.line(lineNumber);
-
-            var metrics = [];
-            var cssClasses = [];
-            for(var key in this._highlightRegexs) {
-                var value = this._highlightRegexs[key];
-                metrics.push(this._measureRegex(lineRow, line, value.regex));
-                cssClasses.push(value.cssClass);
-            }
-
-            var ranges = highlight.ranges;
+            var ranges = syntaxHighlight.ranges;
             this._renderRanges(lineRow, line, ranges);
 
-            for(var i = 0; i < metrics.length; ++i)
-                this._appendOverlayHighlight(lineRow, metrics[i], cssClasses[i]);
+            if (overlayHighlight)
+                for(var i = 0; i < overlayHighlight.length; ++i)
+                    this._appendOverlayHighlight(lineRow, overlayHighlight[i]);
         } finally {
             if (this._rangeToMark && this._rangeToMark.startLine === lineNumber)
                 this._markedRangeElement = WebInspector.highlightSearchResult(lineRow, this._rangeToMark.startColumn, this._rangeToMark.endColumn - this._rangeToMark.startColumn);
@@ -2637,6 +2662,27 @@ WebInspector.TextEditorMainPanel.prototype = {
 
 /**
  * @constructor
+ * @param {Element} element
+ */
+WebInspector.TextEditorMainPanel.ElementMetrics = function(element)
+{
+    this.width = element.offsetWidth;
+    this.left = element.offsetLeft;
+}
+
+/**
+ * @constructor
+ * @param {Array.<WebInspector.TextEditorMainPanel.ElementMetrics>} metrics
+ * @param {string} cssClass
+ */
+WebInspector.TextEditorMainPanel.LineOverlayHighlight = function(metrics, cssClass)
+{
+    this.metrics = metrics;
+    this.cssClass = cssClass;
+}
+
+/**
+ * @constructor
  * @param {WebInspector.TextEditorChunkedPanel} chunkedPanel
  * @param {number} startLine
  * @param {number} endLine
@@ -2755,7 +2801,7 @@ WebInspector.TextEditorMainChunk.prototype = {
         this._expanded = true;
 
         if (this.linesCount === 1) {
-            this._chunkedPanel._paintLine(this.element);
+            this._chunkedPanel._paintLines(this.startLine, this.startLine + 1);
             return;
         }