115116 2013-04-24 10:06:31 -0700 CSS attribute selectors cause unnecessary style recalc when setting attribute to same value. 2013-04-24 10:42:21 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar, Performance P2 Normal --- 1 kling kling cmarcelo commit-queue esprehn+autocc kling koivisto webkit-bug-importer oldest_to_newest 880683 0 kling 2013-04-24 10:06:31 -0700 Setting an attribute to the same value it already has should never trigger a style recalc due to attribute selectors. The code is currently too dumb to figure this out, and ends up restyling the same stuff over and over. 880684 1 webkit-bug-importer 2013-04-24 10:07:27 -0700 <rdar://problem/13727709> 880692 2 199498 kling 2013-04-24 10:26:13 -0700 Created attachment 199498 Proposed patch 880698 3 199498 simon.fraser 2013-04-24 10:38:28 -0700 Comment on attachment 199498 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=199498&action=review > Source/WebCore/ChangeLog:13 > + This reduces unnecessary style recalculation in MAS content. "MAS" is pretty meaningless in this context. 880700 4 199498 koivisto 2013-04-24 10:41:33 -0700 Comment on attachment 199498 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=199498&action=review Better fixes involve some significant refactoring, this is ok for now. > Source/WebCore/dom/Element.cpp:-878 > else if (name == HTMLNames::pseudoAttr) > shouldInvalidateStyle |= testShouldInvalidateStyle && isInShadowTree(); > > - shouldInvalidateStyle |= testShouldInvalidateStyle && styleResolver->hasSelectorForAttribute(name.localName()); It seem wrong that attributeChanged() is called when attribute does not change. It would be better to fix that. > Source/WebCore/dom/Element.cpp:2726 > + if (oldValue != newValue) { > + if (attached() && document()->styleResolver() && document()->styleResolver()->hasSelectorForAttribute(name.localName())) > + setNeedsStyleRecalc(); > + } Bit sad that the invalidation code gets spread around like this. 880701 5 kling 2013-04-24 10:42:21 -0700 Committed r149047: <http://trac.webkit.org/changeset/149047> 199498 2013-04-24 10:26:13 -0700 2013-04-24 10:41:33 -0700 Proposed patch bug-115116.diff text/plain 2258 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAzN2U2OTUzLi4wZDI1MDk1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEg QEAKKzIwMTMtMDQtMjQgIEFuZHJlYXMgS2xpbmcgIDxha2xpbmdAYXBwbGUuY29tPgorCisgICAg ICAgIENTUyBhdHRyaWJ1dGUgc2VsZWN0b3JzIGNhdXNlIHVubmVjZXNzYXJ5IHN0eWxlIHJlY2Fs YyB3aGVuIHNldHRpbmcgYXR0cmlidXRlIHRvIHNhbWUgdmFsdWUuCisgICAgICAgIDxodHRwOi8v d2Via2l0Lm9yZy9iLzExNTExNj4KKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzEzNzI3NzA5Pgor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBsb2dp YyB0aGF0IGRpcnRpZXMgdGhlIHN0eWxlIGlmIHRoZXJlJ3MgYSByZWxldmFudCBhdHRyaWJ1dGUg c2VsZWN0b3IgaW4gdGhlIGRvY3VtZW50CisgICAgICAgIHNob3VsZG4ndCBydW4gaWYgdGhlIGF0 dHJpYnV0ZSBpcyBiZWluZyBvdmVyd3JpdHRlbiB3aXRoIGFuIGlkZW50aWNhbCB2YWx1ZS4KKyAg ICAgICAgTW92ZSB0aGlzIGludG8gd2lsbE1vZGlmeUF0dHJpYnV0ZSgpIGluc3RlYWQsIHNpbmNl IHdlIG5lZWQgYWNjZXNzIHRvIGJvdGggdGhlIG9sZCBhbmQgdGhlIG5ldyB2YWx1ZS4KKworICAg ICAgICBUaGlzIHJlZHVjZXMgdW5uZWNlc3Nhcnkgc3R5bGUgcmVjYWxjdWxhdGlvbiBpbiBNQVMg Y29udGVudC4KKworICAgICAgICAqIGRvbS9FbGVtZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkVsZW1lbnQ6OmF0dHJpYnV0ZUNoYW5nZWQpOgorICAgICAgICAoV2ViQ29yZTo6RWxlbWVudDo6 d2lsbE1vZGlmeUF0dHJpYnV0ZSk6CisKIDIwMTMtMDQtMjQgIEFsYmVydG8gR2FyY2lhICA8YWdh cmNpYUBpZ2FsaWEuY29tPgogCiAgICAgICAgIERPTUZpbGVTeXN0ZW1CYXNlOiBmaXggbXVsdGlw bGUgZGVmaW5pdGlvbnMgaW4gdGhlIEJsYWNrQmVycnkgcG9ydApkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvZG9tL0VsZW1lbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvZG9tL0VsZW1lbnQuY3Bw CmluZGV4IDYyYjZmNjUuLjhiZWNlNzAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2RvbS9F bGVtZW50LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9kb20vRWxlbWVudC5jcHAKQEAgLTg3NSw3 ICs4NzUsNiBAQCB2b2lkIEVsZW1lbnQ6OmF0dHJpYnV0ZUNoYW5nZWQoY29uc3QgUXVhbGlmaWVk TmFtZSYgbmFtZSwgY29uc3QgQXRvbWljU3RyaW5nJiBuZQogICAgIGVsc2UgaWYgKG5hbWUgPT0g SFRNTE5hbWVzOjpwc2V1ZG9BdHRyKQogICAgICAgICBzaG91bGRJbnZhbGlkYXRlU3R5bGUgfD0g dGVzdFNob3VsZEludmFsaWRhdGVTdHlsZSAmJiBpc0luU2hhZG93VHJlZSgpOwogCi0gICAgc2hv dWxkSW52YWxpZGF0ZVN0eWxlIHw9IHRlc3RTaG91bGRJbnZhbGlkYXRlU3R5bGUgJiYgc3R5bGVS ZXNvbHZlci0+aGFzU2VsZWN0b3JGb3JBdHRyaWJ1dGUobmFtZS5sb2NhbE5hbWUoKSk7CiAKICAg ICBpbnZhbGlkYXRlTm9kZUxpc3RDYWNoZXNJbkFuY2VzdG9ycygmbmFtZSwgdGhpcyk7CiAKQEAg LTI3MjEsNiArMjcyMCwxMSBAQCB2b2lkIEVsZW1lbnQ6OndpbGxNb2RpZnlBdHRyaWJ1dGUoY29u c3QgUXVhbGlmaWVkTmFtZSYgbmFtZSwgY29uc3QgQXRvbWljU3RyaW5nJgogICAgICAgICAgICAg dXBkYXRlTGFiZWwoc2NvcGUsIG9sZFZhbHVlLCBuZXdWYWx1ZSk7CiAgICAgfQogCisgICAgaWYg KG9sZFZhbHVlICE9IG5ld1ZhbHVlKSB7CisgICAgICAgIGlmIChhdHRhY2hlZCgpICYmIGRvY3Vt ZW50KCktPnN0eWxlUmVzb2x2ZXIoKSAmJiBkb2N1bWVudCgpLT5zdHlsZVJlc29sdmVyKCktPmhh c1NlbGVjdG9yRm9yQXR0cmlidXRlKG5hbWUubG9jYWxOYW1lKCkpKQorICAgICAgICAgICAgc2V0 TmVlZHNTdHlsZVJlY2FsYygpOworICAgIH0KKwogICAgIGlmIChPd25QdHI8TXV0YXRpb25PYnNl cnZlckludGVyZXN0R3JvdXA+IHJlY2lwaWVudHMgPSBNdXRhdGlvbk9ic2VydmVySW50ZXJlc3RH cm91cDo6Y3JlYXRlRm9yQXR0cmlidXRlc011dGF0aW9uKHRoaXMsIG5hbWUpKQogICAgICAgICBy ZWNpcGllbnRzLT5lbnF1ZXVlTXV0YXRpb25SZWNvcmQoTXV0YXRpb25SZWNvcmQ6OmNyZWF0ZUF0 dHJpYnV0ZXModGhpcywgbmFtZSwgb2xkVmFsdWUpKTsKIAo=