140523 2015-01-15 15:52:17 -0800 Removing an HTML element spends a lot of time in adjustDirectionalityIfNeededAfterChildrenChanged 2015-01-15 19:45:21 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 rniwa rniwa benjamin cdumez commit-queue esprehn+autocc gyuyoung.kim kling koivisto mitz oldest_to_newest 1061674 0 rniwa 2015-01-15 15:52:17 -0800 We're spending a lot of time inside adjustDirectionalityIfNeededAfterChildrenChanged while removing a HTML element. 1061678 1 244721 rniwa 2015-01-15 16:00:14 -0800 Created attachment 244721 Fixes the bug 1061681 2 rniwa 2015-01-15 16:06:21 -0800 <rdar://problem/19464329> 1061689 3 244721 cdumez 2015-01-15 16:29:55 -0800 Comment on attachment 244721 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. 1061690 4 cdumez 2015-01-15 16:31:01 -0800 and BTW, I removed this whole "if" block on Blink side a while back already. 1061695 5 244725 rniwa 2015-01-15 16:42:11 -0800 Created attachment 244725 Updated per Chris' comments 1061700 6 244725 cdumez 2015-01-15 16:48:47 -0800 Comment on attachment 244725 Updated per Chris' comments r=me 1061702 7 244725 cdumez 2015-01-15 16:50:13 -0800 Comment on attachment 244725 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. 1061751 8 rniwa 2015-01-15 19:45:21 -0800 Landed in http://trac.webkit.org/changeset/178571. 244721 2015-01-15 16:00:14 -0800 2015-01-15 16:42:06 -0800 Fixes the bug bug-140523-20150115160021.patch text/plain 2083 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3ODUzNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDE1LTAxLTE1ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIFJlbW92aW5nIGFuIEhUTUwgZWxl bWVudCBzcGVuZHMgYSBsb3Qgb2YgdGltZSBpbiBhZGp1c3REaXJlY3Rpb25hbGl0eUlmTmVlZGVk QWZ0ZXJDaGlsZHJlbkNoYW5nZWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTE0MDUyMworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIFRoZSBidWcgd2FzIGNhdXNlZCBieSBhZGp1c3REaXJlY3Rpb25hbGl0eUlm TmVlZGVkQWZ0ZXJDaGlsZHJlbkNoYW5nZWQgYWx3YXlzIHRyYXZlcnNpbmcgY2hpbGRyZW4gdG8K KyAgICAgICAgdW5zZXQgc2VsZk9yQW5jZXN0b3JIYXNEaXJBdXRvQXR0cmlidXRlIGZsYWcgd2hp bGUgcmVtb3ZpbmcgYSBjaGlsZCBlbGVtZW50LiBGaXhlZCB0aGUgYnVnIGJ5CisgICAgICAgIGV4 aXRpbmcgZWFybHkgd2hlbiB0aGlzIGZsYWcgaXMgbm90IHNldCBvbiB0aGUgcGFyZW50IGVsZW1l bnQgYXMgdGhlIGZsYWcgd291bGQgbm90IGhhdmUgYmVlbiBzZXQgb24KKyAgICAgICAgY2hpbGRy ZW4gaW4gdGhhdCBjYXNlLgorCisgICAgICAgICogaHRtbC9IVE1MRWxlbWVudC5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpIVE1MRWxlbWVudDo6YWRqdXN0RGlyZWN0aW9uYWxpdHlJZk5lZWRlZEFm dGVyQ2hpbGRyZW5DaGFuZ2VkKToKKwogMjAxNS0wMS0xNSAgWmFsYW4gQnVqdGFzICA8emFsYW5A YXBwbGUuY29tPgogCiAgICAgICAgIExheWVyRnJhZ21lbnQgc2hvdWxkIGJlIGFibGUgdG8gaW50 ZXJzZWN0IHdpdGggQ2xpcFJlY3QuCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxFbGVt ZW50LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxFbGVtZW50LmNw cAkocmV2aXNpb24gMTc4NTI3KQorKysgU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MRWxlbWVudC5j cHAJKHdvcmtpbmcgY29weSkKQEAgLTk0Myw2ICs5NDMsOSBAQCB2b2lkIEhUTUxFbGVtZW50Ojpj YWxjdWxhdGVBbmRBZGp1c3REaXJlCiAKIHZvaWQgSFRNTEVsZW1lbnQ6OmFkanVzdERpcmVjdGlv bmFsaXR5SWZOZWVkZWRBZnRlckNoaWxkcmVuQ2hhbmdlZChFbGVtZW50KiBiZWZvcmVDaGFuZ2Us IENoaWxkQ2hhbmdlVHlwZSBjaGFuZ2VUeXBlKQogeworICAgIGlmICghc2VsZk9yQW5jZXN0b3JI YXNEaXJBdXRvQXR0cmlidXRlKCkpCisgICAgICAgIHJldHVybjsKKwogICAgIC8vIEZJWE1FOiBU aGlzIGZ1bmN0aW9uIGxvb2tzIHN1c3BpY2lvdXMuCiAgICAgaWYgKGRvY3VtZW50KCkucmVuZGVy VmlldygpICYmIChjaGFuZ2VUeXBlID09IEVsZW1lbnRSZW1vdmVkIHx8IGNoYW5nZVR5cGUgPT0g VGV4dFJlbW92ZWQpKSB7CiAgICAgICAgIE5vZGUqIG5vZGUgPSBiZWZvcmVDaGFuZ2UgPyBiZWZv cmVDaGFuZ2UtPm5leHRTaWJsaW5nKCkgOiBudWxscHRyOwpAQCAtOTU0LDkgKzk1Nyw2IEBAIHZv aWQgSFRNTEVsZW1lbnQ6OmFkanVzdERpcmVjdGlvbmFsaXR5SWYKICAgICAgICAgfQogICAgIH0K IAotICAgIGlmICghc2VsZk9yQW5jZXN0b3JIYXNEaXJBdXRvQXR0cmlidXRlKCkpCi0gICAgICAg IHJldHVybjsKLQogICAgIE5vZGUqIG9sZE1hcmtlZE5vZGUgPSBudWxscHRyOwogICAgIGlmIChi ZWZvcmVDaGFuZ2UpCiAgICAgICAgIG9sZE1hcmtlZE5vZGUgPSBjaGFuZ2VUeXBlID09IEVsZW1l bnRJbnNlcnRlZCA/IEVsZW1lbnRUcmF2ZXJzYWw6Om5leHRTaWJsaW5nKGJlZm9yZUNoYW5nZSkg OiBiZWZvcmVDaGFuZ2UtPm5leHRTaWJsaW5nKCk7Cg== 244725 2015-01-15 16:42:11 -0800 2015-01-15 16:48:47 -0800 Updated per Chris' comments bug-140523-20150115164217.patch text/plain 2149 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3ODUzNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBACisyMDE1LTAxLTE1ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIFJlbW92aW5nIGFuIEhUTUwgZWxl bWVudCBzcGVuZHMgYSBsb3Qgb2YgdGltZSBpbiBhZGp1c3REaXJlY3Rpb25hbGl0eUlmTmVlZGVk QWZ0ZXJDaGlsZHJlbkNoYW5nZWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTE0MDUyMworICAgICAgICA8cmRhcjovL3Byb2JsZW0vMTk0NjQzMjk+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlIGJ1ZyB3 YXMgY2F1c2VkIGJ5IGFkanVzdERpcmVjdGlvbmFsaXR5SWZOZWVkZWRBZnRlckNoaWxkcmVuQ2hh bmdlZCBhbHdheXMgdHJhdmVyc2luZyBjaGlsZHJlbiB0bworICAgICAgICB1bnNldCBzZWxmT3JB bmNlc3Rvckhhc0RpckF1dG9BdHRyaWJ1dGUgZmxhZyB3aGlsZSByZW1vdmluZyBhIGNoaWxkIGVs ZW1lbnQuCisKKyAgICAgICAgRml4ZWQgdGhlIGJ1ZyBieSByZW1vdmluZyB0aGlzIGNvZGUuIFRo aXMgY29kZSB3YXMgbm8tb3AgcHJpb3IgdG8gYmVpbmcgcmVmYWN0b3JlZCBpbiByMTU0OTU3IHNp bmNlCisgICAgICAgIHdlIG9ubHkgZW50ZXJlZCBhIGZvciBsb29wIHdpdGggdGhlIGludmFyaWFu dCAiY291bnRlciA8IGNoaWxkQ291bnREZWx0YSIgd2hlbiAiY2hpbGRDb3VudERlbHRhIDwgMCIu CisKKyAgICAgICAgU2VlIGh0dHA6Ly90cmFjLndlYmtpdC5vcmcvY2hhbmdlc2V0LzE1NDk1Ny90 cnVuay9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxFbGVtZW50LmNwcC4KKworICAgICAgICAqIGh0 bWwvSFRNTEVsZW1lbnQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SFRNTEVsZW1lbnQ6OmFkanVz dERpcmVjdGlvbmFsaXR5SWZOZWVkZWRBZnRlckNoaWxkcmVuQ2hhbmdlZCk6CisKIDIwMTUtMDEt MTUgIFphbGFuIEJ1anRhcyAgPHphbGFuQGFwcGxlLmNvbT4KIAogICAgICAgICBMYXllckZyYWdt ZW50IHNob3VsZCBiZSBhYmxlIHRvIGludGVyc2VjdCB3aXRoIENsaXBSZWN0LgpJbmRleDogU291 cmNlL1dlYkNvcmUvaHRtbC9IVE1MRWxlbWVudC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dl YkNvcmUvaHRtbC9IVE1MRWxlbWVudC5jcHAJKHJldmlzaW9uIDE3ODUyNykKKysrIFNvdXJjZS9X ZWJDb3JlL2h0bWwvSFRNTEVsZW1lbnQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC05NDQsMTUgKzk0 NCw2IEBAIHZvaWQgSFRNTEVsZW1lbnQ6OmNhbGN1bGF0ZUFuZEFkanVzdERpcmUKIHZvaWQgSFRN TEVsZW1lbnQ6OmFkanVzdERpcmVjdGlvbmFsaXR5SWZOZWVkZWRBZnRlckNoaWxkcmVuQ2hhbmdl ZChFbGVtZW50KiBiZWZvcmVDaGFuZ2UsIENoaWxkQ2hhbmdlVHlwZSBjaGFuZ2VUeXBlKQogewog ICAgIC8vIEZJWE1FOiBUaGlzIGZ1bmN0aW9uIGxvb2tzIHN1c3BpY2lvdXMuCi0gICAgaWYgKGRv Y3VtZW50KCkucmVuZGVyVmlldygpICYmIChjaGFuZ2VUeXBlID09IEVsZW1lbnRSZW1vdmVkIHx8 IGNoYW5nZVR5cGUgPT0gVGV4dFJlbW92ZWQpKSB7Ci0gICAgICAgIE5vZGUqIG5vZGUgPSBiZWZv cmVDaGFuZ2UgPyBiZWZvcmVDaGFuZ2UtPm5leHRTaWJsaW5nKCkgOiBudWxscHRyOwotICAgICAg ICBmb3IgKDsgbm9kZTsgbm9kZSA9IG5vZGUtPm5leHRTaWJsaW5nKCkpIHsKLSAgICAgICAgICAg IGlmIChlbGVtZW50QWZmZWN0c0RpcmVjdGlvbmFsaXR5KCpub2RlKSkKLSAgICAgICAgICAgICAg ICBjb250aW51ZTsKLQotICAgICAgICAgICAgc2V0SGFzRGlyQXV0b0ZsYWdSZWN1cnNpdmVseShu b2RlLCBmYWxzZSk7Ci0gICAgICAgIH0KLSAgICB9CiAKICAgICBpZiAoIXNlbGZPckFuY2VzdG9y SGFzRGlyQXV0b0F0dHJpYnV0ZSgpKQogICAgICAgICByZXR1cm47Cg==