Implement reportValidity()

Source/core/html/HTMLFormControlElement.cpp

Index: Source/core/html/HTMLFormControlElement.cpp
diff --git a/Source/core/html/HTMLFormControlElement.cpp b/Source/core/html/HTMLFormControlElement.cpp
index bd58b9eb3ba07f2537c0ae34b21a82aa198ce43f..7304cf3117d2487a736086cd437e4697696b9476 100644
--- a/Source/core/html/HTMLFormControlElement.cpp
+++ b/Source/core/html/HTMLFormControlElement.cpp
@@ -486,7 +486,7 @@ ValidationMessageClient* HTMLFormControlElement::validationMessageClient() const
     return &page->validationMessageClient();
 }
 
-bool HTMLFormControlElement::checkValidity(WillBeHeapVector<RefPtrWillBeMember<FormAssociatedElement> >* unhandledInvalidControls, CheckValidityEventBehavior eventBehavior)
+bool HTMLFormControlElement::checkValidity(WillBeHeapVector<RefPtrWillBeMember<HTMLFormControlElement> >* unhandledInvalidControls, CheckValidityEventBehavior eventBehavior)
 {
     if (!willValidate() || isValidElement())
         return true;
@@ -501,6 +501,34 @@ bool HTMLFormControlElement::checkValidity(WillBeHeapVector<RefPtrWillBeMember<F
     return false;
 }
 
+bool HTMLFormControlElement::showValidationMessage()

[tkent, 2014/10/22 01:14:15]

Returning bool looks weird for the name |showValidationMessage|.  If this
function keeps the isFocusable check, the function name should be
|showValidationMessageIfFocusable|.

[Bartek Nowierski, 2014/10/22 04:56:43]

Kicked out isFocusable() call from showValidationMessage() and changed the
return type to void

+{
+    if (isFocusable() && inDocument()) {

[tkent, 2014/10/22 01:14:15]

inDocument() is redundant.  isFocusable() contains it.

Also, we prefer early return.  e.g.

if (!isFocusable())
    return false;
scrollIntoViewIfNeeded(false);
focus();
...

[Bartek Nowierski, 2014/10/22 04:56:43]

Done, on the layout.

This function no longer calls isFocusable(), but I removed inDocument()  in 2
places in HTMLFormElement::validateInteractively() where it was called.

+        scrollIntoViewIfNeeded(false);
+        focus();

[tkent, 2014/10/22 01:14:15]

This function should have |RefPtrWillBeRawPtr<HTMLFormControlElement>
protector(this);| because focus() dispatches an event, and JavaScript code can
delete |this|.

[Bartek Nowierski, 2014/10/22 04:56:43]

Done.

I think the current code already has this bug.
HTMLFormElement::validateInteractively doesn't call a protector on the
form-control element, only on the form.

+        updateVisibleValidationMessage();
+        return true;
+    }
+    return false;
+}
+
+bool HTMLFormControlElement::reportValidity()
+{
+    WillBeHeapVector<RefPtrWillBeMember<HTMLFormControlElement> > unhandledInvalidControls;
+    bool isValid = checkValidity(&unhandledInvalidControls);

[tkent, 2014/10/22 01:14:15]

cehckValidity() dispatches an event, and JavaScript code can delete |this|.  We
need |RefPtrWillBeRawPtr<HTMLFormControlElement> protector(this);| before this
line.

[Bartek Nowierski, 2014/10/22 04:56:43]

Done.

I was relying on checkValidity to have its own protector, but I agree it wasn't
enough.

+    if (!isValid && !unhandledInvalidControls.isEmpty()) {

[tkent, 2014/10/22 01:14:15]

We prefer early return. e.g.

if (isValid || unhandledInvalidControls.isEmpty())
    return isValid;
ASSERT(...)

[Bartek Nowierski, 2014/10/22 04:56:43]

Done.

+        ASSERT(unhandledInvalidControls.size() == 1);
+        ASSERT(unhandledInvalidControls[0].get() == this);
+
+        // Needs to update layout now because we'd like to call isFocusable(),
+        // which has !renderer()->needsLayout() assertion.
+        document().updateLayoutIgnorePendingStylesheets();

[tkent, 2014/10/22 01:14:15]

This line should be moved to showValidationMessage() because it's for
isFocusable().

[Bartek Nowierski, 2014/10/22 04:56:43]

I moved isFocusable out of showValidationMessage()

+        RefPtrWillBeRawPtr<HTMLFormControlElement> protector(this);
+        showValidationMessage();

[tkent, 2014/10/22 01:14:15]

If showValidationMessage() returns false, we should show a console message like
HTMLFormElement::validateInteractively() does.

[Bartek Nowierski, 2014/10/22 04:56:43]

Done.

I surrounded it with document().frame(), just like
HTMLFormElement::validateInteractively() does. Is it ok?

+    }
+    return isValid;
+}
+
 bool HTMLFormControlElement::matchesValidityPseudoClasses() const
 {
     return willValidate();