Commit 27466f8c authored by tkent@chromium.org's avatar tkent@chromium.org

Simplify form validation handling

This CL contains some part of https://codereview.chromium.org/460343004/ by
sudarshan.p, which was reverted by a performance regression.

This CL simplifies form validation state handling code by
 - lazy update of m_isValid
   setNeedsValidityCheck just makes m_isValid dirty.
 - m_isValid takes care of willValidate state too.
   m_isValid = !willValidate() || valid()

We switched to invalidation sets for :valid and :invalid by Blink
r184392. Calling pseudoStateChanged() for unchanged validity state won't have so
bad performance. So, we can evaluate m_isValid lazily.

We need to change the meaning of m_isValid slightly so that it takes care of
willValidate(). This is necessary to clean m_validityIsDirty of associated
controls in HTMLFormElement::checkInvalidControlsAndCollectUnhandled.
Without this change, there would be a case where willValidate state
change won't invalidate :valid :invalid style of <form>.

This CL will have a few percent regression on blink_perf.dom:textarea-edit.
A fix for the performance regression will be landed soon.

BUG=

Review URL: https://codereview.chromium.org/943303002

git-svn-id: svn://svn.chromium.org/blink/trunk@190723 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent ee922474
......@@ -76,15 +76,6 @@ bool HTMLFieldSetElement::isValidElement()
return true;
}
void HTMLFieldSetElement::setNeedsValidityCheck()
{
// For now unconditionally order style recalculation, which triggers
// validity recalculation. In the near future, consider implement validity
// cache and recalculate style only if it changed.
pseudoStateChanged(CSSSelector::PseudoValid);
pseudoStateChanged(CSSSelector::PseudoInvalid);
}
void HTMLFieldSetElement::invalidateDisabledStateUnder(Element& base)
{
for (HTMLFormControlElement& element : Traversal<HTMLFormControlElement>::descendantsOf(base))
......
......@@ -41,8 +41,6 @@ public:
const FormAssociatedElement::List& associatedElements() const;
void setNeedsValidityCheck();
protected:
virtual void disabledAttributeChanged() override;
......
......@@ -58,6 +58,7 @@ HTMLFormControlElement::HTMLFormControlElement(const QualifiedName& tagName, Doc
, m_willValidateInitialized(false)
, m_willValidate(true)
, m_isValid(true)
, m_validityIsDirty(false)
, m_wasChangedSinceLastFormControlChangeEvent(false)
, m_wasFocusedByMouse(false)
{
......@@ -283,28 +284,31 @@ void HTMLFormControlElement::removedFrom(ContainerNode* insertionPoint)
void HTMLFormControlElement::willChangeForm()
{
FormAssociatedElement::willChangeForm();
formOwnerSetNeedsValidityCheck(ElementRemoval, isValidElement());
formOwnerSetNeedsValidityCheck();
}
void HTMLFormControlElement::didChangeForm()
{
FormAssociatedElement::didChangeForm();
formOwnerSetNeedsValidityCheck(ElementAddition, isValidElement());
formOwnerSetNeedsValidityCheck();
}
void HTMLFormControlElement::formOwnerSetNeedsValidityCheck(ValidityRecalcReason reason, bool isValid)
void HTMLFormControlElement::formOwnerSetNeedsValidityCheck()
{
HTMLFormElement* form = formOwner();
if (form)
form->setNeedsValidityCheck(reason, isValid);
if (HTMLFormElement* form = formOwner()) {
form->pseudoStateChanged(CSSSelector::PseudoValid);
form->pseudoStateChanged(CSSSelector::PseudoInvalid);
}
}
void HTMLFormControlElement::fieldSetAncestorsSetNeedsValidityCheck(Node* node)
{
if (!node)
return;
for (HTMLFieldSetElement* fieldSet = Traversal<HTMLFieldSetElement>::firstAncestorOrSelf(*node); fieldSet; fieldSet = Traversal<HTMLFieldSetElement>::firstAncestor(*fieldSet))
fieldSet->setNeedsValidityCheck();
for (HTMLFieldSetElement* fieldSet = Traversal<HTMLFieldSetElement>::firstAncestorOrSelf(*node); fieldSet; fieldSet = Traversal<HTMLFieldSetElement>::firstAncestor(*fieldSet)) {
fieldSet->pseudoStateChanged(CSSSelector::PseudoValid);
fieldSet->pseudoStateChanged(CSSSelector::PseudoInvalid);
}
}
void HTMLFormControlElement::setChangedSinceLastFormControlChangeEvent(bool changed)
......@@ -508,7 +512,7 @@ ValidationMessageClient* HTMLFormControlElement::validationMessageClient() const
bool HTMLFormControlElement::checkValidity(WillBeHeapVector<RefPtrWillBeMember<HTMLFormControlElement>>* unhandledInvalidControls, CheckValidityEventBehavior eventBehavior)
{
if (!willValidate() || isValidElement())
if (isValidElement())
return true;
if (eventBehavior != CheckValidityDispatchInvalidEvent)
return false;
......@@ -559,21 +563,23 @@ bool HTMLFormControlElement::matchesValidityPseudoClasses() const
bool HTMLFormControlElement::isValidElement()
{
// If the following assertion fails, setNeedsValidityCheck() is not called
// correctly when something which changes validity is updated.
ASSERT(m_isValid == valid());
if (m_validityIsDirty) {
m_isValid = !willValidate() || valid();
m_validityIsDirty = false;
} else {
// If the following assertion fails, setNeedsValidityCheck() is not
// called correctly when something which changes validity is updated.
ASSERT(m_isValid == (!willValidate() || valid()));
}
return m_isValid;
}
void HTMLFormControlElement::setNeedsValidityCheck()
{
bool newIsValid = valid();
bool changed = newIsValid != m_isValid;
m_isValid = newIsValid;
if (changed) {
formOwnerSetNeedsValidityCheck(ElementModification, newIsValid);
if (!m_validityIsDirty) {
m_validityIsDirty = true;
formOwnerSetNeedsValidityCheck();
fieldSetAncestorsSetNeedsValidityCheck(parentNode());
// Update style for pseudo classes such as :valid :invalid.
pseudoStateChanged(CSSSelector::PseudoValid);
pseudoStateChanged(CSSSelector::PseudoInvalid);
}
......
......@@ -34,7 +34,6 @@ class HTMLFormElement;
class ValidationMessageClient;
enum CheckValidityEventBehavior { CheckValidityDispatchNoEvent, CheckValidityDispatchInvalidEvent };
enum ValidityRecalcReason { ElementAddition, ElementRemoval, ElementModification };
// HTMLFormControlElement is the default implementation of FormAssociatedElement,
// and form-associated element implementations should use HTMLFormControlElement
......@@ -169,9 +168,7 @@ private:
ValidationMessageClient* validationMessageClient() const;
// Requests validity recalc for the form owner, if one exists.
// In case of removal, isValid specifies element validity upon removal.
// In case of addition and modification, it specifies new validity.
void formOwnerSetNeedsValidityCheck(ValidityRecalcReason, bool isValid);
void formOwnerSetNeedsValidityCheck();
// Requests validity recalc for all ancestor fieldsets, if exist.
void fieldSetAncestorsSetNeedsValidityCheck(Node*);
......@@ -193,8 +190,8 @@ private:
mutable bool m_willValidate : 1;
// Cache of valid().
// But "candidate for constraint validation" doesn't affect m_isValid.
bool m_isValid : 1;
bool m_validityIsDirty : 1;
bool m_wasChangedSinceLastFormControlChangeEvent : 1;
bool m_wasFocusedByMouse : 1;
......
......@@ -78,7 +78,6 @@ HTMLFormElement::HTMLFormElement(Document& document)
, m_shouldSubmit(false)
, m_isInResetFunction(false)
, m_wasDemoted(false)
, m_invalidControlsCount(0)
, m_pendingAutocompleteEventsQueue(GenericEventQueue::create(this))
{
}
......@@ -714,31 +713,6 @@ HTMLFormControlElement* HTMLFormElement::defaultButton() const
return 0;
}
void HTMLFormElement::setNeedsValidityCheck(ValidityRecalcReason reason, bool isValid)
{
bool formWasInvalid = m_invalidControlsCount > 0;
switch (reason) {
case ElementRemoval:
if (!isValid)
--m_invalidControlsCount;
break;
case ElementAddition:
if (!isValid)
++m_invalidControlsCount;
break;
case ElementModification:
if (isValid)
--m_invalidControlsCount;
else
++m_invalidControlsCount;
break;
}
if (formWasInvalid && !m_invalidControlsCount)
pseudoStateChanged(CSSSelector::PseudoValid);
if (!formWasInvalid && m_invalidControlsCount)
pseudoStateChanged(CSSSelector::PseudoInvalid);
}
bool HTMLFormElement::checkValidity()
{
return !checkInvalidControlsAndCollectUnhandled(0, CheckValidityDispatchInvalidEvent);
......@@ -746,9 +720,6 @@ bool HTMLFormElement::checkValidity()
bool HTMLFormElement::checkInvalidControlsAndCollectUnhandled(WillBeHeapVector<RefPtrWillBeMember<HTMLFormControlElement>>* unhandledInvalidControls, CheckValidityEventBehavior eventBehavior)
{
if (!unhandledInvalidControls && eventBehavior == CheckValidityDispatchNoEvent)
return m_invalidControlsCount;
RefPtrWillBeRawPtr<HTMLFormElement> protector(this);
// Copy associatedElements because event handlers called from
// HTMLFormControlElement::checkValidity() might change associatedElements.
......@@ -761,12 +732,13 @@ bool HTMLFormElement::checkInvalidControlsAndCollectUnhandled(WillBeHeapVector<R
for (unsigned i = 0; i < elements.size(); ++i) {
if (elements[i]->form() == this && elements[i]->isFormControlElement()) {
HTMLFormControlElement* control = toHTMLFormControlElement(elements[i].get());
if (!control->checkValidity(unhandledInvalidControls, eventBehavior) && control->formOwner() == this)
if (!control->checkValidity(unhandledInvalidControls, eventBehavior) && control->formOwner() == this) {
++invalidControlsCount;
if (!unhandledInvalidControls && eventBehavior == CheckValidityDispatchNoEvent)
return true;
}
}
}
if (eventBehavior == CheckValidityDispatchNoEvent)
ASSERT(invalidControlsCount == m_invalidControlsCount);
return invalidControlsCount;
}
......
......@@ -48,8 +48,6 @@ public:
virtual ~HTMLFormElement();
DECLARE_VIRTUAL_TRACE();
void setNeedsValidityCheck(ValidityRecalcReason, bool isValid);
PassRefPtrWillBeRawPtr<HTMLFormControlsCollection> elements();
void getNamedElements(const AtomicString&, WillBeHeapVector<RefPtrWillBeMember<Element>>&);
......@@ -186,10 +184,6 @@ private:
bool m_wasDemoted : 1;
// Number of invalid elements associated to the form that are candidates
// for constraint validation (their willValidate state is true).
int m_invalidControlsCount;
OwnPtrWillBeMember<GenericEventQueue> m_pendingAutocompleteEventsQueue;
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment