Bug 76441 - [Chromium] Cleanup of WebPopupMenuImpl
Summary: [Chromium] Cleanup of WebPopupMenuImpl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P3 Minor
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-17 02:12 PST by Kent Tamura
Modified: 2012-01-26 20:48 PST (History)
3 users (show)

See Also:


Attachments
Patch (19.49 KB, patch)
2012-01-17 02:30 PST, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch (19.49 KB, patch)
2012-01-17 02:33 PST, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 3 (19.47 KB, patch)
2012-01-26 18:32 PST, 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 2012-01-17 02:12:41 PST
[Chromium] Cleanup of WebPopupMenuImpl
Comment 1 Kent Tamura 2012-01-17 02:30:56 PST
Created attachment 122740 [details]
Patch
Comment 2 Kent Tamura 2012-01-17 02:33:44 PST
Created attachment 122741 [details]
Patch
Comment 3 Kentaro Hara 2012-01-26 08:40:59 PST
Comment on attachment 122741 [details]
Patch

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

> Source/WebKit/chromium/src/WebPopupMenuImpl.cpp:169
> +    deref(); // Balances ref() from WebWidget::Create.

Where is WebWidget::Create? I couldn't find it.

> Source/WebKit/chromium/src/WebPopupMenuImpl.cpp:229
> +    // FIXME: WebKit seems to always return false on mouse events methods. For

FIXME (jcampan)?
Comment 4 Kent Tamura 2012-01-26 18:30:41 PST
Comment on attachment 122741 [details]
Patch

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

>> Source/WebKit/chromium/src/WebPopupMenuImpl.cpp:169
>> +    deref(); // Balances ref() from WebWidget::Create.
> 
> Where is WebWidget::Create? I couldn't find it.

Good catch!  It should be WebPopupMenu::create.

>> Source/WebKit/chromium/src/WebPopupMenuImpl.cpp:229
>> +    // FIXME: WebKit seems to always return false on mouse events methods. For
> 
> FIXME (jcampan)?

Our style is FIXME without a name.
http://www.webkit.org/coding/coding-style.html#comments-fixme
Comment 5 Kent Tamura 2012-01-26 18:32:09 PST
Created attachment 124232 [details]
Patch 3
Comment 6 Kent Tamura 2012-01-26 19:00:28 PST
Comment on attachment 124232 [details]
Patch 3

Thanks!
Comment 7 WebKit Review Bot 2012-01-26 20:48:41 PST
Comment on attachment 124232 [details]
Patch 3

Clearing flags on attachment: 124232

Committed r106084: <http://trac.webkit.org/changeset/106084>
Comment 8 WebKit Review Bot 2012-01-26 20:48:46 PST
All reviewed patches have been landed.  Closing bug.