Drag and dropping text, parsable as url

ui/base/dragdrop/os_exchange_data_provider_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/base/dragdrop/os_exchange_data_provider_win.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 
@@ skipping 204 matching lines @@
   }
   // Carry over
   e->cursor_ = other->cursor_;
   return e;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // 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())->
       data_.get();
 }
 
 // static
 IDataObject* OSExchangeDataProviderWin::GetIDataObject(
     const OSExchangeData& data) {
   return static_cast<const OSExchangeDataProviderWin*>(&data.provider())->
@@ skipping 149 matching lines @@
 }
 
 bool OSExchangeDataProviderWin::GetString(base::string16* data) const {
   return ClipboardUtil::GetPlainText(source_object_.get(), data);
 }
 
 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 {
   std::vector<base::string16> filenames;
   bool success = ClipboardUtil::GetFilenames(source_object_.get(), &filenames);
   if (success)
     *path = base::FilePath(filenames[0]);
   return success;
 }
 
@@ skipping 49 matching lines @@
     *base_url = GURL(url);
   return success;
 }
 
 bool OSExchangeDataProviderWin::HasString() const {
   return ClipboardUtil::HasPlainText(source_object_.get());
 }
 
 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 {
   return ClipboardUtil::HasFilenames(source_object_.get());
 }
 
 bool OSExchangeDataProviderWin::HasFileContents() const {
   return ClipboardUtil::HasFileContents(source_object_.get());
 }
 
@@ skipping 547 matching lines @@
             std::min(file_name.size(), static_cast<size_t>(MAX_PATH - 1u)));
 
   STGMEDIUM* storage = new STGMEDIUM;
   storage->tymed = TYMED_HGLOBAL;
   storage->hGlobal = hdata;
   storage->pUnkForRelease = NULL;
   return storage;
 }
 
 }  // namespace ui