Refactor LauncherItemController and LauncherItemDelegate

ash/launcher/launcher_item_delegate_manager.cc

 // Copyright 2013 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/launcher/launcher_item_delegate_manager.h"
 
+#include "ash/launcher/launcher_item_delegate.h"
+#include "ash/launcher/launcher_model.h"
+#include "ash/shell.h"
+
 namespace ash {
 
-LauncherItemDelegateManager::LauncherItemDelegateManager() {
+LauncherItemDelegateManager::LauncherItemDelegateManager()
+    : register_count_(0),
+      unregister_count_(0) {
+  Shell::GetInstance()->launcher_model()->AddObserver(this);
 }
 
 LauncherItemDelegateManager::~LauncherItemDelegateManager() {
+  Shell::GetInstance()->launcher_model()->RemoveObserver(this);
+  LauncherIDToItemDelegateMap::const_iterator iter =

[sky, 2013/09/11 01:42:57]

STLDeleteContainerPairSecondPointers.

[simonhong_, 2013/09/11 09:05:26]

Done.

+      id_to_item_delegate_map_.begin();
+  for (; iter != id_to_item_delegate_map_.end(); ++iter) {
+    delete iter->second;
+    ++unregister_count_;

[sky, 2013/09/11 01:42:57]

What value does this add beyond the DHECKs you have?

[simonhong_, 2013/09/11 09:05:26]

It is useless.
Removed.

+  }
+
+  // The number of register and unregister must be equal.
+  DCHECK_EQ(register_count_, unregister_count_);
 }
 
 void LauncherItemDelegateManager::RegisterLauncherItemDelegate(
-    ash::LauncherItemType type, LauncherItemDelegate* item_delegate) {
-  // When a new |item_delegate| is registered with an exsiting |type|, it will
-  // get overwritten.
-  item_type_to_item_delegate_map_[type] = item_delegate;
+    ash::LauncherID id,
+    LauncherItemDelegate* item_delegate) {
+  DCHECK(id_to_item_delegate_map_.find(id) == id_to_item_delegate_map_.end());
+  id_to_item_delegate_map_[id] = item_delegate;
+  ++register_count_;
+}
+
+void LauncherItemDelegateManager::UpdateLauncherItemDelegate(
+    ash::LauncherID id,
+    LauncherItemDelegate* item_delegate) {
+  UnregisterLauncherItemDelegate(id);
+  RegisterLauncherItemDelegate(id, item_delegate);
 }
 
 LauncherItemDelegate* LauncherItemDelegateManager::GetLauncherItemDelegate(
-    ash::LauncherItemType item_type) {
-  DCHECK(item_type_to_item_delegate_map_.find(item_type) !=
-      item_type_to_item_delegate_map_.end());
-  return item_type_to_item_delegate_map_[item_type];
+    ash::LauncherID id) {
+  DCHECK(id_to_item_delegate_map_.find(id) != id_to_item_delegate_map_.end());
+  return id_to_item_delegate_map_[id];
+}
+
+void LauncherItemDelegateManager::LauncherItemAdded(int index) {
+}
+
+void LauncherItemDelegateManager::LauncherItemRemoved(int index,
+                                                      ash::LauncherID id) {
+  UnregisterLauncherItemDelegate(id);
+}
+
+void LauncherItemDelegateManager::LauncherItemMoved(int start_index,
+                                                    int target_index) {
+}
+
+void LauncherItemDelegateManager::LauncherItemChanged(
+    int index,
+    const LauncherItem& old_item) {
+}
+
+void LauncherItemDelegateManager::LauncherStatusChanged() {
+}
+
+void LauncherItemDelegateManager::UnregisterAllLauncherItemDelegateForTest() {

[sky, 2013/09/11 01:42:57]

I don't like this being public. Instead put this logic in a class that is a
friend of LauncherItemDelegateManager. There are many examples of this pattern
in ash.

[simonhong_, 2013/09/11 09:05:26]

Done.

+  LauncherIDToItemDelegateMap::const_iterator iter =
+      id_to_item_delegate_map_.begin();
+  for (; iter != id_to_item_delegate_map_.end(); ++iter) {
+    delete iter->second;
+    ++unregister_count_;
+  }
+  id_to_item_delegate_map_.clear();
+}
+
+void LauncherItemDelegateManager::UnregisterLauncherItemDelegate(
+    ash::LauncherID id) {
+  DCHECK(id_to_item_delegate_map_.find(id) != id_to_item_delegate_map_.end());
+  delete id_to_item_delegate_map_[id];
+  id_to_item_delegate_map_.erase(id);
+  ++unregister_count_;
 }
 
 }  // namespace ash