Bug 69812 - Move m_listItems and m_recalcListItems from SelectElementData to HTMLSelectElement
Summary: Move m_listItems and m_recalcListItems from SelectElementData to HTMLSelectEl...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P3 Minor
Assignee: Kent Tamura
URL:
Keywords:
Depends on: 69755 69815
Blocks: 69828
  Show dependency treegraph
 
Reported: 2011-10-10 19:29 PDT by Kent Tamura
Modified: 2011-10-11 16:57 PDT (History)
4 users (show)

See Also:


Attachments
Patch (24.52 KB, patch)
2011-10-10 19:30 PDT, Kent Tamura
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch 2 (24.49 KB, patch)
2011-10-10 19:47 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch for landing (25.23 KB, patch)
2011-10-10 22:58 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.