Refactor LauncherItemController and LauncherItemDelegate

ash/launcher/launcher_view.cc

Index: ash/launcher/launcher_view.cc
diff --git a/ash/launcher/launcher_view.cc b/ash/launcher/launcher_view.cc
index dc02154455490bd1bc74c37360936257ba446aa7..4ed0961cb1069e7de47b297d593d1cda955f035a 100644
--- a/ash/launcher/launcher_view.cc
+++ b/ash/launcher/launcher_view.cc
@@ -986,8 +986,8 @@ void LauncherView::PrepareForDrag(Pointer pointer,
 
   // If the item is no longer draggable, bail out.
   LauncherItemDelegate* item_delegate = item_manager_->GetLauncherItemDelegate(
-      model_->items()[start_drag_index_].type);
-  if (!item_delegate->IsDraggable(model_->items()[start_drag_index_])) {
+      model_->items()[start_drag_index_].id);
+  if (item_delegate && !item_delegate->IsDraggable()) {
     CancelDrag(-1);
     return;
   }
@@ -1004,8 +1004,8 @@ void LauncherView::ContinueDrag(const ui::LocatedEvent& event) {
   DCHECK_NE(-1, current_index);
 
   LauncherItemDelegate* item_delegate = item_manager_->GetLauncherItemDelegate(
-      model_->items()[current_index].type);
-  if (!item_delegate->IsDraggable(model_->items()[current_index])) {
+      model_->items()[current_index].id);
+  if (item_delegate && !item_delegate->IsDraggable()) {
     CancelDrag(-1);
     return;
   }
@@ -1533,9 +1533,9 @@ void LauncherView::PointerPressedOnButton(views::View* view,
     return;
 
   LauncherItemDelegate* item_delegate = item_manager_->GetLauncherItemDelegate(
-      model_->items()[index].type);
+      model_->items()[index].id);
   if (view_model_->view_size() <= 1 ||
-      !item_delegate->IsDraggable(model_->items()[index]))
+      (item_delegate && !item_delegate->IsDraggable()))
     return;  // View is being deleted or not draggable, ignore request.
 
   ShelfLayoutManager* shelf = tooltip_->shelf_layout_manager();
@@ -1605,8 +1605,8 @@ base::string16 LauncherView::GetAccessibleName(const views::View* view) {
     return base::string16();
 
   LauncherItemDelegate* item_delegate = item_manager_->GetLauncherItemDelegate(
-      model_->items()[view_index].type);
-  return item_delegate->GetTitle(model_->items()[view_index]);
+      model_->items()[view_index].id);
+  return item_delegate ? item_delegate->GetTitle() : base::string16();
 }
 
 void LauncherView::ButtonPressed(views::Button* sender,
@@ -1665,8 +1665,9 @@ void LauncherView::ButtonPressed(views::Button* sender,
 
     LauncherItemDelegate* item_delegate =
         item_manager_->GetLauncherItemDelegate(
-            model_->items()[view_index].type);
-    item_delegate->ItemSelected(model_->items()[view_index], event);
+            model_->items()[view_index].id);
+    if (item_delegate)
+      item_delegate->ItemSelected(event);
 
     ShowListMenuForView(model_->items()[view_index], sender, event);
   }
@@ -1677,8 +1678,9 @@ bool LauncherView::ShowListMenuForView(const LauncherItem& item,
                                        const ui::Event& event) {
   scoped_ptr<ash::LauncherMenuModel> menu_model;
   LauncherItemDelegate* item_delegate =
-      item_manager_->GetLauncherItemDelegate(item.type);
-  menu_model.reset(item_delegate->CreateApplicationMenu(item, event.flags()));
+      item_manager_->GetLauncherItemDelegate(item.id);
+  if (item_delegate)
+    menu_model.reset(item_delegate->CreateApplicationMenu(event.flags()));
 
   // Make sure we have a menu and it has at least two items in addition to the
   // application title and the 3 spacing separators.
@@ -1711,10 +1713,11 @@ void LauncherView::ShowContextMenuForView(views::View* source,
   }
   scoped_ptr<ui::MenuModel> menu_model;
   LauncherItemDelegate* item_delegate = item_manager_->GetLauncherItemDelegate(
-      model_->items()[view_index].type);
-  menu_model.reset(item_delegate->CreateContextMenu(
-      model_->items()[view_index],
-      source->GetWidget()->GetNativeView()->GetRootWindow()));
+      model_->items()[view_index].id);
+  if (item_delegate) {
+    menu_model.reset(item_delegate->CreateContextMenu(
+        source->GetWidget()->GetNativeView()->GetRootWindow()));
+  }
   if (!menu_model)
     return;
 
@@ -1874,8 +1877,8 @@ bool LauncherView::ShouldShowTooltipForView(const views::View* view) const {
   if (!item)
     return true;
   LauncherItemDelegate* item_delegate =
-      item_manager_->GetLauncherItemDelegate(item->type);
-  return item_delegate->ShouldShowTooltip(*item);
+      item_manager_->GetLauncherItemDelegate(item->id);

[Mr4D (OOO till 08-26), 2013/10/09 16:14:08]

Is it possible to have an item with an assigned id which has no item delegate?

[simonhong_, 2013/10/09 22:14:10]

In production code, all item have its item delegate.
But some items in unittests don't have.

[Mr4D (OOO till 08-26), 2013/10/10 04:20:15]

I just looked and there are many checks for this.

Since this can only be a problem for unit tests and not for production code it
would be ideal to be able to replace that with DCHECK's instead since it could
indicate a real problem.

Would it be possible to change the unit tests accordingly? Would that be much
work?

+  return item_delegate ? item_delegate->ShouldShowTooltip() : false;
 }
 
 int LauncherView::CalculateShelfDistance(const gfx::Point& coordinate) const {