Revert clipboard history separation for MultiProfile

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

ash/clipboard/clipboard_history.cc

@@ -5,10 +5,8 @@
 #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/stl_util.h"
-#include "components/account_id/account_id.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "ui/base/clipboard/clipboard_monitor.h"
 #include "ui/base/clipboard/clipboard_non_backed.h"
 
@@ -18,10 +16,6 @@ namespace {
 
 constexpr size_t kMaxClipboardItemsShared = 5;
 
-AccountId GetActiveAccountId() {
-  return Shell::Get()->session_controller()->GetActiveAccountId();
-}
-
 }  // namespace
 
 ClipboardHistory::ScopedPause::ScopedPause(ClipboardHistory* clipboard_history)
@@ -35,8 +29,6 @@ ClipboardHistory::ScopedPause::~ScopedPause() {
 
 ClipboardHistory::ClipboardHistory() {
   ui::ClipboardMonitor::GetInstance()->AddObserver(this);
-  Shell::Get()->session_controller()->AddObserver(this);
-  Shell::Get()->AddShellObserver(this);
 }
 
 ClipboardHistory::~ClipboardHistory() {
@@ -44,13 +36,11 @@ ClipboardHistory::~ClipboardHistory() {
 }
 
 const std::list<ui::ClipboardData>& ClipboardHistory::GetItems() const {
-  auto it = items_by_account_id_.find(GetActiveAccountId());
-  DCHECK(it != items_by_account_id_.cend());
-  return it->second;
+  return history_list_;
 }
 
 void ClipboardHistory::Clear() {
-  items_by_account_id_[GetActiveAccountId()] = std::list<ui::ClipboardData>();
+  history_list_ = std::list<ui::ClipboardData>();
 }
 
 bool ClipboardHistory::IsEmpty() const {
@@ -79,17 +69,15 @@ void ClipboardHistory::OnClipboardDataChanged() {
   // immediately by the long form URL.
   commit_data_weak_factory_.InvalidateWeakPtrs();
   base::SequencedTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(&ClipboardHistory::CommitData,
-                                commit_data_weak_factory_.GetWeakPtr(),
-                                GetActiveAccountId(), *clipboard_data));
+      FROM_HERE,
+      base::BindOnce(&ClipboardHistory::CommitData,
+                     commit_data_weak_factory_.GetWeakPtr(), *clipboard_data));
 }
 
-void ClipboardHistory::CommitData(const AccountId& account_id,
-                                  ui::ClipboardData data) {
-  std::list<ui::ClipboardData>& items = items_by_account_id_[account_id];
-  items.push_front(std::move(data));
-  if (items.size() > kMaxClipboardItemsShared)
-    items.pop_back();
+void ClipboardHistory::CommitData(ui::ClipboardData data) {
+  history_list_.push_front(std::move(data));
+  if (history_list_.size() > kMaxClipboardItemsShared)
+    history_list_.pop_back();
 }
 
 void ClipboardHistory::Pause() {
@@ -100,17 +88,4 @@ void ClipboardHistory::Resume() {
   --num_pause_;
 }
 
-void ClipboardHistory::OnActiveUserSessionChanged(
-    const AccountId& active_account_id) {
-  if (!base::Contains(items_by_account_id_, active_account_id))
-    items_by_account_id_[active_account_id] = std::list<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

@@ -9,27 +9,19 @@
 #include <map>
 
 #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 "ui/base/clipboard/clipboard_data.h"
 #include "ui/base/clipboard/clipboard_observer.h"
 
-class AccountId;
-
 namespace ui {
 class ClipboardData;
 }  // namespace ui
 
 namespace ash {
 
-// 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 {
+// Keeps track of the last few things saved in the clipboard.
+class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver {
  public:
   // Prevents clipboard history from being recorded within its scope. If
   // anything is copied within its scope, history will not be recorded.
@@ -49,12 +41,11 @@ class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver,
   ClipboardHistory& operator=(const ClipboardHistory&) = delete;
   ~ClipboardHistory() override;
 
-  // Returns the list of most recent items copied by the active account. The
-  // returned list is sorted by recency.
+  // Returns the list of most recent items. The returned list is sorted by
+  // recency.
   const std::list<ui::ClipboardData>& GetItems() const;
 
-  // Deletes clipboard history of the active account. Does not modify content
-  // stored in the clipboard.
+  // Deletes clipboard history. Does not modify content stored in the clipboard.
   void Clear();
 
   // Returns whether the clipboard history of the active account is empty.
@@ -64,23 +55,18 @@ class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver,
   void OnClipboardDataChanged() override;
 
  private:
-  // Adds |data| to the clipboard history associated with |account_id|.
-  void CommitData(const AccountId& account_id, ui::ClipboardData data);
+  // Adds |data| to the |history_list_|.
+  void CommitData(ui::ClipboardData data);
+
   void Pause();
   void Resume();
 
-  // SessionObserver:
-  void OnActiveUserSessionChanged(const AccountId& account_id) override;
-
-  // ShellObserver:
-  void OnShellDestroying() override;
-
   // The count of pauses.
   size_t num_pause_ = 0;
 
-  // Clipboard history is mapped by account ID to store different items per
-  // account when multiprofile is used. Lists of items are sorted by recency.
-  std::map<AccountId, std::list<ui::ClipboardData>> items_by_account_id_;
+  // The history of data copied to the Clipboard. Items of the list are sorted
+  // by recency.
+  std::list<ui::ClipboardData> history_list_;
 
   // Factory to create WeakPtrs used to debounce calls to CommitData().
   base::WeakPtrFactory<ClipboardHistory> commit_data_weak_factory_{this};

chrome/browser/ui/ash/clipboard_history_browsertest.cc

@@ -110,10 +110,10 @@ class ClipboardHistoryWithMultiProfileBrowserTest
   base::test::ScopedFeatureList feature_list_;
 };
 
-// Verify that the clipboard data history belonging to different users does not
-// interfere with each other.
+// Verify that the clipboard data history is recorded as expected in the
+// Multiuser environment.
 IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
-                       DisconflictInMultiUser) {
+                       VerifyClipboardHistoryAcrossMultiUser) {
   LoginUser(account_id1_);
   EXPECT_TRUE(GetClipboardData().empty());
 
@@ -127,10 +127,10 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
     EXPECT_EQ(copypaste_data1, data.front().text());
   }
 
-  // Log in as the user2. The clipboard history should be empty.
+  // Log in as the user2. The clipboard history should be non-empty.
   chromeos::UserAddingScreen::Get()->Start();
   AddUser(account_id2_);
-  EXPECT_TRUE(GetClipboardData().empty());
+  EXPECT_FALSE(GetClipboardData().empty());
 
   // Store text when the user2 is active.
   const std::string copypaste_data2("user2_text1");
@@ -138,7 +138,7 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
 
   {
     const std::list<ui::ClipboardData>& data = GetClipboardData();
-    EXPECT_EQ(1u, data.size());
+    EXPECT_EQ(2u, data.size());
     EXPECT_EQ(copypaste_data2, data.front().text());
   }
 
@@ -151,13 +151,16 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
 
   {
     const std::list<ui::ClipboardData>& data = GetClipboardData();
-    EXPECT_EQ(2u, data.size());
+    EXPECT_EQ(3u, data.size());
 
     // Note that items in |data| follow the time ordering. The most recent item
     // is always the first one.
     auto it = data.begin();
     EXPECT_EQ(copypaste_data3, it->text());
 
+    std::advance(it, 1u);
+    EXPECT_EQ(copypaste_data2, it->text());
+
     std::advance(it, 1u);
     EXPECT_EQ(copypaste_data1, it->text());
   }