[webkit-reviews] review granted: [Bug 29233] Add ENABLE(DRAG_SUPPORT) : [Attachment 39561] Fix DerivedSources.make for all platforms

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Greg Bolsinga
<bolsinga at apple.com>'s request for review:
Bug 29233: Add ENABLE(DRAG_SUPPORT)
https://bugs.webkit.org/show_bug.cgi?id=29233

Attachment 39561: Fix DerivedSources.make for all platforms
https://bugs.webkit.org/attachment.cgi?id=39561&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
> diff --git a/WebCore/page/DragController.cpp
b/WebCore/page/DragController.cpp
> index 97f53a5..01f4bd6 100644
> --- a/WebCore/page/DragController.cpp
> +++ b/WebCore/page/DragController.cpp
> @@ -26,6 +26,7 @@
>  #include "config.h"
>  #include "DragController.h"
>  
> +#if ENABLE(DRAG_SUPPORT)
>  #include "CSSStyleDeclaration.h"
>  #include "Clipboard.h"
>  #include "ClipboardAccessPolicy.h"
> @@ -785,3 +786,4 @@ void DragController::placeDragCaret(const IntPoint&
windowPoint)
>  }
>  
>  } // namespace WebCore
> +#endif // ENABLE(DRAG_SUPPORT)

Please add a blank line after the "#if ENABLE(.." statement and before the
"#endif // ..." statement.

> diff --git a/WebCore/page/mac/DragControllerMac.mm
b/WebCore/page/mac/DragControllerMac.mm
> index c476df7..e4a99f9 100644
> --- a/WebCore/page/mac/DragControllerMac.mm
> +++ b/WebCore/page/mac/DragControllerMac.mm
> @@ -26,6 +26,7 @@
>  #import "config.h"
>  #import "DragController.h"
>  
> +#if ENABLE(DRAG_SUPPORT)
>  #import "DragData.h"
>  #import "Frame.h"
>  #import "FrameView.h"
> @@ -76,3 +77,4 @@ void DragController::cleanupAfterSystemDrag()
>  }
>  
>  }
> +#endif // ENABLE(DRAG_SUPPORT)

Ditto for blank lines.	Adding a comment for the "}" used to end the WebCore
namespace would be nice, too, but not necessary.

> diff --git a/WebCore/platform/DragData.cpp b/WebCore/platform/DragData.cpp
> index bf2275a..faa4f12 100644
> --- a/WebCore/platform/DragData.cpp
> +++ b/WebCore/platform/DragData.cpp
> @@ -26,6 +26,7 @@
>  #include "config.h"
>  #include "DragData.h"
>  
> +#if ENABLE(DRAG_SUPPORT)
>  namespace WebCore {
>  
>  #if !PLATFORM(MAC)
> @@ -40,3 +41,4 @@ DragData::DragData(DragDataRef data, const IntPoint&
clientPosition, const IntPo
>  #endif
>  
>  }
> +#endif // ENABLE(DRAG_SUPPORT)

Ditto.

> diff --git a/WebCore/platform/DragImage.cpp b/WebCore/platform/DragImage.cpp
> index adf9a57..224a7bd 100644
> --- a/WebCore/platform/DragImage.cpp
> +++ b/WebCore/platform/DragImage.cpp
> @@ -25,6 +25,8 @@
>  
>  #include "config.h"
>  #include "DragImage.h"
> +
> +#if ENABLE(DRAG_SUPPORT)
>  #include "DragController.h"
>  
>  #include "Frame.h"
> @@ -72,3 +74,4 @@ DragImageRef createDragImageForSelection(Frame* frame)
>  }
>  
>  }
> +#endif // ENABLE(DRAG_SUPPORT)

Ditto.

> diff --git a/WebCore/platform/mac/DragDataMac.mm
b/WebCore/platform/mac/DragDataMac.mm
> index a7b751c..e742627 100644
> --- a/WebCore/platform/mac/DragDataMac.mm
> +++ b/WebCore/platform/mac/DragDataMac.mm
> @@ -26,6 +26,7 @@
>  #import "config.h"
>  #import "DragData.h"
>  
> +#if ENABLE(DRAG_SUPPORT)
>  #import "ClipboardMac.h"
>  #import "ClipboardAccessPolicy.h"
>  #import "Document.h"
> @@ -128,4 +129,4 @@ PassRefPtr<DocumentFragment>
DragData::asFragment(Document*) const
>  }
>      
>  }
> -
> +#endif // ENABLE(DRAG_SUPPORT)

Ditto.

> diff --git a/WebCore/platform/mac/DragImageMac.mm
b/WebCore/platform/mac/DragImageMac.mm
> index 842e6d4..065934a 100644
> --- a/WebCore/platform/mac/DragImageMac.mm
> +++ b/WebCore/platform/mac/DragImageMac.mm
> @@ -26,6 +26,7 @@
>  #import "config.h"
>  #import "DragImage.h"
>  
> +#if ENABLE(DRAG_SUPPORT)
>  #import "CachedImage.h"
>  #import "Image.h"
>  #import "KURL.h"
> @@ -99,3 +100,4 @@ RetainPtr<NSImage>
createDragImageIconForCachedImage(CachedImage* image)
>  }
>      
>  }
> +#endif // ENABLE(DRAG_SUPPORT)

Ditto.

> diff --git a/WebKit/mac/ChangeLog b/WebKit/mac/ChangeLog
> diff --git a/WebKit/mac/WebView/WebFrame.mm b/WebKit/mac/WebView/WebFrame.mm
> diff --git a/WebKit/mac/WebView/WebFrameInternal.h
b/WebKit/mac/WebView/WebFrameInternal.h
> diff --git a/WebKit/mac/WebView/WebFrameView.mm
b/WebKit/mac/WebView/WebFrameView.mm
> diff --git a/WebKit/mac/WebView/WebHTMLView.mm
b/WebKit/mac/WebView/WebHTMLView.mm
> diff --git a/WebKit/mac/WebView/WebView.mm b/WebKit/mac/WebView/WebView.mm

As we discussed offline, please leave these changes out.  It doesn't make sense
to disable context menu support for code in WebKit/mac.  When WebKit code for
iPhone lands, it will be refactored or moved instead of shared within
WebKit/mac.  (Note that mac-specific files in WebCore are needed to prevent
excessive use of EXCLUDED_SOURCE_FILE_NAMES in Xcode.)

r=me