Makes multiple calls to OSExchangeData::SetData with the same data

replace existing data.

BUG=none
TEST=none
R=tony@chromium.org

Review URL: http://codereview.chromium.org/7276044

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@90924 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 77 insertions, 1 deletions

diff --git a/ui/base/dragdrop/os_exchange_data_provider_win.cc b/ui/base/dragdrop/os_exchange_data_provider_win.cc
index 3a4b407..3809628 100644
--- a/ui/base/dragdrop/os_exchange_data_provider_win.cc
+++ b/ui/base/dragdrop/os_exchange_data_provider_win.cc
@@ -574,6 +574,23 @@ void DataObjectImpl::StopDownloads() {
   }
 }
 
+void DataObjectImpl::RemoveData(const FORMATETC& format) {
+  if (format.ptd)
+    return;  // Don't attempt to compare target devices.
+
+  for (StoredData::iterator i = contents_.begin(); i != contents_.end(); ++i) {
+    if (!(*i)->format_etc.ptd &&
+        format.cfFormat == (*i)->format_etc.cfFormat &&
+        format.dwAspect == (*i)->format_etc.dwAspect &&
+        format.lindex == (*i)->format_etc.lindex &&
+        format.tymed == (*i)->format_etc.tymed) {
+      delete *i;
+      contents_.erase(i);
+      return;
+    }
+  }
+}
+
 void DataObjectImpl::OnDownloadCompleted(const FilePath& file_path) {
   CLIPFORMAT hdrop_format = ClipboardUtil::GetCFHDropFormat()->cfFormat;
   DataObjectImpl::StoredData::iterator iter = contents_.begin();
@@ -691,6 +708,8 @@ HRESULT DataObjectImpl::GetCanonicalFormatEtc(
 
 HRESULT DataObjectImpl::SetData(
     FORMATETC* format_etc, STGMEDIUM* medium, BOOL should_release) {
+  RemoveData(*format_etc);
+
   STGMEDIUM* local_medium = new STGMEDIUM;
   if (should_release) {
     *local_medium = *medium;
@@ -702,7 +721,8 @@ HRESULT DataObjectImpl::SetData(
       new DataObjectImpl::StoredDataInfo(format_etc->cfFormat, local_medium);
   info->medium->tymed = format_etc->tymed;
   info->owns_medium = !!should_release;
-  contents_.push_back(info);
+  // Make newly added data appear first.
+  contents_.insert(contents_.begin(), info);
 
   return S_OK;
 }
diff --git a/ui/base/dragdrop/os_exchange_data_provider_win.h b/ui/base/dragdrop/os_exchange_data_provider_win.h
index e1cf869..ba92197 100644
--- a/ui/base/dragdrop/os_exchange_data_provider_win.h
+++ b/ui/base/dragdrop/os_exchange_data_provider_win.h
@@ -32,6 +32,9 @@ class DataObjectImpl : public DownloadFileObserver,
   // Accessors.
   void set_observer(Observer* observer) { observer_ = observer; }
 
+  // Number of known formats.
+  size_t size() const { return contents_.size(); }
+
   // DownloadFileObserver implementation:
   virtual void OnDownloadCompleted(const FilePath& file_path);
   virtual void OnDownloadAborted();
@@ -73,6 +76,9 @@ class DataObjectImpl : public DownloadFileObserver,
 
   void StopDownloads();
 
+  // Removes from contents_ the first data that matches |format|.
+  void RemoveData(const FORMATETC& format);
+
   // Our internal representation of stored data & type info.
   struct StoredDataInfo {
     FORMATETC format_etc;
diff --git a/ui/base/dragdrop/os_exchange_data_win_unittest.cc b/ui/base/dragdrop/os_exchange_data_win_unittest.cc
index 0de6841..42b5daf 100644
--- a/ui/base/dragdrop/os_exchange_data_win_unittest.cc
+++ b/ui/base/dragdrop/os_exchange_data_win_unittest.cc
@@ -98,6 +98,56 @@ TEST(OSExchangeDataTest, StringDataWritingViaCOM) {
   EXPECT_EQ(reference_url.spec(), url_from_data.spec());
 }
 
+// Verifies SetData invoked twice with the same data clobbers existing data.
+TEST(OSExchangeDataTest, RemoveData) {
+  OSExchangeData data;
+  std::wstring input = L"http://www.google.com/";
+  std::wstring input2 = L"http://www.google2.com/";
+
+  base::win::ScopedComPtr<IDataObject> com_data(
+      OSExchangeDataProviderWin::GetIDataObject(data));
+
+  // Store data in the object using the COM SetData API.
+  CLIPFORMAT cfstr_ineturl = RegisterClipboardFormat(CFSTR_INETURL);
+  FORMATETC format_etc =
+      { cfstr_ineturl, NULL, DVASPECT_CONTENT, -1, TYMED_HGLOBAL };
+  STGMEDIUM medium;
+  medium.tymed = TYMED_HGLOBAL;
+  {
+    HGLOBAL glob = GlobalAlloc(GPTR, sizeof(wchar_t) * (input.size() + 1));
+    size_t stringsz = input.size();
+    SIZE_T sz = GlobalSize(glob);
+    base::win::ScopedHGlobal<wchar_t> global_lock(glob);
+    wchar_t* buffer_handle = global_lock.get();
+    wcscpy_s(buffer_handle, input.size() + 1, input.c_str());
+    medium.hGlobal = glob;
+    medium.pUnkForRelease = NULL;
+    EXPECT_EQ(S_OK, com_data->SetData(&format_etc, &medium, TRUE));
+  }
+  // This should clobber the existing data.
+  {
+    HGLOBAL glob = GlobalAlloc(GPTR, sizeof(wchar_t) * (input2.size() + 1));
+    size_t stringsz = input2.size();
+    SIZE_T sz = GlobalSize(glob);
+    base::win::ScopedHGlobal<wchar_t> global_lock(glob);
+    wchar_t* buffer_handle = global_lock.get();
+    wcscpy_s(buffer_handle, input2.size() + 1, input2.c_str());
+    medium.hGlobal = glob;
+    medium.pUnkForRelease = NULL;
+    EXPECT_EQ(S_OK, com_data->SetData(&format_etc, &medium, TRUE));
+  }
+  EXPECT_EQ(1u, static_cast<DataObjectImpl*>(com_data.get())->size());
+
+  // Construct a new object with the old object so that we can use our access
+  // APIs.
+  OSExchangeData data2(CloneProvider(data));
+  EXPECT_TRUE(data2.HasURL());
+  GURL url_from_data;
+  std::wstring title;
+  EXPECT_TRUE(data2.GetURLAndTitle(&url_from_data, &title));
+  EXPECT_EQ(GURL(input2).spec(), url_from_data.spec());
+}
+
 TEST(OSExchangeDataTest, URLDataAccessViaCOM) {
   OSExchangeData data;
   GURL url("http://www.google.com/");