Implement :valid and :invalid pseudoclass for <fieldset>

Implement :valid and :invalid pseudoclass for <fieldset>

BUG=424028
TEST=Added layout test: fieldset-pseudo-valid-style.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184047

[Bartek Nowierski, 2014-10-15 10:52:46]

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

[Bartek Nowierski, 2014-10-17 08:48:15]

I resolved this change with https://codereview.chromium.org/658063002/ -- you
can start reviewing now

[tkent, 2014-10-19 23:56:40]

https://codereview.chromium.org/646273004/diff/40001/Source/core/html/HTMLFie...
File Source/core/html/HTMLFieldSetElement.cpp (right):

https://codereview.chromium.org/646273004/diff/40001/Source/core/html/HTMLFie...
Source/core/html/HTMLFieldSetElement.cpp:68:
RefPtrWillBeRawPtr<HTMLFieldSetElement> protector(this);
This variable is not needed.
It would be needed if control->checkValidity() dispatched an event.

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

https://codereview.chromium.org/646273004/diff/40001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.cpp:538:
fieldSetAncestorsSetNeedsValidityCheck(this->parentNode());
|this->| is unnecessary.

I'm afraid this causes a performance regression.
Please confirm PerformanceTest/DOM/textarea-*.html won't regress with this
patch.
You can run them with a command like:
  ./third_party/WebKit/Tools/Scripts/run-perf-tests DOM/textarea-edit.html

[Bartek Nowierski, 2014-10-20 10:43:10]

https://codereview.chromium.org/646273004/diff/40001/Source/core/html/HTMLFie...
File Source/core/html/HTMLFieldSetElement.cpp (right):

https://codereview.chromium.org/646273004/diff/40001/Source/core/html/HTMLFie...
Source/core/html/HTMLFieldSetElement.cpp:68:
RefPtrWillBeRawPtr<HTMLFieldSetElement> protector(this);
On 2014/10/19 23:56:40, tkent wrote:
> This variable is not needed.
> It would be needed if control->checkValidity() dispatched an event.

Done.

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

https://codereview.chromium.org/646273004/diff/40001/Source/core/html/HTMLFor...
Source/core/html/HTMLFormControlElement.cpp:538:
fieldSetAncestorsSetNeedsValidityCheck(this->parentNode());
On 2014/10/19 23:56:40, tkent wrote:
> |this->| is unnecessary.
Done.


> I'm afraid this causes a performance regression.
> Please confirm PerformanceTest/DOM/textarea-*.html won't regress with this
> patch.
> You can run them with a command like:
>   ./third_party/WebKit/Tools/Scripts/run-perf-tests DOM/textarea-edit.html

As discussed on the chat, proceeding as is because this didn't cause a visible
change in perf of DOM/textarea-*.html tests.

[tkent, 2014-10-21 00:11:13]

https://codereview.chromium.org/646273004/diff/80001/Source/core/html/HTMLFie...
File Source/core/html/HTMLFieldSetElement.cpp (right):

https://codereview.chromium.org/646273004/diff/80001/Source/core/html/HTMLFie...
Source/core/html/HTMLFieldSetElement.cpp:70:
ASSERT(elements[i]->isFormControlElement());
We should take care of a case that elements[i] is an HTMLObjectElement.

[Bartek Nowierski, 2014-10-21 03:25:26]

https://codereview.chromium.org/646273004/diff/80001/Source/core/html/HTMLFie...
File Source/core/html/HTMLFieldSetElement.cpp (right):

https://codereview.chromium.org/646273004/diff/80001/Source/core/html/HTMLFie...
Source/core/html/HTMLFieldSetElement.cpp:70:
ASSERT(elements[i]->isFormControlElement());
On 2014/10/21 00:11:13, tkent wrote:
> We should take care of a case that elements[i] is an HTMLObjectElement.

Obviously the assert is incorrect, I don't know what I was thinking. Thanks for
catching!

[tkent, 2014-10-21 03:29:06]

lgtm

[keishi, 2014-10-21 03:43:30]

lgtm

[Bartek Nowierski, 2014-10-21 03:47:57]

The CQ bit was checked by bartekn@chromium.org

[commit-bot: I haz the power, 2014-10-21 03:48:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/646273004/100001

[commit-bot: I haz the power, 2014-10-21 05:05:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-21 05:05:06]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/3...)

[keishi, 2014-10-21 05:18:17]

Patch set 7 LGTM

[Bartek Nowierski, 2014-10-21 05:31:05]

The CQ bit was checked by bartekn@chromium.org

[commit-bot: I haz the power, 2014-10-21 05:32:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/646273004/140001

[commit-bot: I haz the power, 2014-10-21 06:36:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-21 06:36:43]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[tkent, 2014-10-21 06:37:56]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2014-10-21 06:38:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/646273004/140001

[commit-bot: I haz the power, 2014-10-21 06:41:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-21 06:41:06]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[Bartek Nowierski, 2014-10-21 07:14:23]

The CQ bit was checked by bartekn@chromium.org

[commit-bot: I haz the power, 2014-10-21 07:14:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/646273004/140001

[commit-bot: I haz the power, 2014-10-21 07:15:28]

Committed patchset #8 (id:140001) as 184047