[webkit-reviews] review granted: [Bug 52343] WebKit2: add support for drag and drop : [Attachment 79521] Patch10

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 19 17:31:57 PST 2011


Darin Adler <darin at apple.com> has granted Enrica Casucci <enrica at apple.com>'s
request for review:
Bug 52343: WebKit2: add support for drag and drop
https://bugs.webkit.org/show_bug.cgi?id=52343

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=79521&action=review

> Source/WebKit2/Shared/mac/PasteboardTypes.mm:43
> +    CFRetain(types);

This will CFRetain every time the function is called, not just the first time.
So it will work, but its performance will be a little slow and there is some
small risk of overflow.

You could do something simple like this:

    static inline NSArray *retain(NSArray *array)
    {
	CFRetain(array);
	return array;
    }

    ...

    static NSArray *types = retain([NSArray ...]);

That would work better.

> Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:285
> +    RetainPtr<NSImage> dragNSImage = [[[NSImage alloc]
initWithCGImage:CGBitmapContextCreateImage(graphicsContext->platformContext())
size:imageSize] autorelease];

Instead of doing autorelease you should do an adopt here.

    RetainPtr<NSImage> dragNSImage(AdoptNS, [[NSImage alloc]
initWithCGImage:CGBitmapContextCreateImage(graphicsContext->platformContext())
size:imageSize]);

The code is already correct, with no leak, but the adopt version is more
efficient.


More information about the webkit-reviews mailing list