Bug 69812

Summary: Move m_listItems and m_recalcListItems from SelectElementData to HTMLSelectElement
Product: WebKit Reporter: Kent Tamura <tkent>
Component: FormsAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Minor CC: darin, dglazkov, rniwa, webkit.review.bot
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 69755, 69815    
Bug Blocks: 69828    
Attachments:
Description Flags
Patch
webkit.review.bot: commit-queue-
Patch 2
none
Patch for landing none

Description Kent Tamura 2011-10-10 19:29:16 PDT 
Move m_listItems and m_recalcListItems from SelectElementData to HTMLSelectElement
Comment 1 Kent Tamura 2011-10-10 19:30:38 PDT 
Created attachment 110459 [details]
Patch
Comment 2 WebKit Review Bot 2011-10-10 19:36:46 PDT 
Comment on attachment 110459 [details]
Patch

Attachment 110459 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10030195
Comment 3 Early Warning System Bot 2011-10-10 19:37:56 PDT 
Comment on attachment 110459 [details]
Patch

Attachment 110459 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10028167
Comment 4 Gyuyoung Kim 2011-10-10 19:38:14 PDT 
Comment on attachment 110459 [details]
Patch

Attachment 110459 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10028169
Comment 5 Kent Tamura 2011-10-10 19:47:49 PDT 
Created attachment 110462 [details]
Patch 2

Fix Release build
Comment 6 Ryosuke Niwa 2011-10-10 20:07:47 PDT 
Comment on attachment 110462 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=110462&action=review

> Source/WebCore/html/HTMLSelectElement.cpp:645
> +void HTMLSelectElement::checkListItems() const
> +{

It's unfortunate that we'll end up calling this function even in release builds :(
Comment 7 Kent Tamura 2011-10-10 21:16:05 PDT 
Committed r97121: <http://trac.webkit.org/changeset/97121>
Comment 8 Kent Tamura 2011-10-10 21:17:14 PDT 
(In reply to comment #7)
> Committed r97121: <http://trac.webkit.org/changeset/97121>

Landed with an HTMLSelectElementWin.cpp build fix.
Comment 9 Kent Tamura 2011-10-10 21:58:53 PDT 
http://build.webkit.org/results/Chromium%20Linux%20Release%20(Tests)/r97121%20(24480)/results.html

I rolled r97121 out because of regressions.  I should have waited until cr-linux EWS...
Comment 10 Kent Tamura 2011-10-10 22:57:58 PDT 
Comment on attachment 110462 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=110462&action=review

> Source/WebCore/html/HTMLSelectElement.cpp:656
> +        const_cast<HTMLSelectElement*>(this)->recalcListItemsInternal(false);

The function argument should not be 'false.'
The original code didn't have the bool argument, and the default value is 'true.'
Comment 11 Kent Tamura 2011-10-10 22:58:26 PDT 
Created attachment 110478 [details]
Patch for landing

Fix listItems() error.
Comment 12 WebKit Review Bot 2011-10-11 01:44:49 PDT 
Comment on attachment 110478 [details]
Patch for landing

Clearing flags on attachment: 110478

Committed r97135: <http://trac.webkit.org/changeset/97135>
Comment 13 WebKit Review Bot 2011-10-11 01:44:54 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 14 Darin Adler 2011-10-11 16:57:37 PDT 
Comment on attachment 110462 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=110462&action=review

>> Source/WebCore/html/HTMLSelectElement.cpp:645
>> +{
> 
> It's unfortunate that we'll end up calling this function even in release builds :(

The simple way to avoid that is to put an empty inline version of this in the header. Or to make sure this is called only in this file and mark it inline in here.