Skip to content

Commit

Permalink
Remove lazy evaluation of HTMLTextAreaElement::value.
Browse files Browse the repository at this point in the history
Remove it for cleaner code, and preparation to fix crbug.com/706370.

The lazy evaluation isn't very helpful because children of innerEditor is
typically 0 or one Text node and an optional trailing <br>. In such case,
innerEditorValue() can just return Text::data(), and it's a cheap operation.

BUG=706370

Review-Url: https://codereview.chromium.org/2803483005
Cr-Commit-Position: refs/heads/master@{#461998}
  • Loading branch information
tkent-google authored and Commit bot committed Apr 5, 2017
1 parent f936f42 commit 4397adc
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 18 deletions.
19 changes: 4 additions & 15 deletions third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ HTMLTextAreaElement::HTMLTextAreaElement(Document& document)
m_cols(defaultCols),
m_wrap(SoftWrap),
m_isDirty(false),
m_valueIsUpToDate(true),
m_isPlaceholderVisible(false) {}

HTMLTextAreaElement* HTMLTextAreaElement::create(Document& document) {
Expand Down Expand Up @@ -287,7 +286,7 @@ void HTMLTextAreaElement::subtreeHasChanged() {
#endif
addPlaceholderBreakElementIfNecessary();
setChangedSinceLastFormControlChangeEvent(true);
m_valueIsUpToDate = false;
updateValue();
setNeedsValidityCheck();
setAutofilled(false);
updatePlaceholderVisibility();
Expand Down Expand Up @@ -360,19 +359,14 @@ String HTMLTextAreaElement::sanitizeUserInputValue(const String& proposedValue,
return proposedValue.left(i);
}

void HTMLTextAreaElement::updateValue() const {
if (m_valueIsUpToDate)
return;

void HTMLTextAreaElement::updateValue() {
m_value = innerEditorValue();
const_cast<HTMLTextAreaElement*>(this)->m_valueIsUpToDate = true;
const_cast<HTMLTextAreaElement*>(this)->notifyFormStateChanged();
notifyFormStateChanged();
m_isDirty = true;
const_cast<HTMLTextAreaElement*>(this)->updatePlaceholderVisibility();
updatePlaceholderVisibility();
}

String HTMLTextAreaElement::value() const {
updateValue();
return m_value;
}

Expand Down Expand Up @@ -443,11 +437,6 @@ void HTMLTextAreaElement::setValueCommon(
}
}

void HTMLTextAreaElement::setInnerEditorValue(const String& value) {
TextControlElement::setInnerEditorValue(value);
m_valueIsUpToDate = true;
}

String HTMLTextAreaElement::defaultValue() const {
StringBuilder value;

Expand Down
4 changes: 1 addition & 3 deletions third_party/WebKit/Source/core/html/HTMLTextAreaElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ class CORE_EXPORT HTMLTextAreaElement final : public TextControlElement {

void handleBeforeTextInsertedEvent(BeforeTextInsertedEvent*) const;
static String sanitizeUserInputValue(const String&, unsigned maxLength);
void updateValue() const;
void setInnerEditorValue(const String&) override;
void updateValue();
void setNonDirtyValue(const String&);
void setValueCommon(const String&,
TextFieldEventBehavior,
Expand Down Expand Up @@ -148,7 +147,6 @@ class CORE_EXPORT HTMLTextAreaElement final : public TextControlElement {
WrapMethod m_wrap;
mutable String m_value;
mutable bool m_isDirty;
bool m_valueIsUpToDate;
unsigned m_isPlaceholderVisible : 1;
String m_suggestedValue;
};
Expand Down
14 changes: 14 additions & 0 deletions third_party/WebKit/Source/core/html/TextControlElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,20 @@ String TextControlElement::innerEditorValue() const {
if (!innerEditor || !isTextControl())
return emptyString;

// Typically, innerEditor has 0 or one Text node followed by 0 or one <br>.
if (!innerEditor->hasChildren())
return emptyString;
Node& firstChild = *innerEditor->firstChild();
if (firstChild.isTextNode()) {
Node* secondChild = firstChild.nextSibling();
if (!secondChild)
return toText(firstChild).data();
if (!secondChild->nextSibling() && isHTMLBRElement(*secondChild))
return toText(firstChild).data();
} else if (!firstChild.nextSibling() && isHTMLBRElement(firstChild)) {
return emptyString;
}

StringBuilder result;
for (Node& node : NodeTraversal::inclusiveDescendantsOf(*innerEditor)) {
if (isHTMLBRElement(node)) {
Expand Down

0 comments on commit 4397adc

Please sign in to comment.