Bug 138515 - Speed up HTMLInputElement::isEmptyValue()
Summary: Speed up HTMLInputElement::isEmptyValue()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 138538
  Show dependency treegraph
 
Reported: 2014-11-07 13:52 PST by Chris Dumez
Modified: 2014-11-08 17:09 PST (History)
2 users (show)

See Also:


Attachments
Patch (3.78 KB, patch)
2014-11-07 15:11 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2014-11-07 13:52:57 PST
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.
Comment 1 Chris Dumez 2014-11-07 15:11:59 PST
Created attachment 241212 [details]
Patch
Comment 2 Geoffrey Garen 2014-11-07 16:47:08 PST
Comment on attachment 241212 [details]
Patch

r=me
Comment 3 Chris Dumez 2014-11-08 10:07:32 PST
Comment on attachment 241212 [details]
Patch

Clearing flags on attachment: 241212

Committed r175778: <http://trac.webkit.org/changeset/175778>
Comment 4 Chris Dumez 2014-11-08 10:07:40 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2014-11-08 13:51:45 PST
Comment on attachment 241212 [details]
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?
Comment 6 Chris Dumez 2014-11-08 17:03:49 PST
Comment on attachment 241212 [details]
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).