[webkit-reviews] review denied: [Bug 137361] [GTK] Fix build when DRAG_SUPPORT is disabled : [Attachment 239140] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Lorenzo Tilve
<ltilve at igalia.com>'s request for review:
Bug 137361: [GTK] Fix build when DRAG_SUPPORT is disabled
https://bugs.webkit.org/show_bug.cgi?id=137361

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

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=239140&action=review


Thanks for the patch. I have a few comments.

> Source/WebCore/platform/gtk/GtkDragAndDropHelper.cpp:89
> +#if ENABLE(DRAG_SUPPORT)
>      DragData dragData(context->dataObject.get(), position,
>			 convertWidgetPointToScreenPoint(m_widget, position),
>			 DragOperationNone);
>      context->exitedCallback(m_widget, dragData, context->dropHappened);
> +#endif

I think the entire class GtkDragAndDropHelper should be only compiled when drag
support is enabled

> Source/WebKit2/Shared/gtk/ArgumentCodersGtk.cpp:234
> +#if ENABLE(DRAG_SUPPORT)
>      dragData = DragData(platformData.release().leakRef(), clientPosition,
globalPosition, static_cast<DragOperation>(sourceOperationMask),
>			   static_cast<DragApplicationFlags>(flags));
> +#endif

DragData is only used by messages defined inside a drag_support #ifdef, so we
should only define encode/decode for DragData when drag_support is enabled

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:276
>  void PageClientImpl::startDrag(const WebCore::DragData& dragData,
PassRefPtr<ShareableBitmap> dragImage)
>  {
> +#if ENABLE(DRAG_SUPPORT)
>      webkitWebViewBaseStartDrag(WEBKIT_WEB_VIEW_BASE(m_viewWidget), dragData,
dragImage);
> +#endif
>  }

PageClientImpl::startDrag() is only called by WebPageProxy::startDrag(), which
is the handler of an IPC message defined only when drag support is enabled,
see:

#if PLATFORM(GTK) && ENABLE(DRAG_SUPPORT)
    StartDrag(WebCore::DragData dragData, WebKit::ShareableBitmap::Handle
dragImage)
#endif

So, we should add the guard to PageClient.h to only define startDrag when drag
support is enabled.