[webkit-reviews] review denied: [Bug 73916] Drop ENABLE_NO_LISTBOX_RENDERING, and make it a runtime decision. : [Attachment 118040] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Pierre Rossi
<pierre.rossi at gmail.com>'s request for review:
Bug 73916: Drop ENABLE_NO_LISTBOX_RENDERING, and make it a runtime decision.
https://bugs.webkit.org/show_bug.cgi?id=73916

Attachment 118040: Patch
https://bugs.webkit.org/attachment.cgi?id=118040&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=118040&action=review


Thank you for working on this!

> Source/WebCore/html/HTMLSelectElement.cpp:187
> +    Page* page = 0;
> +    if (Frame* frame = document()->frame())
> +	   page = frame->page();

We have Document::page().

> Source/WebCore/html/HTMLSelectElement.cpp:188
> +    RenderTheme* renderTheme = page ? page->theme() :
RenderTheme::defaultTheme().leakRef();

Do not leak.
This should be RefPtr<RenterTheme> renderTheme = ...

> Source/WebCore/rendering/RenderTheme.h:210
> +    virtual bool usesMenuList() const { return false; }

The function name should be more descriptive. 
selectElementAlwaysUsesMenuList() or something?