New SELECT Popup: Very slow to open a popup with many items.

New SELECT Popup: Very slow to open a popup with many items.

To open it quickly, this CL does:
 - Skip color and font styling if the popup will have 100+ items.
 - Optimize ListPicker._updateChildren
   It adds elements via DocumentFragment.
 - Reorder various things to avoid extra layout
 - Avoid to access IDL attributes

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

[tkent, 2015-05-26 02:04:57]

Patchset #1 (id:1) has been deleted

[tkent, 2015-05-26 03:06:40]

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

[tkent, 2015-05-26 03:06:41]

Keishi, please review this.

This makes http://www.anoukis-m.com/test.php 4x faster.  But It still takes two
seconds or so.

[keishi, 2015-05-26 03:48:01]

LGTM

[tkent, 2015-05-26 06:31:20]

I updated listPicker.js.  Please take another look at it.

[tkent, 2015-05-26 06:32:46]

On 2015/05/26 06:31:20, tkent wrote:
> I updated listPicker.js.  Please take another look at it.

Note that removing getScrollbarWidth() didn't have notable performance gain.

[keishi, 2015-05-26 06:41:26]

> Note that removing getScrollbarWidth() didn't have notable performance gain.
I'm curious how fast the popup opens when there is no extra layout (i.e. don't
call _fixWindowSize)?

https://codereview.chromium.org/1149153003/diff/40001/Source/web/resources/li...
File Source/web/resources/listPicker.js (right):

https://codereview.chromium.org/1149153003/diff/40001/Source/web/resources/li...
Source/web/resources/listPicker.js:278: return;
When you change from
<select><optgroup></optgroup></select>
to
<select><option>foo</option></select>
, the fragment will contain the new option but we still need to remove the
obsolete optgroup.

[tkent, 2015-05-26 07:01:41]

On 2015/05/26 06:41:26, keishi wrote:
> > Note that removing getScrollbarWidth() didn't have notable performance gain.
> I'm curious how fast the popup opens when there is no extra layout (i.e. don't
> call _fixWindowSize)?

With Patch Set 2, ListPicker constructor takes 1.4 seconds.
ListPicker constructor skipping _fixWindowSize() takes 1.1 seconds.

>
https://codereview.chromium.org/1149153003/diff/40001/Source/web/resources/li...
> File Source/web/resources/listPicker.js (right):
> 
>
https://codereview.chromium.org/1149153003/diff/40001/Source/web/resources/li...
> Source/web/resources/listPicker.js:278: return;
> When you change from
> <select><optgroup></optgroup></select>
> to
> <select><option>foo</option></select>
> , the fragment will contain the new option but we still need to remove the
> obsolete optgroup.

DocumentFragment is used only if |parent| has no children or all of |parent|
children are recycled.

[keishi, 2015-05-26 08:25:47]

On 2015/05/26 07:01:41, tkent wrote:
> On 2015/05/26 06:41:26, keishi wrote:
> > > Note that removing getScrollbarWidth() didn't have notable performance
gain.
> > I'm curious how fast the popup opens when there is no extra layout (i.e.
don't
> > call _fixWindowSize)?
> 
> With Patch Set 2, ListPicker constructor takes 1.4 seconds.
> ListPicker constructor skipping _fixWindowSize() takes 1.1 seconds.

Hmm. Firefox is so fast.

> >
>
https://codereview.chromium.org/1149153003/diff/40001/Source/web/resources/li...
> > File Source/web/resources/listPicker.js (right):
> > 
> >
>
https://codereview.chromium.org/1149153003/diff/40001/Source/web/resources/li...
> > Source/web/resources/listPicker.js:278: return;
> > When you change from
> > <select><optgroup></optgroup></select>
> > to
> > <select><option>foo</option></select>
> > , the fragment will contain the new option but we still need to remove the
> > obsolete optgroup.
> 
> DocumentFragment is used only if |parent| has no children or all of |parent|
> children are recycled.

Right, sorry.

LGTM

[tkent, 2015-05-26 08:27:39]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2015-05-26 08:27:46]

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

[commit-bot: I haz the power, 2015-05-26 08:30:59]

Committed patchset #2 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195900