Bug 55456

Summary: Web Inspector: Extremely slow DOM search in GMail
Product: WebKit Reporter: Alexander Pavlov (apavlov) <apavlov>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, apavlov, bweinstein, commit-queue, eric, joepeck, keishi, loislo, mathias, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Suggested solution
none
[PATCH] Avoid storing refs to normalHTML in nodes not participating in search highlighting
yurys: review+
[PATCH] Comments addressed, please re-review yurys: review+

Description Alexander Pavlov (apavlov) 2011-03-01 04:08:20 PST
1. Open a message in gmail
2. Click inspect element somewhere
3. Search for '.oM' using the search box on top of the Web Inspector
The search takes forever (almost).

Upstreaming Chromium issue http://code.google.com/p/chromium/issues/detail?id=64928
Comment 1 Alexander Pavlov (apavlov) 2011-03-01 05:17:22 PST
Created attachment 84217 [details]
[PATCH] Suggested solution
Comment 2 Alexander Pavlov (apavlov) 2011-03-01 05:58:48 PST
Created attachment 84220 [details]
[PATCH] Avoid storing refs to normalHTML in nodes not participating in search highlighting
Comment 3 Yury Semikhatsky 2011-03-01 06:16:40 PST
Comment on attachment 84220 [details]
[PATCH] Avoid storing refs to normalHTML in nodes not participating in search highlighting

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

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:1451
>              text = text.substring(match.index + 1);

while (match) {
1449            highlightSearchResult(this.listItemElement, offset + match.index, match[0].length);
Could we replace quadratic algorithm here with a linear one?

> Source/WebCore/inspector/front-end/utilities.js:1023
> +function setAtTextOffset(textNodeIterator, relativeOffset)

This should be a method on the iterator.

> Source/WebCore/inspector/front-end/utilities.js:1062
> +    var textNodeIterator = new OrderedNodeSnapshotIterator(document.evaluate(".//text()", element, null, XPathResult.ORDERED_NODE_SNAPSHOT_TYPE, null));

Consider inlining the iterator. Looks like a loop would work equally well here.
Comment 4 Alexander Pavlov (apavlov) 2011-03-01 08:01:08 PST
Created attachment 84231 [details]
[PATCH] Comments addressed, please re-review
Comment 5 Yury Semikhatsky 2011-03-01 08:13:01 PST
Comment on attachment 84231 [details]
[PATCH] Comments addressed, please re-review

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

> Source/WebCore/inspector/front-end/utilities.js:1034
> +    var previousOffset = 0;

This is not used. Please remove.
Comment 6 Alexander Pavlov (apavlov) 2011-03-01 08:52:15 PST
Landed with the unused var fix.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       Source/WebCore/ChangeLog
        M       Source/WebCore/inspector/front-end/ElementsPanel.js
        M       Source/WebCore/inspector/front-end/ElementsTreeOutline.js
        M       Source/WebCore/inspector/front-end/utilities.js
Committed r80003
Comment 7 WebKit Review Bot 2011-03-01 09:42:51 PST
http://trac.webkit.org/changeset/80003 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
loader/reload-subresource-when-type-changes.html