Bug 137361 - [GTK] Fix build when DRAG_SUPPORT is disabled
Summary: [GTK] Fix build when DRAG_SUPPORT is disabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Lorenzo Tilve
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-02 14:28 PDT by Lorenzo Tilve
Modified: 2014-10-05 04:28 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.72 KB, patch)
2014-10-02 14:42 PDT, Lorenzo Tilve
no flags Details | Formatted Diff | Diff
Patch (7.67 KB, patch)
2014-10-05 01:11 PDT, Lorenzo Tilve
no flags Details | Formatted Diff | Diff
Patch (7.73 KB, patch)
2014-10-05 01:57 PDT, Lorenzo Tilve
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lorenzo Tilve 2014-10-02 14:28:15 PDT
It is necessary to flag out some code to fix the build if DRAG_SUPPORT is disabled.
Comment 1 Lorenzo Tilve 2014-10-02 14:42:16 PDT
Created attachment 239140 [details]
Patch
Comment 2 WebKit Commit Bot 2014-10-02 14:44:27 PDT
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
Comment 3 Carlos Garcia Campos 2014-10-02 23:46:36 PDT
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.
Comment 4 Lorenzo Tilve 2014-10-05 01:11:24 PDT
Created attachment 239297 [details]
Patch
Comment 5 Lorenzo Tilve 2014-10-05 01:16:13 PDT
(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.
Comment 6 Carlos Garcia Campos 2014-10-05 01:21:50 PDT
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?
Comment 7 Lorenzo Tilve 2014-10-05 01:57:25 PDT
Created attachment 239298 [details]
Patch
Comment 8 WebKit Commit Bot 2014-10-05 04:28:31 PDT
Comment on attachment 239298 [details]
Patch

Clearing flags on attachment: 239298

Committed r174328: <http://trac.webkit.org/changeset/174328>
Comment 9 WebKit Commit Bot 2014-10-05 04:28:35 PDT
All reviewed patches have been landed.  Closing bug.