[webkit-reviews] review granted: [Bug 25847] Clean up ClipboardMac : [Attachment 30437] ClipboardMac cleanup

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 25847: Clean up ClipboardMac
https://bugs.webkit.org/show_bug.cgi?id=25847

Attachment 30437: ClipboardMac cleanup
https://bugs.webkit.org/attachment.cgi?id=30437&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
I think it's unnecessary to shout "NOTE".

There's no reason to have a default value for the "onlyFirstURL" boolean.

> +    if (!absoluteURLs) {
> +	   if ([availableTypes containsObject:NSURLPboardType]) {

Using && here would avoid an otherwise-unnecessary level of if nesting. But I
think early return would be even better. One of the advantages of breaking
something into a separate function.

> +	   // "URL" and "text/url-list" both map to NSURLPboardType in
cocoaTypeFromMIMEType(), "URL" only wants the first URL

It's a little strange to have this same comment twice.

> +    unsigned count = [types count];
> +    for (unsigned i = 0; i < count; i++) {

As long as you are touching this code, the correct type here is NSUInteger
rather than unsigned. Same for the other count and index in
absoluteURLsFromPasteboardFilenames.

r=me