Modify Pasteboard handling in OSExchangeData

ui/base/dragdrop/os_exchange_data_provider_mac.mm

Index: ui/base/dragdrop/os_exchange_data_provider_mac.mm
diff --git a/ui/base/dragdrop/os_exchange_data_provider_mac.mm b/ui/base/dragdrop/os_exchange_data_provider_mac.mm
index 030d0d084121808976d4e6ff1ec56bc82c9fd16e..e34edabfd9de112363bb4f1a7d3af1e185847a70 100644
--- a/ui/base/dragdrop/os_exchange_data_provider_mac.mm
+++ b/ui/base/dragdrop/os_exchange_data_provider_mac.mm
@@ -12,6 +12,7 @@
 #include "base/strings/utf_string_conversions.h"
 #import "third_party/mozilla/NSPasteboard+Utils.h"
 #import "ui/base/clipboard/clipboard_util_mac.h"
+#import "ui/base/dragdrop/cocoa_dnd_util.h"
 #include "url/gurl.h"
 
 namespace ui {
@@ -40,16 +41,16 @@ bool OSExchangeDataProviderMac::DidOriginateFromRenderer() const {
 }
 
 void OSExchangeDataProviderMac::SetString(const base::string16& string) {
-  [pasteboard_->get() writeObjects:@[ base::SysUTF16ToNSString(string) ]];
+  [pasteboard_->get() setString:base::SysUTF16ToNSString(string)
+                        forType:NSPasteboardTypeString];
 }
 
 void OSExchangeDataProviderMac::SetURL(const GURL& url,
                                        const base::string16& title) {
-  NSURL* ns_url = [NSURL URLWithString:base::SysUTF8ToNSString(url.spec())];
-  [pasteboard_->get() writeObjects:@[ ns_url ]];
-
-  [pasteboard_->get() setString:base::SysUTF16ToNSString(title)
-                        forType:kCorePasteboardFlavorType_urln];
+  base::scoped_nsobject<NSPasteboardItem> item =
+      ClipboardUtil::PasteboardItemFromUrl(base::SysUTF8ToNSString(url.spec()),
+                                           base::SysUTF16ToNSString(title));
+  ui::ClipboardUtil::AddDataToPasteboard(pasteboard_->get(), item);
 }
 
 void OSExchangeDataProviderMac::SetFilename(const base::FilePath& path) {
@@ -71,12 +72,10 @@ void OSExchangeDataProviderMac::SetPickledData(
 
 bool OSExchangeDataProviderMac::GetString(base::string16* data) const {
   DCHECK(data);
-  NSArray* items =
-      [pasteboard_->get() readObjectsForClasses:@[ [NSString class] ]
-                                        options:@{}];
+  NSString* item = [pasteboard_->get() stringForType:NSPasteboardTypeString];

[erikchen, 2016/05/04 18:56:21]

Can you confirm that this doesn't break anything? Your new code is "cleaner"
than the old code, but also changes the behavior

[spqchan, 2016/05/05 17:41:05]

Sure thing, currently Cocoa doesn't actually use OSExchangeData. The existing
implementation was implemented to just pass the unit tests. I made sure that my
changes will still pass the OSExchangeDataTest

 
   // There was no NSString, check for an NSURL.
-  if ([items count] == 0) {
+  if (!item) {

[erikchen, 2016/05/04 18:56:21]

minor nit: Why a negative conditional? All else being held equal, prefer
positive conditions. e.g.:


"""
if (item) {
  *data = base::SysNSStringToUTF16(item);
  return true;
}

// There was no NSString, check for an NSURL.
"""

[spqchan, 2016/05/05 17:41:05]

Done. I was trying to follow the format of the existing code.

     GURL url;
     base::string16 title;
     bool result =
@@ -86,7 +85,7 @@ bool OSExchangeDataProviderMac::GetString(base::string16* data) const {
     return result;
   }
 
-  *data = base::SysNSStringToUTF16([items objectAtIndex:0]);
+  *data = base::SysNSStringToUTF16(item);
   return true;
 }
 
@@ -96,29 +95,9 @@ bool OSExchangeDataProviderMac::GetURLAndTitle(
     base::string16* title) const {
   DCHECK(url);
   DCHECK(title);
-  NSArray* items =
-      [pasteboard_->get() readObjectsForClasses:@[ [NSURL class] ] options:@{}];
-  if ([items count] == 0)
-    return false;
-
-  NSURL* ns_url = [items objectAtIndex:0];
-
-  if (policy == OSExchangeData::DO_NOT_CONVERT_FILENAMES) {
-    // If the URL matches a filename, assume that it came from SetFilename().
-    // Don't return it if we are not supposed to convert filename to URL.
-    NSArray* paths =
-        [pasteboard_->get() propertyListForType:NSFilenamesPboardType];
-    NSString* url_path = [[ns_url path] stringByStandardizingPath];
-    for (NSString* path in paths) {
-      if ([[path stringByStandardizingPath] isEqualToString:url_path])
-        return false;
-    }
-  }
-
-  *url = GURL([[ns_url absoluteString] UTF8String]);
-  *title = base::SysNSStringToUTF16(
-      [pasteboard_->get() stringForType:kCorePasteboardFlavorType_urln]);
-  return true;
+  return PopulateURLAndTitleFromPasteboard(

[erikchen, 2016/05/04 18:56:21]

Is it possible to use methods from ui/base/clipboard/clipboard_util_mac.mm
instead? 

[spqchan, 2016/05/05 17:41:05]

Done

+      url, title, pasteboard_->get(),
+      policy == OSExchangeData::DO_NOT_CONVERT_FILENAMES);
 }
 
 bool OSExchangeDataProviderMac::GetFilename(base::FilePath* path) const {
@@ -151,8 +130,7 @@ bool OSExchangeDataProviderMac::GetPickledData(
 }
 
 bool OSExchangeDataProviderMac::HasString() const {

[erikchen, 2016/05/04 18:56:21]

Hm. I expect HasString() to mirror GetString(). Any possibility we can remove
the URL processing part of GetString() and still have everything work?

[spqchan, 2016/05/05 17:41:05]

Discussed offline: it needs to be there. Made a change so that HasString() will
take the URL processing part in account.

-  NSArray* classes = @[ [NSString class] ];
-  return [pasteboard_->get() canReadObjectForClasses:classes options:nil];
+  return [[pasteboard_->get() types] containsObject:NSPasteboardTypeString];
 }
 
 bool OSExchangeDataProviderMac::HasURL(