MacViews: Get OSExchangeDataTest to pass

MacViews: Get OSExchangeDataTest to pass.

Add filenames support. DO_NOT_CONVERT_FILENAMES policy is not
supported because writing NSFilenamesPboardType to NSPasteboard always
populates the URL types.

Modify SetURL so that it doesn't overwrite previous SetString.

The following tests now passes:
OSExchangeDataTest.URLAndString
OSExchangeDataTest.TestFileToURLConversion

BUG=378134
Committed: https://crrev.com/31c81b78e5c6a0142e1755721574e2f399aca9ea
Cr-Commit-Position: refs/heads/master@{#296444}

[Andre, 2014-09-05 22:28:33]

andresantoso@chromium.org changed reviewers:
+ dcheng@chromium.org

[Andre, 2014-09-05 22:28:33]

dcheng PTAL.

[dcheng, 2014-09-10 00:22:54]

Sorry for the delay on the review.

https://codereview.chromium.org/542203004/diff/1/ui/base/dragdrop/os_exchange...
File ui/base/dragdrop/os_exchange_data_unittest.cc (right):

https://codereview.chromium.org/542203004/diff/1/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_unittest.cc:108: // NSPasteboard does not
support not converting, writing NSFilenamesPboardType
We need to manually clear it in the code that reads it. This property is
important to prevent leakage of file paths to normal web content.

[dcheng, 2014-09-10 00:22:56]

Sorry for the delay on the review.

[Andre, 2014-09-13 01:10:50]

https://codereview.chromium.org/542203004/diff/1/ui/base/dragdrop/os_exchange...
File ui/base/dragdrop/os_exchange_data_unittest.cc (right):

https://codereview.chromium.org/542203004/diff/1/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_unittest.cc:108: // NSPasteboard does not
support not converting, writing NSFilenamesPboardType
On 2014/09/10 00:22:54, dcheng (OOO) wrote:
> We need to manually clear it in the code that reads it. This property is
> important to prevent leakage of file paths to normal web content.

I couldn't find a way to clear the URL from the pasteboard.
I've implemented a different scheme in patchset #2, PTAL.

[dcheng, 2014-09-15 21:38:47]

https://codereview.chromium.org/542203004/diff/20001/ui/base/dragdrop/os_exch...
File ui/base/dragdrop/os_exchange_data_provider_mac.mm (right):

https://codereview.chromium.org/542203004/diff/20001/ui/base/dragdrop/os_exch...
ui/base/dragdrop/os_exchange_data_provider_mac.mm:97: if ([paths count] > 0 &&
[[paths objectAtIndex:0] isEqual:[ns_url path]])
Is there some guarantee that the URLs and filenames are populated in 'parallel'
order?

[Andre, 2014-09-16 18:00:08]

https://codereview.chromium.org/542203004/diff/20001/ui/base/dragdrop/os_exch...
File ui/base/dragdrop/os_exchange_data_provider_mac.mm (right):

https://codereview.chromium.org/542203004/diff/20001/ui/base/dragdrop/os_exch...
ui/base/dragdrop/os_exchange_data_provider_mac.mm:97: if ([paths count] > 0 &&
[[paths objectAtIndex:0] isEqual:[ns_url path]])
On 2014/09/15 21:38:47, dcheng (OOO) wrote:
> Is there some guarantee that the URLs and filenames are populated in
'parallel'
> order?

Maybe not.
Changed it to use containsObject.

[dcheng, 2014-09-22 20:25:17]

Sorry for the delay on this, as I was OOO. If I don't reply by tomorrow, please
ping me again. Again, sorry for the delay!

[dcheng, 2014-09-23 17:49:10]

https://codereview.chromium.org/542203004/diff/40001/ui/base/dragdrop/os_exch...
File ui/base/dragdrop/os_exchange_data_provider_mac.mm (right):

https://codereview.chromium.org/542203004/diff/40001/ui/base/dragdrop/os_exch...
ui/base/dragdrop/os_exchange_data_provider_mac.mm:97: if ([paths
containsObject:[ns_url path]])
Do we need to try to normalize/canonicalize the paths before comparing them
here? What if we have file:///Users/foo/x.html as the URL and /Users/foo//x.html
as the filename? Is there some canonicalization that's guaranteed to happen
elsewhere?

[Andre, 2014-09-24 01:12:40]

https://codereview.chromium.org/542203004/diff/40001/ui/base/dragdrop/os_exch...
File ui/base/dragdrop/os_exchange_data_provider_mac.mm (right):

https://codereview.chromium.org/542203004/diff/40001/ui/base/dragdrop/os_exch...
ui/base/dragdrop/os_exchange_data_provider_mac.mm:97: if ([paths
containsObject:[ns_url path]])
On 2014/09/23 17:49:10, dcheng wrote:
> Do we need to try to normalize/canonicalize the paths before comparing them
> here? What if we have file:///Users/foo/x.html as the URL and
/Users/foo//x.html
> as the filename? Is there some canonicalization that's guaranteed to happen
> elsewhere?

Done.
Added calls to stringByStandardizingPath before comparing.

[dcheng, 2014-09-24 02:36:34]

https://codereview.chromium.org/542203004/diff/60001/ui/base/dragdrop/os_exch...
File ui/base/dragdrop/os_exchange_data_provider_mac.mm (right):

https://codereview.chromium.org/542203004/diff/60001/ui/base/dragdrop/os_exch...
ui/base/dragdrop/os_exchange_data_provider_mac.mm:94: // If the URL matches the
filename, assume that it came from SetFilename().
s/the/a/

https://codereview.chromium.org/542203004/diff/60001/ui/base/dragdrop/os_exch...
ui/base/dragdrop/os_exchange_data_provider_mac.mm:104: if (url) {
From OSExchangeDataProviderWin, I don't think either of these parameters is
optional. Just DCHECK they're not NULL at the top.

[Andre, 2014-09-24 03:35:58]

https://codereview.chromium.org/542203004/diff/60001/ui/base/dragdrop/os_exch...
File ui/base/dragdrop/os_exchange_data_provider_mac.mm (right):

https://codereview.chromium.org/542203004/diff/60001/ui/base/dragdrop/os_exch...
ui/base/dragdrop/os_exchange_data_provider_mac.mm:94: // If the URL matches the
filename, assume that it came from SetFilename().
On 2014/09/24 02:36:34, dcheng wrote:
> s/the/a/

Done.

https://codereview.chromium.org/542203004/diff/60001/ui/base/dragdrop/os_exch...
ui/base/dragdrop/os_exchange_data_provider_mac.mm:104: if (url) {
On 2014/09/24 02:36:34, dcheng wrote:
> From OSExchangeDataProviderWin, I don't think either of these parameters is
> optional. Just DCHECK they're not NULL at the top.

Done.

[dcheng, 2014-09-24 03:45:39]

lgtm

[Andre, 2014-09-24 04:43:00]

andresantoso@chromium.org changed reviewers:
+ sky@chromium.org

[Andre, 2014-09-24 04:43:00]

sky, please review.

[sky, 2014-09-24 14:37:01]

LGTM

[Andre, 2014-09-24 16:30:28]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2014-09-24 16:30:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/542203004/80001

[commit-bot: I haz the power, 2014-09-24 17:01:58]

Committed patchset #5 (id:80001) as f45976f731f5aff594f437e2b2fb6590dece8e94

[commit-bot: I haz the power, 2014-09-24 17:02:38]

Patchset 5 (id:??) landed as
https://crrev.com/31c81b78e5c6a0142e1755721574e2f399aca9ea
Cr-Commit-Position: refs/heads/master@{#296444}