Fix bug where form/fieldset :valid/:invalid won't be recalculated upon control's willValidate change

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

[Bartek Nowierski, 2014-10-23 10:12:48]

bartekn@chromium.org changed reviewers:
+ keishi@chromium.org, tkent@chromium.org

[keishi, 2014-10-24 10:25:49]

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
File Source/core/html/HTMLFormControlElement.cpp (right):

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.cpp:417: // call willValidate() again.
Is this true? m_willValidateInitialized is set to true above and
recalcWillValidate will set m_dataListAncestorState so I don't think this
creates an infinite recursion.

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
File Source/core/html/HTMLFormControlElement.h (right):

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.h:37: enum ValidityChangeAction {
ElementAddition, ElementRemoval, ElementModification };
nit: It might be just me. The name * Action gives me the impression of a pending
task. I would prefer ValidityChangeReason or ValidityChangeType but I don't have
strong feelings.

[Bartek Nowierski, 2014-10-24 10:48:30]

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
File Source/core/html/HTMLFormControlElement.cpp (right):

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.cpp:417: // call willValidate() again.
On 2014/10/24 10:25:49, keishi wrote:
> Is this true? m_willValidateInitialized is set to true above and
> recalcWillValidate will set m_dataListAncestorState so I don't think this
> creates an infinite recursion.

It's a little more complex than that. Calling isValidElement() here will trigger
the m_isValid==valid() assert, because the above two line will change the
validity result.

So what I tried was calling isValidElement two lines above, but that one does
cause infinite recursion as you noted.

One way or the other, the statement that willValidate may be called is true.

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
File Source/core/html/HTMLFormControlElement.h (right):

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.h:37: enum ValidityChangeAction {
ElementAddition, ElementRemoval, ElementModification };
On 2014/10/24 10:25:49, keishi wrote:
> nit: It might be just me. The name * Action gives me the impression of a
pending
> task. I would prefer ValidityChangeReason or ValidityChangeType but I don't
have
> strong feelings.

Reason seems like a much better option. Done.

[keishi, 2014-10-24 11:07:17]

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
File Source/core/html/HTMLFormControlElement.cpp (right):

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.cpp:417: // call willValidate() again.
On 2014/10/24 10:48:30, Bartek Nowierski wrote:
> On 2014/10/24 10:25:49, keishi wrote:
> > Is this true? m_willValidateInitialized is set to true above and
> > recalcWillValidate will set m_dataListAncestorState so I don't think this
> > creates an infinite recursion.
> 
> It's a little more complex than that. Calling isValidElement() here will
trigger
> the m_isValid==valid() assert, because the above two line will change the
> validity result.
> 
> So what I tried was calling isValidElement two lines above, but that one does
> cause infinite recursion as you noted.
> 
> One way or the other, the statement that willValidate may be called is true.

I think using ElementAddition/Removal here when we should be using
ElementModified is bad.

The problem seems to be in HTMLFormControlElement::setNeedsValidityCheck.
formOwnerSetNeedsValidityCheck and fieldSetAncestorsSetNeedsValidityCheck should
be called from setNeedsValidityCheck() but it isn't. 
Inside HTMLFormControlElement::setNeedsValidityCheck,
formOwnerSetNeedsValidityCheck and fieldSetAncestorsSetNeedsValidityCheck should
be moved out of the if statement.

[Bartek Nowierski, 2014-10-24 14:47:16]

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
File Source/core/html/HTMLFormControlElement.cpp (right):

https://codereview.chromium.org/672163002/diff/60001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.cpp:417: // call willValidate() again.
On 2014/10/24 11:07:17, keishi wrote:
> On 2014/10/24 10:48:30, Bartek Nowierski wrote:
> > On 2014/10/24 10:25:49, keishi wrote:
> > > Is this true? m_willValidateInitialized is set to true above and
> > > recalcWillValidate will set m_dataListAncestorState so I don't think this
> > > creates an infinite recursion.
> > 
> > It's a little more complex than that. Calling isValidElement() here will
> trigger
> > the m_isValid==valid() assert, because the above two line will change the
> > validity result.
> > 
> > So what I tried was calling isValidElement two lines above, but that one
does
> > cause infinite recursion as you noted.
> > 
> > One way or the other, the statement that willValidate may be called is true.
> 
> I think using ElementAddition/Removal here when we should be using
> ElementModified is bad.
> 
> The problem seems to be in HTMLFormControlElement::setNeedsValidityCheck.
> formOwnerSetNeedsValidityCheck and fieldSetAncestorsSetNeedsValidityCheck
should
> be called from setNeedsValidityCheck() but it isn't. 
> Inside HTMLFormControlElement::setNeedsValidityCheck,
> formOwnerSetNeedsValidityCheck and fieldSetAncestorsSetNeedsValidityCheck
should
> be moved out of the if statement.

Done.