Multipaste: Prevent duplicate copies of identical data in clipboard history

This change stops duplicate items from being added to the clipboard
history. When an item is copied, if an identical item already exists in
the clipboard history then the existing item will be moved to the front
of the list. (Note: items with some of the same data but copied from a
different location are considered different because they will have a
non-equal DataTransferEndpoint)

Bug: 1131730
Change-Id: I97c8655363ecb34c5ab01eb0323b3e2792ee532b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2578123
Commit-Queue: Matthew Mourgos <mmourgos@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835926}

ash/clipboard/clipboard_history.cc

@@ -110,9 +110,21 @@ void ClipboardHistory::MaybeCommitData(ui::ClipboardData data) {
   if (!ClipboardHistoryUtil::IsSupported(data))
     return;
 
-  history_list_.emplace_front(std::move(data));
+  auto iter =
+      std::find_if(history_list_.cbegin(), history_list_.cend(),
+                   [&data](const auto& item) { return item.data() == data; });
+  bool is_duplicate = iter != history_list_.cend();
+  if (is_duplicate) {
+    // If |data| already exists in |history_list_| then move it to the front
+    // instead of creating a new one because creating a new one will result in a
+    // new unique identifier.
+    history_list_.splice(history_list_.begin(), history_list_, iter);
+  } else {
+    history_list_.emplace_front(std::move(data));
+  }
+
   for (auto& observer : observers_)
-    observer.OnClipboardHistoryItemAdded(history_list_.front());
+    observer.OnClipboardHistoryItemAdded(history_list_.front(), is_duplicate);
 
   if (history_list_.size() > ClipboardHistoryUtil::kMaxClipboardItemsShared) {
     auto removed = std::move(history_list_.back());

ash/clipboard/clipboard_history.h

@@ -25,8 +25,8 @@ class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver {
   class ASH_EXPORT Observer : public base::CheckedObserver {
    public:
     // Called when a ClipboardHistoryItem has been added.
-    virtual void OnClipboardHistoryItemAdded(const ClipboardHistoryItem& item) {
-    }
+    virtual void OnClipboardHistoryItemAdded(const ClipboardHistoryItem& item,
+                                             bool is_duplicate) {}
     // Called when a ClipboardHistoryItem has been removed.
     virtual void OnClipboardHistoryItemRemoved(
         const ClipboardHistoryItem& item) {}

ash/clipboard/clipboard_history_resource_manager.cc

@@ -229,7 +229,12 @@ void ClipboardHistoryResourceManager::CancelUnfinishedRequests() {
 }
 
 void ClipboardHistoryResourceManager::OnClipboardHistoryItemAdded(
-    const ClipboardHistoryItem& item) {
+    const ClipboardHistoryItem& item,
+    bool is_duplicate) {
+  // If this item is a duplicate then there is no new item to render.
+  if (is_duplicate)
+    return;
+
   // For items that will be represented by their rendered HTML, we need to do
   // some prep work to pre-render and cache an image model.
   if (ClipboardHistoryUtil::CalculateDisplayFormat(item.data()) !=

ash/clipboard/clipboard_history_resource_manager.h

@@ -76,7 +76,8 @@ class ASH_EXPORT ClipboardHistoryResourceManager
   void CancelUnfinishedRequests();
 
   // ClipboardHistory::Observer:
-  void OnClipboardHistoryItemAdded(const ClipboardHistoryItem& item) override;
+  void OnClipboardHistoryItemAdded(const ClipboardHistoryItem& item,
+                                   bool is_duplicate) override;
   void OnClipboardHistoryItemRemoved(const ClipboardHistoryItem& item) override;
   void OnClipboardHistoryCleared() override;
 

ash/clipboard/clipboard_history_resource_manager_unittest.cc

@@ -266,13 +266,19 @@ TEST_F(ClipboardHistoryResourceManagerTest, DuplicateHTML) {
   EXPECT_CALL(*mock_image_factory(), CancelRequest).Times(0);
   EXPECT_CALL(*mock_image_factory(), Render).Times(1);
 
-  for (int i = 0; i < 2; ++i) {
-    {
-      ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste);
-      scw.WriteHTML(base::UTF8ToUTF16("<img test>"), "source_url");
-    }
-    FlushMessageLoop();
+  // Use identical markup but differing source url so that both items are added
+  // to the clipboard history.
+  {
+    ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste);
+    scw.WriteHTML(base::UTF8ToUTF16("<img test>"), "source_url_1");
+  }
+  FlushMessageLoop();
+  {
+    ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste);
+    scw.WriteHTML(base::UTF8ToUTF16("<img test>"), "source_url_2");
   }
+  FlushMessageLoop();
+
   auto items = clipboard_history()->GetItems();
   EXPECT_EQ(2u, items.size());
   for (const auto& item : items)

ash/clipboard/clipboard_history_unittest.cc

@@ -154,11 +154,12 @@ TEST_F(ClipboardHistoryTest, OneThingCopiedOneThingSaved) {
   WriteAndEnsureTextHistory(input_strings, expected_strings);
 }
 
-// Tests that if the same (non bitmap) thing is copied, both things are saved.
+// Tests that if the same (non bitmap) thing is copied, only one of the
+// duplicates is in the list.
 TEST_F(ClipboardHistoryTest, DuplicateBasic) {
   std::vector<base::string16> input_strings{base::UTF8ToUTF16("test"),
                                             base::UTF8ToUTF16("test")};
-  std::vector<base::string16> expected_strings = input_strings;
+  std::vector<base::string16> expected_strings{base::UTF8ToUTF16("test")};
 
   // Test that both things are saved.
   WriteAndEnsureTextHistory(input_strings, expected_strings);
@@ -190,18 +191,19 @@ TEST_F(ClipboardHistoryTest, HistoryIsReverseChronological) {
   WriteAndEnsureTextHistory(input_strings, expected_strings);
 }
 
-// Tests that when a duplicate is copied, the duplicate shows up in the proper
-// order and that the older version is still returned.
+// Tests that when a duplicate is copied, the existing duplicate item moves up
+// to the front of the clipboard history.
 TEST_F(ClipboardHistoryTest, DuplicatePrecedesPreviousRecord) {
   // Input holds a unique string sandwiched by a copy.
   std::vector<base::string16> input_strings{
       base::UTF8ToUTF16("test1"), base::UTF8ToUTF16("test2"),
       base::UTF8ToUTF16("test1"), base::UTF8ToUTF16("test3")};
   // The result should be a reversal of the copied elements. When a duplicate
-  // is copied, history will show all versions of the recent duplicate.
-  std::vector<base::string16> expected_strings{
-      base::UTF8ToUTF16("test3"), base::UTF8ToUTF16("test1"),
-      base::UTF8ToUTF16("test2"), base::UTF8ToUTF16("test1")};
+  // is copied, history will have that item moved to the front instead of adding
+  // a new item.
+  std::vector<base::string16> expected_strings{base::UTF8ToUTF16("test3"),
+                                               base::UTF8ToUTF16("test1"),
+                                               base::UTF8ToUTF16("test2")};
 
   WriteAndEnsureTextHistory(input_strings, expected_strings);
 }
@@ -292,7 +294,8 @@ TEST_F(ClipboardHistoryTest, BasicBitmap) {
   WriteAndEnsureBitmapHistory(input_bitmaps, expected_bitmaps);
 }
 
-// Tests that duplicate bitmaps show up in history in most-recent order.
+// Tests that duplicate bitmaps show up in history as one item placed in
+// most-recent order.
 TEST_F(ClipboardHistoryTest, DuplicateBitmap) {
   SkBitmap test_bitmap_1;
   test_bitmap_1.allocN32Pixels(3, 2);
@@ -303,7 +306,7 @@ TEST_F(ClipboardHistoryTest, DuplicateBitmap) {
 
   std::vector<SkBitmap> input_bitmaps{test_bitmap_1, test_bitmap_2,
                                       test_bitmap_1};
-  std::vector<SkBitmap> expected_bitmaps = input_bitmaps;
+  std::vector<SkBitmap> expected_bitmaps{test_bitmap_1, test_bitmap_2};
   WriteAndEnsureBitmapHistory(input_bitmaps, expected_bitmaps);
 }
 

ash/clipboard/clipboard_nudge_controller.cc

@@ -89,7 +89,8 @@ void ClipboardNudgeController::RegisterProfilePrefs(
 }
 
 void ClipboardNudgeController::OnClipboardHistoryItemAdded(
-    const ClipboardHistoryItem& item) {
+    const ClipboardHistoryItem& item,
+    bool is_duplicate) {
   PrefService* prefs =
       Shell::Get()->session_controller()->GetLastActiveUserPrefService();
   if (!ShouldShowNudge(prefs))

ash/clipboard/clipboard_nudge_controller.h

@@ -50,7 +50,8 @@ class ASH_EXPORT ClipboardNudgeController
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   // ui::ClipboardHistory::Observer:
-  void OnClipboardHistoryItemAdded(const ClipboardHistoryItem& item) override;
+  void OnClipboardHistoryItemAdded(const ClipboardHistoryItem& item,
+                                   bool is_duplicate = false) override;
 
   // ui::ClipboardObserver:
   void OnClipboardDataRead() override;