[webkit-reviews] review granted: [Bug 69631] Towards making PopupMenuClient more testable : [Attachment 110592] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 11 16:11:24 PDT 2011


Simon Fraser (smfr) <simon.fraser at apple.com> has granted Fady Samuel
<fsamuel at chromium.org>'s request for review:
Bug 69631: Towards making PopupMenuClient more testable
https://bugs.webkit.org/show_bug.cgi?id=69631

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=110592&action=review


> Source/WebCore/rendering/RenderMenuList.cpp:307
> +    m_popup->show(boundingBoxRect(), document()->view(),
>	   select->optionToListIndex(select->selectedIndex()));

You could unwrap this line.

> Source/WebCore/testing/Internals.cpp:457
> +    return reinterpret_cast<PopupMenuClient*>(object);

This should be a static_cast too.

> Source/WebCore/testing/Internals.cpp:472
> +    return 0;
> +
> +}

Extra blank line.

> Source/WebCore/testing/Internals.h:107
> +    static const PopupMenuClient* toPopupMenuClient(RenderObject*);

I don't think there's any benefit in this being a class method. It could just
be static in the impl.


More information about the webkit-reviews mailing list