chromeos: use unique_ptr in MultiUserWindowManager

BUG=756085,875111
TEST=covered by test

Change-Id: Idc65b7ca5b340401855694f1f123402612b73003
Reviewed-on: https://chromium-review.googlesource.com/c/1341021
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609372}

ash/multi_user/multi_user_window_manager.cc

@@ -164,13 +164,12 @@ MultiUserWindowManager::~MultiUserWindowManager() {
     animation_->CancelAnimation();
 
   // Remove all window observers.
-  WindowToEntryMap::iterator window = window_to_entry_.begin();
-  while (window != window_to_entry_.end()) {
+  while (!window_to_entry_.empty()) {
     // Explicitly remove this from window observer list since OnWindowDestroyed
     // no longer does that.
-    window->first->RemoveObserver(this);
-    OnWindowDestroyed(window->first);
-    window = window_to_entry_.begin();
+    aura::Window* window = window_to_entry_.begin()->first;
+    window->RemoveObserver(this);
+    OnWindowDestroyed(window);
   }
 
   Shell::Get()->session_controller()->RemoveObserver(this);
@@ -193,10 +192,13 @@ void MultiUserWindowManager::SetWindowOwner(aura::Window* window,
   if (GetWindowOwner(window) == account_id)
     return;
   DCHECK(GetWindowOwner(window).empty());
-  window_to_entry_[window] = new WindowEntry(account_id);
+  std::unique_ptr<WindowEntry> window_entry_ptr =
+      std::make_unique<WindowEntry>(account_id);
+  WindowEntry* window_entry = window_entry_ptr.get();
+  window_to_entry_[window] = std::move(window_entry_ptr);
 
   // Remember the initial visibility of the window.
-  window_to_entry_[window]->set_show(window->IsVisible());
+  window_entry->set_show(window->IsVisible());
 
   // Add observers to track state changes.
   window->AddObserver(this);
@@ -205,7 +207,7 @@ void MultiUserWindowManager::SetWindowOwner(aura::Window* window,
   // Check if this window was created due to a user interaction. If it was,
   // transfer it to the current user.
   if (show_for_current_user)
-    window_to_entry_[window]->set_show_for_user(current_account_id_);
+    window_entry->set_show_for_user(current_account_id_);
 
   // Add all transient children to our set of windows. Note that the function
   // will add the children but not the owner to the transient children map.
@@ -237,9 +239,8 @@ void MultiUserWindowManager::ShowWindowForUser(aura::Window* window,
 }
 
 bool MultiUserWindowManager::AreWindowsSharedAmongUsers() const {
-  WindowToEntryMap::const_iterator it = window_to_entry_.begin();
-  for (; it != window_to_entry_.end(); ++it) {
-    if (it->second->owner() != it->second->show_for_user())
+  for (auto& window_pair : window_to_entry_) {
+    if (window_pair.second->owner() != window_pair.second->show_for_user())
       return true;
   }
   return false;
@@ -300,8 +301,6 @@ void MultiUserWindowManager::OnWindowDestroyed(aura::Window* window) {
     return;
   }
   ::wm::TransientWindowManager::GetOrCreate(window)->RemoveObserver(this);
-  // Remove the window from the owners list.
-  delete window_to_entry_[window];
   window_to_entry_.erase(window);
 }
 
@@ -371,7 +370,7 @@ void MultiUserWindowManager::OnTransientChildRemoved(
 }
 
 void MultiUserWindowManager::OnTabletModeStarted() {
-  for (auto entry : window_to_entry_)
+  for (auto& entry : window_to_entry_)
     Shell::Get()->tablet_mode_controller()->AddWindow(entry.first);
 }
 
@@ -403,15 +402,15 @@ bool MultiUserWindowManager::ShowWindowForUserIntern(
   if (account_id != owner && minimized)
     return false;
 
-  WindowToEntryMap::iterator it = window_to_entry_.find(window);
-  it->second->set_show_for_user(account_id);
+  WindowEntry* window_entry = window_to_entry_[window].get();
+  window_entry->set_show_for_user(account_id);
 
   const bool teleported = !IsWindowOnDesktopOfUser(window, owner);
 
   // Show the window if the added user is the current one.
   if (account_id == current_account_id_) {
     // Only show the window if it should be shown according to its state.
-    if (it->second->show())
+    if (window_entry->show())
       SetWindowVisibility(window, true, kTeleportAnimationTime);
   } else {
     SetWindowVisibility(window, false, kTeleportAnimationTime);

ash/multi_user/multi_user_window_manager.h

@@ -7,8 +7,6 @@
 
 #include <map>
 #include <memory>
-#include <set>
-#include <string>
 
 #include "ash/ash_export.h"
 #include "ash/session/session_observer.h"
@@ -140,8 +138,8 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
     DISALLOW_COPY_AND_ASSIGN(WindowEntry);
   };
 
-  // TODO: make map to std::unique_ptr<WindowEntry>.
-  using WindowToEntryMap = std::map<aura::Window*, WindowEntry*>;
+  using WindowToEntryMap =
+      std::map<aura::Window*, std::unique_ptr<WindowEntry>>;
 
   const AccountId& GetWindowOwner(aura::Window* window) const;
 

chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc

@@ -150,13 +150,12 @@ MultiUserWindowManagerChromeOS::~MultiUserWindowManagerChromeOS() {
   ash_multi_user_window_manager_.reset();
 
   // Remove all window observers.
-  WindowToEntryMap::iterator window = window_to_entry_.begin();
-  while (window != window_to_entry_.end()) {
+  while (!window_to_entry_.empty()) {
     // Explicitly remove this from window observer list since OnWindowDestroyed
     // no longer does that.
-    window->first->RemoveObserver(this);
-    OnWindowDestroyed(window->first);
-    window = window_to_entry_.begin();
+    aura::Window* window = window_to_entry_.begin()->first;
+    window->RemoveObserver(this);
+    OnWindowDestroyed(window);
   }
 
   // Remove all app observers.
@@ -207,7 +206,10 @@ void MultiUserWindowManagerChromeOS::SetWindowOwner(
     return;
 
   DCHECK(GetWindowOwner(window).empty());
-  window_to_entry_[window] = new WindowEntry(account_id);
+  std::unique_ptr<WindowEntry> window_entry_ptr =
+      std::make_unique<WindowEntry>(account_id);
+  WindowEntry* window_entry = window_entry_ptr.get();
+  window_to_entry_[window] = std::move(window_entry_ptr);
 
   // Check if this window was created due to a user interaction. If it was,
   // transfer it to the current user.
@@ -228,7 +230,7 @@ void MultiUserWindowManagerChromeOS::SetWindowOwner(
   window->AddObserver(this);
 
   if (show_for_current_user)
-    window_to_entry_[window]->set_show_for_user(current_account_id_);
+    window_entry->set_show_for_user(current_account_id_);
 
   // Notify entry adding.
   for (Observer& observer : observers_)
@@ -259,9 +261,8 @@ void MultiUserWindowManagerChromeOS::ShowWindowForUser(
 }
 
 bool MultiUserWindowManagerChromeOS::AreWindowsSharedAmongUsers() const {
-  WindowToEntryMap::const_iterator it = window_to_entry_.begin();
-  for (; it != window_to_entry_.end(); ++it) {
-    if (it->second->owner() != it->second->show_for_user())
+  for (auto& window_pair : window_to_entry_) {
+    if (window_pair.second->owner() != window_pair.second->show_for_user())
       return true;
   }
   return false;
@@ -269,10 +270,9 @@ bool MultiUserWindowManagerChromeOS::AreWindowsSharedAmongUsers() const {
 
 void MultiUserWindowManagerChromeOS::GetOwnersOfVisibleWindows(
     std::set<AccountId>* account_ids) const {
-  for (WindowToEntryMap::const_iterator it = window_to_entry_.begin();
-       it != window_to_entry_.end(); ++it) {
-    if (it->first->IsVisible())
-      account_ids->insert(it->second->owner());
+  for (auto& window_pair : window_to_entry_) {
+    if (window_pair.first->IsVisible())
+      account_ids->insert(window_pair.second->owner());
   }
 }
 
@@ -333,8 +333,6 @@ void MultiUserWindowManagerChromeOS::RemoveObserver(Observer* observer) {
 }
 
 void MultiUserWindowManagerChromeOS::OnWindowDestroyed(aura::Window* window) {
-  // Remove the window from the owners list.
-  delete window_to_entry_[window];
   window_to_entry_.erase(window);
 }
 

chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.h

@@ -7,7 +7,6 @@
 
 #include <map>
 #include <memory>
-#include <string>
 
 #include "ash/multi_user/multi_user_window_manager_delegate.h"
 #include "ash/public/interfaces/multi_user_window_manager.mojom.h"
@@ -92,8 +91,8 @@ class MultiUserWindowManagerChromeOS
   // Returns the current user for unit tests.
   const AccountId& GetCurrentUserForTest() const;
 
- protected:
-  friend class UserSwitchAnimatorChromeOS;
+ private:
+  friend class ash::MultiUserWindowManagerChromeOSTest;
 
   class WindowEntry {
    public:
@@ -123,15 +122,10 @@ class MultiUserWindowManagerChromeOS
     DISALLOW_COPY_AND_ASSIGN(WindowEntry);
   };
 
-  // TODO(sky): make this map to unique_ptr<WindowEntry>.
-  using WindowToEntryMap = std::map<aura::Window*, WindowEntry*>;
-
-  const WindowToEntryMap& window_to_entry() { return window_to_entry_; }
-
- private:
-  friend class ash::MultiUserWindowManagerChromeOSTest;
+  using AccountIdToAppWindowObserver = std::map<AccountId, AppObserver*>;
 
-  typedef std::map<AccountId, AppObserver*> AccountIdToAppWindowObserver;
+  using WindowToEntryMap =
+      std::map<aura::Window*, std::unique_ptr<WindowEntry>>;
 
   // Add a browser window to the system so that the owner can be remembered.
   void AddBrowserWindow(Browser* browser);