Fix a problem where a control's willValidate change wouldn't trigger the...

Fix a problem where a control's willValidate change wouldn't trigger the containing form's or fieldset's style change. Cache form validity while at it to avoid style recalculation

BUG=360466
TEST=Added cases to layout tests that toggle controls disabled and readonly in form and fieldset

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

git-svn-id: svn://svn.chromium.org/blink/trunk@185347 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/LayoutTests/fast/css/invalidation/fieldset-disabled-expected.txt

@@ -4,7 +4,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 PASS getComputedStyle(fieldset, '').backgroundColor is transparent
-PASS internals.updateStyleAndReturnAffectedElementCount() is 7
+PASS internals.updateStyleAndReturnAffectedElementCount() is 2
 PASS getComputedStyle(fieldset, '').backgroundColor is green
 PASS successfullyParsed is true
 

third_party/WebKit/LayoutTests/fast/css/invalidation/fieldset-disabled.html

@@ -35,7 +35,7 @@ fieldset.disabled = true;
 if (window.internals) {
     // There are still instances of SubtreeStyleChange left when updating
     // disabled state. This count should become lower.
-    shouldBe("internals.updateStyleAndReturnAffectedElementCount()", "7");
+    shouldBe("internals.updateStyleAndReturnAffectedElementCount()", "2");
 }
 
 shouldBe("getComputedStyle(fieldset, '').backgroundColor", "green");

third_party/WebKit/LayoutTests/fast/forms/fieldset-pseudo-valid-style-expected.txt

@@ -3,7 +3,7 @@ Check if :valid/:invalid CSS pseudo selectors are lively applied for fieldsets
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-Removing and adding required text inputs and modifying ther value by a DOM tree mutation:
+Removing and adding required text inputs and modifying their value by a DOM tree mutation:
 PASS backgroundOf(fieldset1) is invalidColor
 PASS backgroundOf(sub1) is subInvalidColor
 PASS fieldset1.removeChild(input1); backgroundOf(fieldset1) is validColor
@@ -15,6 +15,16 @@ PASS backgroundOf(sub1) is subValidColor
 PASS input2.setAttribute("value", ""); backgroundOf(fieldset1) is invalidColor
 PASS backgroundOf(sub1) is subInvalidColor
 
+Disabling and marking inputs readonly by a DOM tree mutation:
+PASS backgroundOf(fieldset1) is invalidColor
+PASS backgroundOf(sub1) is subInvalidColor
+PASS input1.disabled=1; backgroundOf(fieldset1) is validColor
+PASS backgroundOf(sub1) is subValidColor
+PASS input1.disabled=0; backgroundOf(fieldset1) is invalidColor
+PASS backgroundOf(sub1) is subInvalidColor
+PASS input1.setAttribute("readonly", "1"); backgroundOf(fieldset1) is validColor
+PASS backgroundOf(sub1) is subValidColor
+
 Adding a required text input that is not a direct child of the fieldset:
 PASS backgroundOf(fieldset1) is validColor
 PASS div1.appendChild(input1); backgroundOf(fieldset1) is invalidColor

third_party/WebKit/LayoutTests/fast/forms/fieldset-pseudo-valid-style.html

@@ -29,7 +29,7 @@ var subValidColor = 'rgb(0, 127, 0)';
 var parent = document.createElement('div');
 document.body.appendChild(parent);
 
-debug('Removing and adding required text inputs and modifying ther value by a DOM tree mutation:');
+debug('Removing and adding required text inputs and modifying their value by a DOM tree mutation:');
 parent.innerHTML = '<fieldset id=fieldset1><input type=text id=input1 required><input type=text id=input2 required value=a><input type=submit id=sub1></fieldset>';
 var fieldset1 = $('fieldset1');
 var input1 = $('input1');
@@ -47,6 +47,21 @@ shouldBe('input2.setAttribute("value", ""); backgroundOf(fieldset1)', 'invalidCo
 shouldBe('backgroundOf(sub1)', 'subInvalidColor');
 debug('')
 
+debug('Disabling and marking inputs readonly by a DOM tree mutation:');
+parent.innerHTML = '<fieldset id=fieldset1><input type=text id=input1 required><input type=text id=input2 required value=a><input type=submit id=sub1></fieldset>';
+var fieldset1 = $('fieldset1');
+var input1 = $('input1');
+var sub1 = $('sub1');
+shouldBe('backgroundOf(fieldset1)', 'invalidColor');
+shouldBe('backgroundOf(sub1)', 'subInvalidColor');
+shouldBe('input1.disabled=1; backgroundOf(fieldset1)', 'validColor');
+shouldBe('backgroundOf(sub1)', 'subValidColor');
+shouldBe('input1.disabled=0; backgroundOf(fieldset1)', 'invalidColor');
+shouldBe('backgroundOf(sub1)', 'subInvalidColor');
+shouldBe('input1.setAttribute("readonly", "1"); backgroundOf(fieldset1)', 'validColor');
+shouldBe('backgroundOf(sub1)', 'subValidColor');
+debug('')
+
 debug('Adding a required text input that is not a direct child of the fieldset:');
 parent.innerHTML = '<fieldset id=fieldset1></fieldset>';
 var fieldset1 = $('fieldset1');

third_party/WebKit/LayoutTests/fast/forms/form-pseudo-valid-style-expected.txt

@@ -3,7 +3,7 @@ Check if :valid/:invalid CSS pseudo selectors are lively applied for forms
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-Removing and adding required text inputs and modifying ther value by a DOM tree mutation:
+Removing and adding required text inputs and modifying their value by a DOM tree mutation:
 PASS backgroundOf(form1) is invalidColor
 PASS backgroundOf(sub1) is subInvalidColor
 PASS form1.removeChild(input1); backgroundOf(form1) is validColor
@@ -15,6 +15,22 @@ PASS backgroundOf(sub1) is subValidColor
 PASS input2.setAttribute("value", ""); backgroundOf(form1) is invalidColor
 PASS backgroundOf(sub1) is subInvalidColor
 
+Disabling and marking inputs readonly by a DOM tree mutation:
+PASS backgroundOf(form1) is invalidColor
+PASS backgroundOf(sub1) is subInvalidColor
+PASS input1.disabled=1; backgroundOf(form1) is validColor
+PASS backgroundOf(sub1) is subValidColor
+PASS input1.disabled=0; backgroundOf(form1) is invalidColor
+PASS backgroundOf(sub1) is subInvalidColor
+PASS input1.setAttribute("readonly", "1"); backgroundOf(form1) is validColor
+PASS backgroundOf(sub1) is subValidColor
+
+Move element under datalist by a DOM tree mutation:
+PASS backgroundOf(form1) is invalidColor
+PASS parent.removeChild(input1); backgroundOf(form1) is validColor
+PASS dl1.appendChild(input1); backgroundOf(form1) is validColor
+PASS parent.appendChild(input1); backgroundOf(form1) is invalidColor
+
 Adding a required text input that is not a direct child of the form:
 PASS backgroundOf(form1) is validColor
 PASS div1.appendChild(input1); backgroundOf(form1) is invalidColor

third_party/WebKit/LayoutTests/fast/forms/form-pseudo-valid-style.html

@@ -29,7 +29,7 @@ var subValidColor = 'rgb(0, 127, 0)';
 var parent = document.createElement('div');
 document.body.appendChild(parent);
 
-debug('Removing and adding required text inputs and modifying ther value by a DOM tree mutation:');
+debug('Removing and adding required text inputs and modifying their value by a DOM tree mutation:');
 parent.innerHTML = '<form id=form1><input type=text id=input1 required><input type=text id=input2 required value=a><input type=submit id=sub1></form>';
 var form1 = $('form1');
 var input1 = $('input1');
@@ -47,6 +47,32 @@ shouldBe('input2.setAttribute("value", ""); backgroundOf(form1)', 'invalidColor'
 shouldBe('backgroundOf(sub1)', 'subInvalidColor');
 debug('')
 
+debug('Disabling and marking inputs readonly by a DOM tree mutation:');
+parent.innerHTML = '<form id=form1><input type=text id=input1 required><input type=text id=input2 required value=a><input type=submit id=sub1></form>';
+var form1 = $('form1');
+var input1 = $('input1');
+var sub1 = $('sub1');
+shouldBe('backgroundOf(form1)', 'invalidColor');
+shouldBe('backgroundOf(sub1)', 'subInvalidColor');
+shouldBe('input1.disabled=1; backgroundOf(form1)', 'validColor');
+shouldBe('backgroundOf(sub1)', 'subValidColor');
+shouldBe('input1.disabled=0; backgroundOf(form1)', 'invalidColor');
+shouldBe('backgroundOf(sub1)', 'subInvalidColor');
+shouldBe('input1.setAttribute("readonly", "1"); backgroundOf(form1)', 'validColor');
+shouldBe('backgroundOf(sub1)', 'subValidColor');
+debug('')
+
+debug('Move element under datalist by a DOM tree mutation:');
+parent.innerHTML = '<form id=form1></form><datalist id=dl1></datalist><input type=text id=input1 required form=form1>';
+var form1 = $('form1');
+var input1 = $('input1');
+var dl1 = $('dl1');
+shouldBe('backgroundOf(form1)', 'invalidColor');
+shouldBe('parent.removeChild(input1); backgroundOf(form1)', 'validColor');
+shouldBe('dl1.appendChild(input1); backgroundOf(form1)', 'validColor');
+shouldBe('parent.appendChild(input1); backgroundOf(form1)', 'invalidColor');
+debug('')
+
 debug('Adding a required text input that is not a direct child of the form:');
 parent.innerHTML = '<form id=form1></form>';
 var form1 = $('form1');

third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp

@@ -263,27 +263,28 @@ void HTMLFormControlElement::removedFrom(ContainerNode* insertionPoint)
     m_hasValidationMessage = false;
     m_ancestorDisabledState = AncestorDisabledStateUnknown;
     m_dataListAncestorState = Unknown;
+    setNeedsWillValidateCheck();
     HTMLElement::removedFrom(insertionPoint);
     FormAssociatedElement::removedFrom(insertionPoint);
 }
 
 void HTMLFormControlElement::willChangeForm()
 {
-    formOwnerSetNeedsValidityCheck();
     FormAssociatedElement::willChangeForm();
+    formOwnerSetNeedsValidityCheck(ElementRemoval, isValidElement());
 }
 
 void HTMLFormControlElement::didChangeForm()
 {
-    formOwnerSetNeedsValidityCheck();
     FormAssociatedElement::didChangeForm();
+    formOwnerSetNeedsValidityCheck(ElementAddition, isValidElement());
 }
 
-void HTMLFormControlElement::formOwnerSetNeedsValidityCheck()
+void HTMLFormControlElement::formOwnerSetNeedsValidityCheck(ValidityRecalcReason reason, bool isValid)
 {
     HTMLFormElement* form = formOwner();
     if (form)
-        form->setNeedsValidityCheck();
+        form->setNeedsValidityCheck(reason, isValid);
 }
 
 void HTMLFormControlElement::fieldSetAncestorsSetNeedsValidityCheck(Node* node)
@@ -406,12 +407,7 @@ bool HTMLFormControlElement::recalcWillValidate() const
 bool HTMLFormControlElement::willValidate() const
 {
     if (!m_willValidateInitialized || m_dataListAncestorState == Unknown) {
-        m_willValidateInitialized = true;
-        bool newWillValidate = recalcWillValidate();
-        if (m_willValidate != newWillValidate) {
-            m_willValidate = newWillValidate;
-            const_cast<HTMLFormControlElement*>(this)->setNeedsValidityCheck();
-        }
+        const_cast<HTMLFormControlElement*>(this)->setNeedsWillValidateCheck();
     } else {
         // If the following assertion fails, setNeedsWillValidateCheck() is not
         // called correctly when something which changes recalcWillValidate() result
@@ -430,7 +426,10 @@ void HTMLFormControlElement::setNeedsWillValidateCheck()
     m_willValidateInitialized = true;
     m_willValidate = newWillValidate;
     setNeedsValidityCheck();
-    setNeedsStyleRecalc(SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::Validate));
+    // No need to trigger style recalculation here because
+    // setNeedsValidityCheck() does it in the right away. This relies on
+    // the assumption that valid() is always true if willValidate() is false.
+
     if (!m_willValidate)
         hideVisibleValidationMessage();
 }
@@ -557,14 +556,15 @@ bool HTMLFormControlElement::isValidElement()
 void HTMLFormControlElement::setNeedsValidityCheck()
 {
     bool newIsValid = valid();
-    if (willValidate() && newIsValid != m_isValid) {
-        formOwnerSetNeedsValidityCheck();
+    bool changed = newIsValid != m_isValid;
+    m_isValid = newIsValid;
+    if (changed) {
+        formOwnerSetNeedsValidityCheck(ElementModification, newIsValid);
         fieldSetAncestorsSetNeedsValidityCheck(parentNode());
         // Update style for pseudo classes such as :valid :invalid.
         pseudoStateChanged(CSSSelector::PseudoValid);
         pseudoStateChanged(CSSSelector::PseudoInvalid);
     }
-    m_isValid = newIsValid;
 
     // Updates only if this control already has a validation message.
     if (isValidationMessageVisible()) {

third_party/WebKit/Source/core/html/HTMLFormControlElement.h

@@ -34,6 +34,7 @@ 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
@@ -168,7 +169,9 @@ private:
     ValidationMessageClient* validationMessageClient() const;
 
     // Requests validity recalc for the form owner, if one exists.
-    void formOwnerSetNeedsValidityCheck();
+    // 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);
     // Requests validity recalc for all ancestor fieldsets, if exist.
     void fieldSetAncestorsSetNeedsValidityCheck(Node*);
 

third_party/WebKit/Source/core/html/HTMLFormElement.cpp

@@ -78,6 +78,7 @@ HTMLFormElement::HTMLFormElement(Document& document)
     , m_shouldSubmit(false)
     , m_isInResetFunction(false)
     , m_wasDemoted(false)
+    , m_invalidControlsCount(0)
     , m_pendingAutocompleteEventsQueue(GenericEventQueue::create(this))
 {
 }
@@ -715,13 +716,29 @@ HTMLFormControlElement* HTMLFormElement::defaultButton() const
     return 0;
 }
 
-void HTMLFormElement::setNeedsValidityCheck()
-{
-    // For now unconditionally order style recalculation, which triggers
-    // validity recalculation. In the near future, implement validity cache and
-    // recalculate style only if it changed.
-    pseudoStateChanged(CSSSelector::PseudoValid);
-    pseudoStateChanged(CSSSelector::PseudoInvalid);
+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()
@@ -731,6 +748,9 @@ 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.
@@ -739,15 +759,17 @@ bool HTMLFormElement::checkInvalidControlsAndCollectUnhandled(WillBeHeapVector<R
     elements.reserveCapacity(associatedElements.size());
     for (unsigned i = 0; i < associatedElements.size(); ++i)
         elements.append(associatedElements[i]);
-    bool hasInvalidControls = false;
+    int invalidControlsCount = 0;
     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)
-                hasInvalidControls = true;
+                ++invalidControlsCount;
         }
     }
-    return hasInvalidControls;
+    if (eventBehavior == CheckValidityDispatchNoEvent)
+        ASSERT(invalidControlsCount == m_invalidControlsCount);
+    return invalidControlsCount;
 }
 
 bool HTMLFormElement::reportValidity()

third_party/WebKit/Source/core/html/HTMLFormElement.h

@@ -48,7 +48,7 @@ public:
     virtual ~HTMLFormElement();
     virtual void trace(Visitor*) override;
 
-    void setNeedsValidityCheck();
+    void setNeedsValidityCheck(ValidityRecalcReason, bool isValid);
 
     PassRefPtrWillBeRawPtr<HTMLFormControlsCollection> elements();
     void getNamedElements(const AtomicString&, WillBeHeapVector<RefPtrWillBeMember<Element>>&);
@@ -186,6 +186,10 @@ 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;
 };