Improve conformance of default OPTION selection of SELECT element

Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was unclear.  Now we can run only default OPTION selection explicitly.
  We could run default OPTION selection by recalcListItems().  However it was
  hard to optimize it.
  We apply the new default OPTION selection function to HTMLSelectElement::
  resetImpl().  Default OPTION selection code in resetImpl() and recalcListItems()
  were inconsistent for disabled OPTION elements.

- Remove !isFinisedParsingChildren() hack
  It was necessary because we needed to call recalcListItems(), which is O(N), to
  run default OPTION selection.
  optionInserted() skips default OPTION selection if we already have the selected
  OPTION.  We don't need the hack any longer.

Tests:
This CL changes the beahvior of fast/forms/select/HTMLOptionElement_selected3.html.
It's expected because we run default OPTION selection immediately after adding an
OPTION, not run it lazily in HTMLOptionElement::selected().

BUG=335876
Committed: https://crrev.com/8dcbfa5cdab0982e4ebc485a9c8e70e20cef5026
Cr-Commit-Position: refs/heads/master@{#383480}

[tkent, 2016-03-25 09:39:20]

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

[tkent, 2016-03-25 23:01:25]

Description was changed from

==========
Improve conformance of default OPTION selectionn of SELECT element

BUG=335876
==========

to

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was non-deterministic.  We run default OPTION selection explicitly.


BUG=335876
==========

[tkent, 2016-03-27 23:56:34]

Description was changed from

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was non-deterministic.  We run default OPTION selection explicitly.


BUG=335876
==========

to

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was unclear.  Now we can run only default OPTION selection explicitly.
  We could run default OPTION selection by recalcListItems().  However it was
  hard to optimize it.
  


BUG=335876
==========

[tkent, 2016-03-28 00:07:06]

Description was changed from

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was unclear.  Now we can run only default OPTION selection explicitly.
  We could run default OPTION selection by recalcListItems().  However it was
  hard to optimize it.
  


BUG=335876
==========

to

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was unclear.  Now we can run only default OPTION selection explicitly.
  We could run default OPTION selection by recalcListItems().  However it was
  hard to optimize it.
  We apply the new default OPTION selection function to HTMLSelectElement::
  resetImpl().  Default OPTION selection code in resetImpl() and
recalcListItems()
  were inconsistent for disabled OPTION elements.

- Remove !isFinisedParsingChildren() hack
  It was necessary because we needed to call recalcListitems(), which is O(N),
to
  run default OPTION selection.
  optionInserted() skips default OPTION selection if we already have the
selected
  OPTION.  We don't need the hack any longer.

Tests:
This CL changes the beahvior of
fast/forms/select/HTMLOptionElement_selected3.html/
It's expected because we run default OPTION selection immediately after adding
an
OPTION, not run it lazily in HTMLOptionElement::selected().

BUG=335876
==========

[tkent, 2016-03-28 00:07:56]

Keishi, would you review this please?

[keishi, 2016-03-28 01:55:35]

By removing !isFinisedParsingChildren() hack, won't we call recalcListitems()
for every option when all options are disabled?

https://codereview.chromium.org/1837463002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLSelectElement.h (right):

https://codereview.chromium.org/1837463002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLSelectElement.h:196: void
resetDefaultSelection();
nit: would renaming this to resetToDefaultSelection make it clear that this is
changing the actual selection, not an internally kept selection.

[tkent, 2016-03-28 03:34:18]

On 2016/03/28 at 01:55:35, keishi wrote:
> By removing !isFinisedParsingChildren() hack, won't we call recalcListitems()
for every option when all options are disabled?

We will.  Also, we'll call recalcListItmes() every time on parsing and adding
<option selected>.
So, we'll have O(N^2) if all of OPTION children are disabled or selected.
I don't think we should optimize for such rare cases now.

[tkent, 2016-03-28 03:43:26]

https://codereview.chromium.org/1837463002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLSelectElement.h (right):

https://codereview.chromium.org/1837463002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLSelectElement.h:196: void
resetDefaultSelection();
On 2016/03/28 at 01:55:35, keishi wrote:
> nit: would renaming this to resetToDefaultSelection make it clear that this is
changing the actual selection, not an internally kept selection.

Done.

[keishi, 2016-03-28 04:29:26]

LGTM

[tkent, 2016-03-28 04:42:10]

Description was changed from

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was unclear.  Now we can run only default OPTION selection explicitly.
  We could run default OPTION selection by recalcListItems().  However it was
  hard to optimize it.
  We apply the new default OPTION selection function to HTMLSelectElement::
  resetImpl().  Default OPTION selection code in resetImpl() and
recalcListItems()
  were inconsistent for disabled OPTION elements.

- Remove !isFinisedParsingChildren() hack
  It was necessary because we needed to call recalcListitems(), which is O(N),
to
  run default OPTION selection.
  optionInserted() skips default OPTION selection if we already have the
selected
  OPTION.  We don't need the hack any longer.

Tests:
This CL changes the beahvior of
fast/forms/select/HTMLOptionElement_selected3.html/
It's expected because we run default OPTION selection immediately after adding
an
OPTION, not run it lazily in HTMLOptionElement::selected().

BUG=335876
==========

to

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was unclear.  Now we can run only default OPTION selection explicitly.
  We could run default OPTION selection by recalcListItems().  However it was
  hard to optimize it.
  We apply the new default OPTION selection function to HTMLSelectElement::
  resetImpl().  Default OPTION selection code in resetImpl() and
recalcListItems()
  were inconsistent for disabled OPTION elements.

- Remove !isFinisedParsingChildren() hack
  It was necessary because we needed to call recalcListItems(), which is O(N),
to
  run default OPTION selection.
  optionInserted() skips default OPTION selection if we already have the
selected
  OPTION.  We don't need the hack any longer.

Tests:
This CL changes the beahvior of
fast/forms/select/HTMLOptionElement_selected3.html.
It's expected because we run default OPTION selection immediately after adding
an
OPTION, not run it lazily in HTMLOptionElement::selected().

BUG=335876
==========

[tkent, 2016-03-28 04:43:10]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2016-03-28 04:43:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1837463002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1837463002/20001

[commit-bot: I haz the power, 2016-03-28 05:03:33]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-28 05:05:10]

Description was changed from

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was unclear.  Now we can run only default OPTION selection explicitly.
  We could run default OPTION selection by recalcListItems().  However it was
  hard to optimize it.
  We apply the new default OPTION selection function to HTMLSelectElement::
  resetImpl().  Default OPTION selection code in resetImpl() and
recalcListItems()
  were inconsistent for disabled OPTION elements.

- Remove !isFinisedParsingChildren() hack
  It was necessary because we needed to call recalcListItems(), which is O(N),
to
  run default OPTION selection.
  optionInserted() skips default OPTION selection if we already have the
selected
  OPTION.  We don't need the hack any longer.

Tests:
This CL changes the beahvior of
fast/forms/select/HTMLOptionElement_selected3.html.
It's expected because we run default OPTION selection immediately after adding
an
OPTION, not run it lazily in HTMLOptionElement::selected().

BUG=335876
==========

to

==========
Improve conformance of default OPTION selection of SELECT element

This CL changes the following behavior:

* When an OPTION element is removed from the owner SELECT element, default
OPTION
selection happens immediately

* Do not select disabled OPTIONs in default OPTION selection

New behavior matches to Firefox, and conforms to the HTML standard.

Implementation:
- Separate default OPTION selection code from recalcListItems().
  recalcListItems() is called lazily.  So the timing of default OPTION selection
  was unclear.  Now we can run only default OPTION selection explicitly.
  We could run default OPTION selection by recalcListItems().  However it was
  hard to optimize it.
  We apply the new default OPTION selection function to HTMLSelectElement::
  resetImpl().  Default OPTION selection code in resetImpl() and
recalcListItems()
  were inconsistent for disabled OPTION elements.

- Remove !isFinisedParsingChildren() hack
  It was necessary because we needed to call recalcListItems(), which is O(N),
to
  run default OPTION selection.
  optionInserted() skips default OPTION selection if we already have the
selected
  OPTION.  We don't need the hack any longer.

Tests:
This CL changes the beahvior of
fast/forms/select/HTMLOptionElement_selected3.html.
It's expected because we run default OPTION selection immediately after adding
an
OPTION, not run it lazily in HTMLOptionElement::selected().

BUG=335876

Committed: https://crrev.com/8dcbfa5cdab0982e4ebc485a9c8e70e20cef5026
Cr-Commit-Position: refs/heads/master@{#383480}
==========

[commit-bot: I haz the power, 2016-03-28 05:05:11]

Patchset 2 (id:??) landed as
https://crrev.com/8dcbfa5cdab0982e4ebc485a9c8e70e20cef5026
Cr-Commit-Position: refs/heads/master@{#383480}