[webkit-reviews] review denied: [Bug 35811] [chromium] need DragImage implementation : [Attachment 50241] review comments

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Evan Stade
<estade at chromium.org>'s request for review:
Bug 35811: [chromium] need DragImage implementation
https://bugs.webkit.org/show_bug.cgi?id=35811

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> Index: WebCore/platform/chromium/DragImageChromium.cpp
...
> +#if OS(DARWIN)
> +#include "skia/ext/skia_utils_mac.h"
> +#endif

Can you use CG in the Mac port instead?  We should really avoid using
skia/ext in WebCore.  It introduces a circular dependency between the
repositories!


> +DragImageRef createDragImageFromImage(Image* image)
> +{
> +#if OS(DARWIN)
> +    SkBitmap bitmap = gfx::CGImageToSkBitmap(image->getCGImageRef());

please avoid adding gfx:: namespace stuff to WebCore!  that stuff is
chromium only.


> Index: WebKit/chromium/ChangeLog
...
> +	   * public/WebDragImageRef.h: Added.
> +	   * src/DragClientImpl.cpp:
> +	   (WebKit::DragClientImpl::startDrag):
> +	   * src/WebViewImpl.cpp:
> +	   (WebKit::WebViewImpl::startDragging):
> +	   * src/WebViewImpl.h:

is there some other public interface change missing?  I don't see
any public API that uses WebDragImageRef.

also, please see WebImage.h to note how things differ between
WEBKIT_USING_SKIA and WEBKIT_USING_CG.

It seems like WebDragImageRef should be made consistent with
WebImage.