[Webkit-unassigned] [Bug 63857] [DRT] implement createPopupMenu for Chromium DRT port

Wed Jul 6 09:44:13 PDT 2011

https://bugs.webkit.org/show_bug.cgi?id=63857

Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #99545|review?                     |review-
               Flag|                            |

--- Comment #4 from Tony Chang <tony at chromium.org>  2011-07-06 09:44:13 PST ---
(From update of attachment 99545)
View in context: https://bugs.webkit.org/attachment.cgi?id=99545&action=review

Just some small style nits.

> Tools/DumpRenderTree/chromium/WebViewHost.cpp:249
> +        if (widget)

Nit: Do we really need this check?  Shouldn't WebPopupMenu::create always succeed?

> Tools/DumpRenderTree/chromium/WebViewHost.cpp:254
> +    default:
> +        ASSERT_NOT_REACHED();

I would remove the default case.  That way if someone adds another WebPopupType, there will be a compiler error and the person can update this code.

> Tools/DumpRenderTree/chromium/WebViewHost.cpp:261
> +    // In Chromium, we do not this method.

Nit: This comment isn't that useful.  Can you either explain why we don't need this method or just remove the comment?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.