Summary: | Removing an HTML element spends a lot of time in adjustDirectionalityIfNeededAfterChildrenChanged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||
Component: | DOM | Assignee: | Ryosuke Niwa <rniwa> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | benjamin, cdumez, commit-queue, esprehn+autocc, gyuyoung.kim, kling, koivisto, mitz | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Ryosuke Niwa
2015-01-15 15:52:17 PST
Created attachment 244721 [details]
Fixes the bug
Comment on attachment 244721 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=244721&action=review > Source/WebCore/html/HTMLElement.cpp:950 > if (document().renderView() && (changeType == ElementRemoved || changeType == TextRemoved)) { I think we can just remove this whole if. This code was refactored by Antti in https://bugs.webkit.org/show_bug.cgi?id=120599 and his changeling says he tried to keep the previous behavior of this function. However, if you look at the for loop before the refactoring, it is clear that it was never iterating. if (childCountDelta < 0) { for() } and the for looped only until counter < childCountDelta. counter started at 0. and BTW, I removed this whole "if" block on Blink side a while back already. Created attachment 244725 [details]
Updated per Chris' comments
Comment on attachment 244725 [details]
Updated per Chris' comments
r=me
Comment on attachment 244725 [details] Updated per Chris' comments View in context: https://bugs.webkit.org/attachment.cgi?id=244725&action=review > Source/WebCore/html/HTMLElement.cpp:946 > // FIXME: This function looks suspicious. And this comment is right. I think it is time we take a better look at this whole function and see if it makes sense. Landed in http://trac.webkit.org/changeset/178571. |