[MultiPaste] Separate clipboard history owned by different users

This CL utilizes the mapping between account ids and history clipboard
data to handle the multi-user environment.

A browser test is created to protect this feature.

Bug: 1106857
Change-Id: I76d4d7ee2dc5b9525e4173586df168266a7fbb59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310848
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791366}

ash/clipboard/clipboard_history.cc

@@ -5,10 +5,12 @@
 #include "ash/clipboard/clipboard_history.h"
 
 #include "ash/clipboard/clipboard_history_controller.h"
+#include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/account_id/account_id.h"
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/clipboard/clipboard_buffer.h"
 #include "ui/base/clipboard/clipboard_constants.h"
@@ -21,6 +23,10 @@ namespace {
 
 constexpr size_t kMaxClipboardItemsShared = 5;
 
+AccountId GetActiveAccountId() {
+  return Shell::Get()->session_controller()->GetActiveAccountId();
+}
+
 }  // namespace
 
 ClipboardHistory::ScopedPause::ScopedPause(ClipboardHistory* clipboard_history)
@@ -34,6 +40,8 @@ ClipboardHistory::ScopedPause::~ScopedPause() {
 
 ClipboardHistory::ClipboardHistory() {
   ui::ClipboardMonitor::GetInstance()->AddObserver(this);
+  Shell::Get()->session_controller()->AddObserver(this);
+  Shell::Get()->AddShellObserver(this);
 }
 
 ClipboardHistory::~ClipboardHistory() {
@@ -41,14 +49,17 @@ ClipboardHistory::~ClipboardHistory() {
 }
 
 void ClipboardHistory::ClearHistory() {
-  history_with_duplicates_ = std::deque<ui::ClipboardData>();
+  history_with_duplicates_mappings_[GetActiveAccountId()] =
+      std::deque<ui::ClipboardData>();
 }
 
 std::vector<ui::ClipboardData>
 ClipboardHistory::GetRecentClipboardDataWithNoDuplicates() const {
   std::vector<ui::ClipboardData> history_without_duplicates;
-  for (auto it = history_with_duplicates_.crbegin();
-       it != history_with_duplicates_.crend(); ++it) {
+  const std::deque<ui::ClipboardData>& history_with_duplicates =
+      history_with_duplicates_mappings_.find(GetActiveAccountId())->second;
+  for (auto it = history_with_duplicates.crbegin();
+       it != history_with_duplicates.crend(); ++it) {
     if (history_without_duplicates.size() == kMaxClipboardItemsShared)
       break;
     const ui::ClipboardData& current_data = *it;
@@ -59,7 +70,9 @@ ClipboardHistory::GetRecentClipboardDataWithNoDuplicates() const {
 }
 
 bool ClipboardHistory::IsEmpty() const {
-  return history_with_duplicates_.empty();
+  auto it = history_with_duplicates_mappings_.find(GetActiveAccountId());
+  DCHECK(it != history_with_duplicates_mappings_.cend());
+  return it->second.empty();
 }
 
 void ClipboardHistory::OnClipboardDataChanged() {
@@ -108,27 +121,43 @@ void ClipboardHistory::OnClipboardDataChanged() {
     clipboard->ReadImage(
         ui::ClipboardBuffer::kCopyPaste, /*data_dst=*/nullptr,
         base::BindOnce(&ClipboardHistory::OnRecievePNGFromClipboard,
-                       weak_ptr_factory_.GetWeakPtr(), std::move(new_data)));
+                       weak_ptr_factory_.GetWeakPtr(), GetActiveAccountId(),
+                       std::move(new_data)));
     return;
   }
-  CommitData(std::move(new_data));
+  CommitData(GetActiveAccountId(), std::move(new_data));
 }
 
-void ClipboardHistory::CommitData(ui::ClipboardData data) {
+void ClipboardHistory::CommitData(const AccountId& account_id,
+                                  ui::ClipboardData data) {
+  const AccountId active_account_id = GetActiveAccountId();
+
+  // CommitData() is called asynchronously when handling bitmap data.
+  // Theoretically the active account may switch before CommitData() is handled.
+  // Return early in this edge case.
+  if (account_id != active_account_id)
+    return;
+
+  DCHECK(active_account_id.is_valid());
+
+  std::deque<ui::ClipboardData>& history_with_duplicates =
+      history_with_duplicates_mappings_[active_account_id];
+
   // Keep duplicates to maintain most recent ordering.
-  history_with_duplicates_.push_back(std::move(data));
+  history_with_duplicates.push_back(std::move(data));
 
   // Keep the history with duplicates to a reasonable limit, but greater than
   // kMaxClipboardItemsShared, so that repeated copies to not push off relevant
   // data.
-  if (history_with_duplicates_.size() > kMaxClipboardItemsShared * 2)
-    history_with_duplicates_.pop_front();
+  if (history_with_duplicates.size() > kMaxClipboardItemsShared * 2)
+    history_with_duplicates.pop_front();
 }
 
-void ClipboardHistory::OnRecievePNGFromClipboard(ui::ClipboardData data,
+void ClipboardHistory::OnRecievePNGFromClipboard(const AccountId& account_id,
+                                                 ui::ClipboardData data,
                                                  const SkBitmap& bitmap) {
   data.SetBitmapData(bitmap);
-  CommitData(std::move(data));
+  CommitData(account_id, std::move(data));
 }
 
 void ClipboardHistory::PauseClipboardHistory() {
@@ -139,4 +168,19 @@ void ClipboardHistory::UnPauseClipboardHistory() {
   --num_pause_clipboard_history_;
 }
 
-}  // namespace ash
\ No newline at end of file
+void ClipboardHistory::OnActiveUserSessionChanged(
+    const AccountId& active_account_id) {
+  if (!base::Contains(history_with_duplicates_mappings_, active_account_id)) {
+    history_with_duplicates_mappings_[active_account_id] =
+        std::deque<ui::ClipboardData>();
+  }
+}
+
+void ClipboardHistory::OnShellDestroying() {
+  // ClipboardHistory depends on Shell to access the classes it observes. So
+  // remove itself from observer lists before the Shell instance is destroyed.
+  Shell::Get()->session_controller()->RemoveObserver(this);
+  Shell::Get()->RemoveShellObserver(this);
+}
+
+}  // namespace ash

ash/clipboard/clipboard_history.h

@@ -6,23 +6,32 @@
 #define ASH_CLIPBOARD_CLIPBOARD_HISTORY_H_
 
 #include <deque>
+#include <map>
 #include <vector>
 
 #include "ash/ash_export.h"
+#include "ash/public/cpp/session/session_observer.h"
+#include "ash/shell_observer.h"
 #include "base/component_export.h"
 #include "base/memory/weak_ptr.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/clipboard/clipboard_data.h"
 #include "ui/base/clipboard/clipboard_observer.h"
 
+class AccountId;
+
 namespace ui {
 class ClipboardData;
 }
 
 namespace ash {
 
-// Keeps track of the last few things saved in the clipboard.
-class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver {
+// Keeps track of the last few things saved in the clipboard. For multiprofile,
+// one account's clipboard history is separated from others and indexed by the
+// account id.
+class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver,
+                                    public SessionObserver,
+                                    public ShellObserver {
  public:
   // Prevents clipboard history from recording history within its scope. If
   // anything is copied within its scope, history will not be recorded.
@@ -42,36 +51,50 @@ class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver {
   ClipboardHistory& operator=(const ClipboardHistory&) = delete;
   ~ClipboardHistory() override;
 
-  // Deletes content from |history_with_duplicates_|. Does not modify content
+  // Deletes clipboard history of the active account. Does not modify content
   // stored in the clipboard.
   void ClearHistory();
 
+  // Returns the recent unique clipboard data of the active account.
   std::vector<ui::ClipboardData> GetRecentClipboardDataWithNoDuplicates() const;
 
-  // Whether there is no history recorded.
+  // Returns whether the clipboard history of the active account is empty.
   bool IsEmpty() const;
 
   // ClipboardMonitor:
   void OnClipboardDataChanged() override;
 
  private:
-  // Adds |data| to |history_with_duplicates|.
-  void CommitData(ui::ClipboardData data);
-  // Callback to read a bitmap from the Cliboard.
-  void OnRecievePNGFromClipboard(ui::ClipboardData data,
+  // Adds |data| to the clipboard history belonging to the account indicated
+  // by |account_id|.
+  void CommitData(const AccountId& account_id, ui::ClipboardData data);
+
+  // Callback to read a bitmap from the clipboard data belonging to the account
+  // indicated by |active_account_id|.
+  void OnRecievePNGFromClipboard(const AccountId& active_account_id,
+                                 ui::ClipboardData data,
                                  const SkBitmap& bitmap);
   void PauseClipboardHistory();
   void UnPauseClipboardHistory();
 
+  // SessionObserver:
+  void OnActiveUserSessionChanged(const AccountId& account_id) override;
+
+  // ShellObserver:
+  void OnShellDestroying() override;
+
   // The count of clipboard history pauses.
   size_t num_pause_clipboard_history_ = 0;
-  // History of the most recent things copied. Duplicates are kept to keep the
-  // most recent ordering.
-  std::deque<ui::ClipboardData> history_with_duplicates_;
+
+  // Clipboard history is mapped by account ID to store different histories per
+  // account when multiprofile is used. Duplicates are kept to maintain the most
+  // recent ordering.
+  std::map<AccountId, std::deque<ui::ClipboardData>>
+      history_with_duplicates_mappings_;
 
   base::WeakPtrFactory<ClipboardHistory> weak_ptr_factory_{this};
 };
 
 }  // namespace ash
 
-#endif  // ASH_CLIPBOARD_CLIPBOARD_HISTORY_H_
\ No newline at end of file
+#endif  // ASH_CLIPBOARD_CLIPBOARD_HISTORY_H_

ash/clipboard/clipboard_history_controller.cc

@@ -251,4 +251,4 @@ void ClipboardHistoryController::MenuOptionSelected(int index) {
       base::TimeDelta::FromMilliseconds(100));
 }
 
-}  // namespace ash
\ No newline at end of file
+}  // namespace ash

ash/clipboard/clipboard_history_controller.h

@@ -60,4 +60,4 @@ class ClipboardHistoryController {
 
 }  // namespace ash
 
-#endif  // ASH_CLIPBOARD_CLIPBOARD_HISTORY_CONTROLLER_H_
\ No newline at end of file
+#endif  // ASH_CLIPBOARD_CLIPBOARD_HISTORY_CONTROLLER_H_

ash/clipboard/clipboard_history_unittest.cc

@@ -207,4 +207,4 @@ TEST_F(ClipboardHistoryTest, DuplicateBitmap) {
   WriteAndEnsureBitmapHistory(input_bitmaps, expected_bitmaps);
 }
 
-}  // namespace ash
\ No newline at end of file
+}  // namespace ash

chrome/browser/ui/ash/clipboard_history_browsertest.cc

+// Copyright 2020 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 "ash/clipboard/clipboard_history.h"
+#include "ash/clipboard/clipboard_history_controller.h"
+#include "ash/shell.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/chromeos/login/login_manager_test.h"
+#include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
+#include "chrome/browser/chromeos/login/ui/user_adding_screen.h"
+#include "chromeos/constants/chromeos_features.h"
+#include "components/user_manager/user_manager.h"
+#include "content/public/test/browser_test.h"
+#include "ui/base/clipboard/scoped_clipboard_writer.h"
+
+// Verify clipboard history's features in the multiprofile environment.
+class ClipboardHistoryWithMultiProfileBrowserTest
+    : public chromeos::LoginManagerTest {
+ public:
+  ClipboardHistoryWithMultiProfileBrowserTest() : LoginManagerTest() {
+    login_mixin_.AppendRegularUsers(2);
+    account_id1_ = login_mixin_.users()[0].account_id;
+    account_id2_ = login_mixin_.users()[1].account_id;
+
+    feature_list_.InitAndEnableFeature(chromeos::features::kClipboardHistory);
+  }
+
+  ~ClipboardHistoryWithMultiProfileBrowserTest() override = default;
+
+ protected:
+  void SetClipboardText(const std::string& text) {
+    ui::ScopedClipboardWriter(ui::ClipboardBuffer::kCopyPaste)
+        .WriteText(base::ASCIIToUTF16(text));
+  }
+
+  std::vector<ui::ClipboardData> GetClipboardData() const {
+    return ash::Shell::Get()
+        ->clipboard_history_controller()
+        ->clipboard_history()
+        ->GetRecentClipboardDataWithNoDuplicates();
+  }
+
+  AccountId account_id1_;
+  AccountId account_id2_;
+  chromeos::LoginManagerMixin login_mixin_{&mixin_host_};
+
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Verify that the clipboard data history belonging to different users does not
+// interfere with each other.
+IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
+                       DisconflictInMultiUser) {
+  LoginUser(account_id1_);
+  EXPECT_TRUE(GetClipboardData().empty());
+
+  // Store text when the user1 is active.
+  const std::string copypaste_data1("user1_text1");
+  SetClipboardText(copypaste_data1);
+
+  {
+    const std::vector<ui::ClipboardData> data = GetClipboardData();
+    EXPECT_EQ(1u, data.size());
+    EXPECT_EQ(copypaste_data1, data[0].text());
+  }
+
+  // Log in as the user2. The clipboard history should be empty.
+  chromeos::UserAddingScreen::Get()->Start();
+  AddUser(account_id2_);
+  EXPECT_TRUE(GetClipboardData().empty());
+
+  // Store text when the user2 is active.
+  const std::string copypaste_data2("user2_text1");
+  SetClipboardText(copypaste_data2);
+
+  {
+    const std::vector<ui::ClipboardData> data = GetClipboardData();
+    EXPECT_EQ(1u, data.size());
+    EXPECT_EQ(copypaste_data2, data[0].text());
+  }
+
+  // Switch to the user1.
+  user_manager::UserManager::Get()->SwitchActiveUser(account_id1_);
+
+  // Store text when the user1 is active.
+  const std::string copypaste_data3("user1_text2");
+  SetClipboardText(copypaste_data3);
+
+  {
+    const std::vector<ui::ClipboardData> data = GetClipboardData();
+    EXPECT_EQ(2u, data.size());
+
+    // Note that items in |data| follow the time ordering. The most recent item
+    // is always the first one.
+    EXPECT_EQ(copypaste_data3, data[0].text());
+    EXPECT_EQ(copypaste_data1, data[1].text());
+  }
+}

chrome/test/BUILD.gn

@@ -2484,6 +2484,7 @@ if (!is_android) {
         "../browser/ui/ash/back_gesture_browsertest.cc",
         "../browser/ui/ash/chrome_new_window_client_browsertest.cc",
         "../browser/ui/ash/chrome_screenshot_grabber_browsertest.cc",
+        "../browser/ui/ash/clipboard_history_browsertest.cc",
         "../browser/ui/ash/keyboard/keyboard_controller_browsertest.cc",
         "../browser/ui/ash/keyboard/keyboard_end_to_end_browsertest.cc",
         "../browser/ui/ash/launcher/app_service/app_service_app_window_browsertest.cc",