SELECT element: Avoid to use listItems() in HTMLSelectElement::selectOption()

SELECT element: Avoid to use listItems() in HTMLSelectElement::selectOption()

Remove listItems() usage from:
 - HTMLOptionElement::index()
 - HTMLSelectElement::saveListboxActiveSelection()
 - HTMLSelectElement::selectedOption()
 - HTMLSelectElement::deselectItemsWithoutValidation()
We use optionList() instead.

These changes don't improve computational complexity. However we can avoid to
update m_listItems when we add/remove OPTIONs.

Other changes:
 - HTMLSelectElement::updateListBoxSelectioN() needs to be updated to synchronize
  with saveListboxActiveSelection().  m_cachedStateForActiveSelection now
  represents OPTIONs instead of listItems, which includes OPTGROUP and HR.
 - Change the arguments of OptionList and OptionListIterator to |const| so that
  we can create them in const member functions of HTMLSelectElement.
 - Remove the optimized fast path in HTMLSelectElement::setRecalcListItems(). We
  don't use m_listItems in critical paths any longer.  So the optimization isn't
  necessary.

This CL improves performance because of the removal of the fast path.
blink_perf.dom:
select-multiple-add: 1,772 runs/s ->  1,912 runs/s
select-single-add: 867.9 runs/s -> 944.7 runs/s
select-single-remove: 137.9 runs/s -> 145.5 runs/.s

BUG=577989
Committed: https://crrev.com/3ebd491419a62d22093a68e7ae3820157a32ecb5
Cr-Commit-Position: refs/heads/master@{#404591}

[tkent, 2016-07-10 23:45:43]

Description was changed from

==========
SELECT element: Avoid to use listItems() in HTMLSelectElement::selectOption()

BUG=
==========

to

==========
SELECT element: Avoid to use listItems() in HTMLSelectElement::selectOption()

Remove listItems() usage from:
 - HTMLOptionElement::index()
 - HTMLSelectElement::saveListboxActiveSelection()
 - HTMLSelectElement::selectedOption()
 - HTMLSelectElement::deselectItemsWithoutValidation()
We use optionList() instead.

These changes don't improve computational complexity. However we can avoid to
update m_listItems when we add/remove OPTIONs.

Other changes:
 - HTMLSelectElement::updateListBoxSelectioN() needs to be updated to
synchronize
  with saveListboxActiveSelection().  m_cachedStateForActiveSelection now
  represents OPTIONs instead of listItems, which includes OPTGROUP and HR.
 - Change the arguments of OptionList and OptionListIterator to |const| so that
  we can create them in const member functions of HTMLSelectElement.
 - Remove the optimized fast path in HTMLSelectElement::setRecalcListItems(). We
  don't use m_listItems in critical paths any longer.  So the optimization isn't
  necessary.

This CL improves performance because of the removal of the fast path.
blink_perf.dom:
select-multiple-add: 1,772 runs/s ->  1,912 runs/s
select-single-add: 867.9 runs/s -> 944.7 runs/s
select-single-remove: 137.9 runs/s -> 145.5 runs/.s


BUG=577989
==========

[tkent, 2016-07-10 23:45:54]

Patchset #1 (id:1) has been deleted

[tkent, 2016-07-10 23:45:59]

Patchset #1 (id:20001) has been deleted

[tkent, 2016-07-11 00:26:20]

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

[tkent, 2016-07-11 00:26:21]

Keishi, would you review this please?

[keishi, 2016-07-11 02:00:39]

LGTM

https://codereview.chromium.org/2131073002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/2131073002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLSelectElement.cpp:879: for (const auto
option : optionList()) {
&?

[tkent, 2016-07-11 02:12:32]

https://codereview.chromium.org/2131073002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/2131073002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLSelectElement.cpp:879: for (const auto
option : optionList()) {
On 2016/07/11 at 02:00:39, keishi wrote:
> &?

& is not used intentionally. If we add & here, compilation fails at |return
option| below because of returning a temporary object address.
Omitting & has almost no performance penalty for OptionList because copying
HTMLOptionElement* is cheap.

[tkent, 2016-07-11 02:12:35]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2016-07-11 02:12:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-11 02:16:28]

Committed patchset #1 (id:40001)

[commit-bot: I haz the power, 2016-07-11 02:17:53]

Description was changed from

==========
SELECT element: Avoid to use listItems() in HTMLSelectElement::selectOption()

Remove listItems() usage from:
 - HTMLOptionElement::index()
 - HTMLSelectElement::saveListboxActiveSelection()
 - HTMLSelectElement::selectedOption()
 - HTMLSelectElement::deselectItemsWithoutValidation()
We use optionList() instead.

These changes don't improve computational complexity. However we can avoid to
update m_listItems when we add/remove OPTIONs.

Other changes:
 - HTMLSelectElement::updateListBoxSelectioN() needs to be updated to
synchronize
  with saveListboxActiveSelection().  m_cachedStateForActiveSelection now
  represents OPTIONs instead of listItems, which includes OPTGROUP and HR.
 - Change the arguments of OptionList and OptionListIterator to |const| so that
  we can create them in const member functions of HTMLSelectElement.
 - Remove the optimized fast path in HTMLSelectElement::setRecalcListItems(). We
  don't use m_listItems in critical paths any longer.  So the optimization isn't
  necessary.

This CL improves performance because of the removal of the fast path.
blink_perf.dom:
select-multiple-add: 1,772 runs/s ->  1,912 runs/s
select-single-add: 867.9 runs/s -> 944.7 runs/s
select-single-remove: 137.9 runs/s -> 145.5 runs/.s


BUG=577989
==========

to

==========
SELECT element: Avoid to use listItems() in HTMLSelectElement::selectOption()

Remove listItems() usage from:
 - HTMLOptionElement::index()
 - HTMLSelectElement::saveListboxActiveSelection()
 - HTMLSelectElement::selectedOption()
 - HTMLSelectElement::deselectItemsWithoutValidation()
We use optionList() instead.

These changes don't improve computational complexity. However we can avoid to
update m_listItems when we add/remove OPTIONs.

Other changes:
 - HTMLSelectElement::updateListBoxSelectioN() needs to be updated to
synchronize
  with saveListboxActiveSelection().  m_cachedStateForActiveSelection now
  represents OPTIONs instead of listItems, which includes OPTGROUP and HR.
 - Change the arguments of OptionList and OptionListIterator to |const| so that
  we can create them in const member functions of HTMLSelectElement.
 - Remove the optimized fast path in HTMLSelectElement::setRecalcListItems(). We
  don't use m_listItems in critical paths any longer.  So the optimization isn't
  necessary.

This CL improves performance because of the removal of the fast path.
blink_perf.dom:
select-multiple-add: 1,772 runs/s ->  1,912 runs/s
select-single-add: 867.9 runs/s -> 944.7 runs/s
select-single-remove: 137.9 runs/s -> 145.5 runs/.s

BUG=577989

Committed: https://crrev.com/3ebd491419a62d22093a68e7ae3820157a32ecb5
Cr-Commit-Position: refs/heads/master@{#404591}
==========

[commit-bot: I haz the power, 2016-07-11 02:17:54]

Patchset 1 (id:??) landed as
https://crrev.com/3ebd491419a62d22093a68e7ae3820157a32ecb5
Cr-Commit-Position: refs/heads/master@{#404591}