Bug 88059 – REGRESSION(r109729) [Form] Rendering of select/optgroup/option combination is too slow.

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

This bug is imported from http://crbug.com/129334

Slowness is brought by StyleResolver::locateSharedStyle. When style object is shared among optgroup element, StyleResolver::locateSharedStyle checks option elements before it in document order.

For example, if we have following HTML:

<select>
  <optgroup label="1"> <option value="1-1>1-1</option> ... x 100 </optgroup>
  <optgroup label="2"> <option value="2-1>2-1</option> ... x 100 </optgroup>
  ...
  <optgroup label="10"> <option value="2-1>2-1</option> ... x 100 </optgroup>
</select>

locateSharedStyle checks last 10 option elements in optgroup[label="9" to label="1"] for option elements in optgroup[label="10"] by

            if (currentNode->renderStyle() == parentStyle && currentNode->lastChild()) {
                // Adjust for unused reserved tries.
                visitedNodeCount -= cStyleSearchThreshold - subcount;
                return currentNode->lastChild();
            }

Before r109729, style object isn't shared for optgroup element by:

bool StyleResolver::canShareStyleWithControl(StyledElement* element) const
{
    HTMLInputElement* thisInputElement = element->toInputElement();
    HTMLInputElement* otherInputElement = m_element->toInputElement();

    if (!thisInputElement || !otherInputElement)
        return false;

Because, "optgroup" element isn't "input" element.

BTW, canShareStyleWithControl should be smarter than non-input element to reduce memory usage for RenderStyle object.

Created attachment 145241 [details]
Patch 1

Created attachment 145242 [details]
Patch 2

Comment on attachment 145242 [details]
Patch 2

Could you review this patch?
Thanks in advance.

[Kent Tamura]

Comment on attachment 145242 [details]
Patch 2

Looks ok.

[WebKit Review Bot]

Comment on attachment 145242 [details]
Patch 2

Clearing flags on attachment: 145242

Committed r119213: <http://trac.webkit.org/changeset/119213>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Zoltan Arvai]

Layout tests failing on Qt and GTK after r119213

fast/forms/select/menulist-disabled-option.html:

Three and Two changed to One and Two on the select box.


fast/css/text-transform-select.html:

--- /ramdisk/qt-linux-release/build/layout-test-results/fast/css/text-transform-select-expected.txt 
+++ /ramdisk/qt-linux-release/build/layout-test-results/fast/css/text-transform-select-actual.txt 
@@ -9,8 +9,8 @@
       RenderBlock {DIV} at (0,21) size 768x96
         RenderMenuList {SELECT} at (2,68) size 88x26 [bgcolor=#FFFFFF]
           RenderBlock (anonymous) at (4,2) size 60x21
-            RenderText at (0,0) size 53x21
-              text run at (0,0) width 53: "HELLO"
+            RenderText at (0,0) size 45x21
+              text run at (0,0) width 45: "heLLo"
         RenderText {#text} at (92,70) size 4x21
           text run at (92,70) width 4: " "
         RenderListBox {SELECT} at (98,2) size 82x89 [bgcolor=#FFFFFF] [border: (1px inset #808080)]
@@ -23,8 +23,8 @@
       RenderBlock {DIV} at (0,117) size 768x96
         RenderMenuList {SELECT} at (2,68) size 82x26 [bgcolor=#FFFFFF]
           RenderBlock (anonymous) at (4,2) size 54x21
-            RenderText at (0,0) size 48x21
-              text run at (0,0) width 48: "HeLLo"
+            RenderText at (0,0) size 45x21
+              text run at (0,0) width 45: "heLLo"
         RenderText {#text} at (86,70) size 4x21
           text run at (86,70) width 4: " "
         RenderListBox {SELECT} at (92,2) size 76x89 [bgcolor=#FFFFFF] [border: (1px inset #808080)]
@@ -37,8 +37,8 @@
       RenderBlock {DIV} at (0,213) size 768x96
         RenderMenuList {SELECT} at (2,68) size 67x26 [bgcolor=#FFFFFF]
           RenderBlock (anonymous) at (4,2) size 39x21
-            RenderText at (0,0) size 35x21
-              text run at (0,0) width 35: "hello"
+            RenderText at (0,0) size 45x21
+              text run at (0,0) width 45: "heLLo"
         RenderText {#text} at (71,70) size 4x21
           text run at (71,70) width 4: " "
         RenderListBox {SELECT} at (77,2) size 61x89 [bgcolor=#FFFFFF] [border: (1px inset #808080)]
@@ -51,8 +51,8 @@
       RenderBlock {DIV} at (0,309) size 768x96
         RenderMenuList {SELECT} at (2,68) size 50x26 [bgcolor=#FFFFFF]
           RenderBlock (anonymous) at (4,2) size 22x21
-            RenderText at (0,0) size 22x21
-              text run at (0,0) width 22: "SS"
+            RenderText at (0,0) size 10x21
+              text run at (0,0) width 10: "\x{DF}"
         RenderText {#text} at (54,70) size 4x21
           text run at (54,70) width 4: " "
         RenderListBox {SELECT} at (60,2) size 44x89 [bgcolor=#FFFFFF] [border: (1px inset #808080)]

[Csaba Osztrogonác]

and Lion and EFL too ... Could you guys fix it or should we roll it out?

[WebKit Review Bot]

Re-opened since this is blocked by 88084

[Stephen Chenney]

r119213 also broke Chromium tests:
fast/css/text-transform-select.html
fast/forms/select/menulist-disabled-option.html

It was rolled out.

[Stephen Chenney]

The Chromium failures were on Mac and Win, not Linux.

Created attachment 145979 [details]
Patch 3

(In reply to comment #10)
> r119213 also broke Chromium tests:
> fast/css/text-transform-select.html
> fast/forms/select/menulist-disabled-option.html
> 
> It was rolled out.

I file a bug to share RenderStyle object for option and optgroup elements, bug 88405. A root causes are different in these test cases.

Comment on attachment 145979 [details]
Patch 3

Could you review this patch?
Thanks in advance.

[Kent Tamura]

Comment on attachment 145979 [details]
Patch 3

ok

[WebKit Review Bot]

Comment on attachment 145979 [details]
Patch 3

Clearing flags on attachment: 145979

Committed r119672: <http://trac.webkit.org/changeset/119672>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.