[webkit-reviews] review granted: [Bug 103092] [chromium] Allow plugins to opt-in to receive synthetic mouse events out of touch events. : [Attachment 176057] Build fix!

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Sadrul Habib Chowdhury
<sadrul at chromium.org>'s request for review:
Bug 103092: [chromium] Allow plugins to opt-in to receive synthetic mouse
events out of touch events.
https://bugs.webkit.org/show_bug.cgi?id=103092

Attachment 176057: Build fix!
https://bugs.webkit.org/attachment.cgi?id=176057&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176057&action=review


> Source/WebKit/chromium/public/WebPluginContainer.h:121
> +    // Notifies when the plugin starts/stops accepting touch events. This is
deprecated, use requestTouchEventType instead.
>      virtual void setIsAcceptingTouchEvents(bool) = 0;

The plan is to remove this after updating all the callers on the Chromium side,
right?

> Source/WebKit/chromium/src/WebPluginContainerImpl.cpp:527
>  void WebPluginContainerImpl::setIsAcceptingTouchEvents(bool
acceptingTouchEvents)
>  {
> -    if (m_isAcceptingTouchEvents == acceptingTouchEvents)
> +    requestTouchEventType(acceptingTouchEvents ? TouchEventRequestTypeRaw :
TouchEventRequestTypeNone);
> +}

Do tests still call this?  Could you replace the body with a NOT_REACHED()?

> Tools/DumpRenderTree/chromium/TestWebPlugin.cpp:124
> +    static const WebString kPrimitiveTrue = WebString::fromUTF8("true");
> +    static const WebString kPrimitiveSynthetic =
WebString::fromUTF8("synthetic");

I would update the old tests to use "raw" instead of "true".  It's confusing
that the valid values for the attribute are "true" or "synthetic".

Also, this should probably just be const char* to avoid initialization
(operator== is defined for String, const char* so you can still use == below).