Summary: | [GTK] Fix build when DRAG_SUPPORT is disabled | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Lorenzo Tilve <ltilve> | ||||||||
Component: | WebKitGTK | Assignee: | Lorenzo Tilve <ltilve> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | berto, cgarcia, commit-queue, gustavo, mrobinson | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Lorenzo Tilve
2014-10-02 14:28:15 PDT
Created attachment 239140 [details]
Patch
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 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. Created attachment 239297 [details]
Patch
(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 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? Created attachment 239298 [details]
Patch
Comment on attachment 239298 [details] Patch Clearing flags on attachment: 239298 Committed r174328: <http://trac.webkit.org/changeset/174328> All reviewed patches have been landed. Closing bug. |