Drag and dropping text, parsable as url

ui/base/dragdrop/os_exchange_data_provider_win.cc

Index: ui/base/dragdrop/os_exchange_data_provider_win.cc
diff --git a/ui/base/dragdrop/os_exchange_data_provider_win.cc b/ui/base/dragdrop/os_exchange_data_provider_win.cc
index c09780fdf9e2ca5eceb72e40132bba5a00255096..f61804b045d6eb39dbd3b7c7675f4f0158ab979d 100644
--- a/ui/base/dragdrop/os_exchange_data_provider_win.cc
+++ b/ui/base/dragdrop/os_exchange_data_provider_win.cc
@@ -222,28 +222,6 @@ FormatEtcEnumerator* FormatEtcEnumerator::CloneFromOther(
 // OSExchangeDataProviderWin, public:
 
 // static
-bool OSExchangeDataProviderWin::HasPlainTextURL(IDataObject* source) {
-  base::string16 plain_text;
-  return (ClipboardUtil::GetPlainText(source, &plain_text) &&
-          !plain_text.empty() && GURL(plain_text).is_valid());
-}
-
-// static
-bool OSExchangeDataProviderWin::GetPlainTextURL(IDataObject* source,
-                                                GURL* url) {
-  base::string16 plain_text;
-  if (ClipboardUtil::GetPlainText(source, &plain_text) &&
-      !plain_text.empty()) {
-    GURL gurl(plain_text);
-    if (gurl.is_valid()) {
-      *url = gurl;
-      return true;
-    }
-  }
-  return false;
-}
-
-// static
 DataObjectImpl* OSExchangeDataProviderWin::GetDataObjectImpl(
     const OSExchangeData& data) {
   return static_cast<const OSExchangeDataProviderWin*>(&data.provider())->
@@ -413,21 +391,8 @@ bool OSExchangeDataProviderWin::GetURLAndTitle(
     OSExchangeData::FilenameToURLPolicy policy,
     GURL* url,
     base::string16* title) const {
-  base::string16 url_str;
-  bool success = ClipboardUtil::GetUrl(
-      source_object_.get(), url, title,
-      policy == OSExchangeData::CONVERT_FILENAMES ? true : false);
-  if (success) {
-    DCHECK(url->is_valid());

[dyaroshev, 2016/09/14 17:12:06]

Am I correct that os_exchange_data GetURLAndTitle can not return invalid url?
Why do most of the callers check? Should they not?

[Peter Kasting, 2016/09/15 21:26:53]

You mean in the existing code?  It looks to me like if GetUrl() fails, perhaps
GetPlainTextURL() can succeed but with an invalid URL?  In that case, this can
return an invalid URL.  Unless the code below lies -- if GetPlainTextURL() can't
actually return true unless url->is_valid(), then you'd be right, we can't
return an invalid URL.
Similarly with my reply to one of your other questions, I think if we're asking
for a URL and the function indicates whether it succeeded, that indication
should only be "true" when the URL in question is valid.  So I think callers
should not have to check this, ideally.

-    return true;
-  } else if (GetPlainTextURL(source_object_.get(), url)) {
-    if (url->is_valid())
-      *title = net::GetSuggestedFilename(*url, "", "", "", "", std::string());
-    else
-      title->clear();
-    return true;
-  }
-  return false;
+  return ClipboardUtil::GetUrl(source_object_.get(), url, title,
+                               policy == OSExchangeData::CONVERT_FILENAMES);
 }
 
 bool OSExchangeDataProviderWin::GetFilename(base::FilePath* path) const {
@@ -497,10 +462,8 @@ bool OSExchangeDataProviderWin::HasString() const {
 
 bool OSExchangeDataProviderWin::HasURL(
     OSExchangeData::FilenameToURLPolicy policy) const {
-  return (ClipboardUtil::HasUrl(
-              source_object_.get(),
-              policy == OSExchangeData::CONVERT_FILENAMES ? true : false) ||
-          HasPlainTextURL(source_object_.get()));
+  return ClipboardUtil::HasUrl(source_object_.get(),
+                               policy == OSExchangeData::CONVERT_FILENAMES);
 }
 
 bool OSExchangeDataProviderWin::HasFile() const {