Bug 137361 – [GTK] Fix build when DRAG_SUPPORT is disabled

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Lorenzo Tilve]

It is necessary to flag out some code to fix the build if DRAG_SUPPORT is disabled.

[Lorenzo Tilve]

Created attachment 239140 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Carlos Garcia Campos]

Comment on attachment 239140 [details]
Patch

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.

[Lorenzo Tilve]

Created attachment 239297 [details]
Patch

[Lorenzo Tilve]

(In reply to comment #3)
> (From update of attachment 239140 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239140&action=review

Thanks for the review, I've fixed the commented issues.

[Carlos Garcia Campos]

Comment on attachment 239297 [details]
Patch

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

> Source/WebKit2/Shared/gtk/ArgumentCodersGtk.h:47
>  template<> struct ArgumentCoder<WebCore::DragData> {
> +#if ENABLE(DRAG_SUPPORT)
>      static void encode(ArgumentEncoder&, const WebCore::DragData&);
>      static bool decode(ArgumentDecoder&, WebCore::DragData&);
> +#endif
>  };

Why are the ifdefs inside the struct instead of the whole struct definition inside the #ifdef block?

[Lorenzo Tilve]

Created attachment 239298 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 239298 [details]
Patch

Clearing flags on attachment: 239298

Committed r174328: <http://trac.webkit.org/changeset/174328>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.