138515 2014-11-07 13:52:57 -0800 Speed up HTMLInputElement::isEmptyValue() 2014-11-08 17:09:55 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 138538 1 cdumez cdumez kling rniwa oldest_to_newest 1047165 0 cdumez 2014-11-07 13:52:57 -0800 HTMLInputElement::isEmptyValue() currently calls HTMLTextFormControlElement::innerTextValue() which causes a full subtree traversal to construct a string representation of that subtree using a StringBuilder. In the case of HTMLInputElement::isEmptyValue(), we shouldn't have to do all this: We don't need to construct a String and we can return false as soon as we find a non-empty descendant. 1047202 1 241212 cdumez 2014-11-07 15:11:59 -0800 Created attachment 241212 Patch 1047248 2 241212 ggaren 2014-11-07 16:47:08 -0800 Comment on attachment 241212 Patch r=me 1047287 3 241212 cdumez 2014-11-08 10:07:32 -0800 Comment on attachment 241212 Patch Clearing flags on attachment: 241212 Committed r175778: <http://trac.webkit.org/changeset/175778> 1047288 4 cdumez 2014-11-08 10:07:40 -0800 All reviewed patches have been landed. Closing bug. 1047311 5 241212 darin 2014-11-08 13:51:45 -0800 Comment on attachment 241212 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241212&action=review > Source/WebCore/html/HTMLInputElement.cpp:1684 > + if (!isTextField()) > + return true; I suggest putting this code into a virtual InputType::isEmptyValue instead of using the virtual InputType::isTextField to do a boolean check and putting the algorithm here. > Source/WebCore/html/HTMLInputElement.cpp:1691 > + if (text->length()) > + return false; There’s no whitespace collapsing rule we have to consider here? 1047336 6 241212 cdumez 2014-11-08 17:03:49 -0800 Comment on attachment 241212 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241212&action=review >> Source/WebCore/html/HTMLInputElement.cpp:1684 >> + return true; > > I suggest putting this code into a virtual InputType::isEmptyValue instead of using the virtual InputType::isTextField to do a boolean check and putting the algorithm here. Sounds good. >> Source/WebCore/html/HTMLInputElement.cpp:1691 >> + return false; > > There’s no whitespace collapsing rule we have to consider here? I did not change behavior. It was previously calling innerTextValue().isEmpty() which returns true only of the actual empty string. This function is used to determine if the placeholder should be visible or not. I would expect the placeholder to not be visible if the input contains white spaces (and as I said, this was WebKit's behavior already). 241212 2014-11-07 15:11:59 -0800 2014-11-08 10:07:32 -0800 Patch bug-138515-20141107151210.patch text/plain 3867 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTc1NzU5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggN2VlZTAxNDI5MzRiYWI0 ZDk4YzhkM2QyODk3NTkwY2IxYjEzYmU4Yi4uYzhjMWU0YjkxZGQxOTVmYTQyMzY1ODEyZmQ3YWRi YzRkODBjMDRhMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDE0LTExLTA3ICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgU3BlZWQgdXAgSFRNTElucHV0 RWxlbWVudDo6aXNFbXB0eVZhbHVlKCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTEzODUxNQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIEhUTUxJbnB1dEVsZW1lbnQ6OmlzRW1wdHlWYWx1ZSgpIHdhcyBjYWxs aW5nCisgICAgICAgIEhUTUxUZXh0Rm9ybUNvbnRyb2xFbGVtZW50Ojppbm5lclRleHRWYWx1ZSgp IHdoaWNoIGNhdXNlcyBhIGZ1bGwKKyAgICAgICAgc3VidHJlZSB0cmF2ZXJzYWwgdG8gY29uc3Ry dWN0IGEgc3RyaW5nIHJlcHJlc2VudGF0aW9uIG9mIHRoYXQgc3VidHJlZQorICAgICAgICB1c2lu ZyBhIFN0cmluZ0J1aWxkZXIuIEluIHRoZSBjYXNlIG9mIEhUTUxJbnB1dEVsZW1lbnQ6OmlzRW1w dHlWYWx1ZSgpLAorICAgICAgICB3ZSBkb24ndCBoYXZlIHRvIGRvIGFsbCB0aGlzOiBXZSBkb24n dCBuZWVkIHRvIGNvbnN0cnVjdCBhIFN0cmluZworICAgICAgICBhbmQgd2UgY2FuIHJldHVybiBm YWxzZSBhcyBzb29uIGFzIHdlIGZpbmQgYSBub24tZW1wdHkgZGVzY2VuZGFudC4KKworICAgICAg ICBObyBuZXcgdGVzdHMsIG5vIGJlaGF2aW9yIGNoYW5nZS4KKworICAgICAgICAqIGh0bWwvSFRN TElucHV0RWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MSW5wdXRFbGVtZW50Ojpp c0VtcHR5VmFsdWUpOgorICAgICAgICAqIGh0bWwvSFRNTElucHV0RWxlbWVudC5oOgorCiAyMDE0 LTExLTA3ICBKb3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAKICAgICAgICAg V2ViIEluc3BlY3RvcjogUHNldWRvIGVsZW1lbnQgbWF0Y2hlZENTU1J1bGVzIGRvIG5vdCBpbmNs dWRlIG1hdGNoaW5nIHNlbGVjdG9yIGluZm8KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0 bWwvSFRNTElucHV0RWxlbWVudC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxJbnB1dEVs ZW1lbnQuY3BwCmluZGV4IDYzZmEyN2ExMGZiOGFlNGEyZWRhZmNhODcxOGYzYTkwM2MyNTI4MTUu Ljk3ODY0NWZmZWFiMjQ1MTY4MGM4ZjUxYmNiYTczNGNiNzRkNTdiODYgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL2h0bWwvSFRNTElucHV0RWxlbWVudC5jcHAKKysrIGIvU291cmNlL1dlYkNv cmUvaHRtbC9IVE1MSW5wdXRFbGVtZW50LmNwcApAQCAtNjIsNiArNjIsOCBAQAogI2luY2x1ZGUg IlNlYXJjaElucHV0VHlwZS5oIgogI2luY2x1ZGUgIlN0eWxlUmVzb2x2ZXIuaCIKICNpbmNsdWRl ICJUZXh0QnJlYWtJdGVyYXRvci5oIgorI2luY2x1ZGUgIlRleHRDb250cm9sSW5uZXJFbGVtZW50 cy5oIgorI2luY2x1ZGUgIlRleHROb2RlVHJhdmVyc2FsLmgiCiAjaW5jbHVkZSA8d3RmL01hdGhF eHRyYXMuaD4KICNpbmNsdWRlIDx3dGYvUmVmLmg+CiAKQEAgLTE2NzYsNiArMTY3OCwyMSBAQCB2 b2lkIEhUTUxJbnB1dEVsZW1lbnQ6OnVwZGF0ZVBsYWNlaG9sZGVyVGV4dCgpCiAgICAgcmV0dXJu IG1faW5wdXRUeXBlLT51cGRhdGVQbGFjZWhvbGRlclRleHQoKTsKIH0KIAorYm9vbCBIVE1MSW5w dXRFbGVtZW50Ojppc0VtcHR5VmFsdWUoKSBjb25zdAoreworICAgIGlmICghaXNUZXh0RmllbGQo KSkKKyAgICAgICAgcmV0dXJuIHRydWU7CisKKyAgICBUZXh0Q29udHJvbElubmVyVGV4dEVsZW1l bnQqIGlubmVyVGV4dCA9IGlubmVyVGV4dEVsZW1lbnQoKTsKKyAgICBBU1NFUlQoaW5uZXJUZXh0 KTsKKworICAgIGZvciAoVGV4dCogdGV4dCA9IFRleHROb2RlVHJhdmVyc2FsOjpmaXJzdFdpdGhp bihpbm5lclRleHQpOyB0ZXh0OyB0ZXh0ID0gVGV4dE5vZGVUcmF2ZXJzYWw6Om5leHQodGV4dCwg aW5uZXJUZXh0KSkgeworICAgICAgICBpZiAodGV4dC0+bGVuZ3RoKCkpCisgICAgICAgICAgICBy ZXR1cm4gZmFsc2U7CisgICAgfQorICAgIHJldHVybiB0cnVlOworfQorCiB2b2lkIEhUTUxJbnB1 dEVsZW1lbnQ6OnBhcnNlTWF4TGVuZ3RoQXR0cmlidXRlKGNvbnN0IEF0b21pY1N0cmluZyYgdmFs dWUpCiB7CiAgICAgaW50IG1heExlbmd0aDsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0 bWwvSFRNTElucHV0RWxlbWVudC5oIGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MSW5wdXRFbGVt ZW50LmgKaW5kZXggN2I0ZmFhYmU1MjE3YWJkNTIzMGM5OWE3MDYwNTU5NjMzNzFhYjRjNC4uOTZl ZWUzNTdlYzgxMDMzZTI0ZGM5MDE3ZTRiOGRlYTQ2MjQ5ODM5ZCAxMDA2NDQKLS0tIGEvU291cmNl L1dlYkNvcmUvaHRtbC9IVE1MSW5wdXRFbGVtZW50LmgKKysrIGIvU291cmNlL1dlYkNvcmUvaHRt bC9IVE1MSW5wdXRFbGVtZW50LmgKQEAgLTE0NCw3ICsxNDQsNyBAQCBwdWJsaWM6CiAjZW5kaWYK IAogICAgIEhUTUxFbGVtZW50KiBjb250YWluZXJFbGVtZW50KCkgY29uc3Q7Ci0gICAgdmlydHVh bCBUZXh0Q29udHJvbElubmVyVGV4dEVsZW1lbnQqIGlubmVyVGV4dEVsZW1lbnQoKSBjb25zdCBv dmVycmlkZTsKKyAgICB2aXJ0dWFsIFRleHRDb250cm9sSW5uZXJUZXh0RWxlbWVudCogaW5uZXJU ZXh0RWxlbWVudCgpIGNvbnN0IG92ZXJyaWRlIGZpbmFsOwogICAgIEhUTUxFbGVtZW50KiBpbm5l ckJsb2NrRWxlbWVudCgpIGNvbnN0OwogICAgIEhUTUxFbGVtZW50KiBpbm5lclNwaW5CdXR0b25F bGVtZW50KCkgY29uc3Q7CiAgICAgSFRNTEVsZW1lbnQqIHJlc3VsdHNCdXR0b25FbGVtZW50KCkg Y29uc3Q7CkBAIC0zNDEsNyArMzQxLDcgQEAgcHJpdmF0ZToKICAgICB2aXJ0dWFsIHZvaWQgdXBk YXRlRm9jdXNBcHBlYXJhbmNlKGJvb2wgcmVzdG9yZVByZXZpb3VzU2VsZWN0aW9uKSBvdmVycmlk ZTsKICAgICB2aXJ0dWFsIGJvb2wgc2hvdWxkVXNlSW5wdXRNZXRob2QoKSBvdmVycmlkZSBmaW5h bDsKIAotICAgIHZpcnR1YWwgYm9vbCBpc1RleHRGb3JtQ29udHJvbCgpIGNvbnN0IG92ZXJyaWRl IHsgcmV0dXJuIGlzVGV4dEZpZWxkKCk7IH0KKyAgICB2aXJ0dWFsIGJvb2wgaXNUZXh0Rm9ybUNv bnRyb2woKSBjb25zdCBvdmVycmlkZSBmaW5hbCB7IHJldHVybiBpc1RleHRGaWVsZCgpOyB9CiAK ICAgICB2aXJ0dWFsIGJvb2wgY2FuVHJpZ2dlckltcGxpY2l0U3VibWlzc2lvbigpIGNvbnN0IG92 ZXJyaWRlIHsgcmV0dXJuIGlzVGV4dEZpZWxkKCk7IH0KIApAQCAtMzg5LDcgKzM4OSw3IEBAIHBy aXZhdGU6CiAKICAgICB2aXJ0dWFsIGJvb2wgc3VwcG9ydHNQbGFjZWhvbGRlcigpIGNvbnN0IG92 ZXJyaWRlOwogICAgIHZpcnR1YWwgdm9pZCB1cGRhdGVQbGFjZWhvbGRlclRleHQoKSBvdmVycmlk ZTsKLSAgICB2aXJ0dWFsIGJvb2wgaXNFbXB0eVZhbHVlKCkgY29uc3Qgb3ZlcnJpZGUgeyByZXR1 cm4gaW5uZXJUZXh0VmFsdWUoKS5pc0VtcHR5KCk7IH0KKyAgICB2aXJ0dWFsIGJvb2wgaXNFbXB0 eVZhbHVlKCkgY29uc3Qgb3ZlcnJpZGUgZmluYWw7CiAgICAgdmlydHVhbCB2b2lkIGhhbmRsZUZv Y3VzRXZlbnQoTm9kZSogb2xkRm9jdXNlZE5vZGUsIEZvY3VzRGlyZWN0aW9uKSBvdmVycmlkZTsK ICAgICB2aXJ0dWFsIHZvaWQgaGFuZGxlQmx1ckV2ZW50KCkgb3ZlcnJpZGU7CiAK