Refactor LauncherDelegate

chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc

 // Copyright (c) 2012 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 "chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h"
 
 #include <vector>
 
 #include "ash/ash_switches.h"
 #include "ash/launcher/launcher.h"
@@ skipping 682 matching lines @@
 void ChromeLauncherControllerPerApp::CreateNewWindow() {
   chrome::NewEmptyWindow(
       GetProfileForNewWindows(), chrome::HOST_DESKTOP_TYPE_ASH);
 }
 
 void ChromeLauncherControllerPerApp::CreateNewIncognitoWindow() {
   chrome::NewEmptyWindow(GetProfileForNewWindows()->GetOffTheRecordProfile(),
                          chrome::HOST_DESKTOP_TYPE_ASH);
 }
 
-bool ChromeLauncherControllerPerApp::CanPin() const {
-  const PrefService::Preference* pref =
-      profile_->GetPrefs()->FindPreference(prefs::kPinnedLauncherApps);
-  return pref && pref->IsUserModifiable();
-}
-
 void ChromeLauncherControllerPerApp::PersistPinnedState() {
   if (ignore_persist_pinned_state_change_)
     return;
   // It is a coding error to call PersistPinnedState() if the pinned apps are
   // not user-editable. The code should check earlier and not perform any
   // modification actions that trigger persisting the state.
   if (!CanPin()) {
     NOTREACHED() << "Can't pin but pinned state being updated";
     return;
   }
@@ skipping 219 matching lines @@
   return new LauncherContextMenu(this, &item, root_window);
 }
 
 ash::LauncherMenuModel* ChromeLauncherControllerPerApp::CreateApplicationMenu(
     const ash::LauncherItem& item,
     int event_flags) {
   return new LauncherApplicationMenuItemModel(GetApplicationList(item,
                                                                  event_flags));
 }
 
+bool ChromeLauncherControllerPerApp::IsDraggable(
+    const ash::LauncherItem& item) {
+  return (item.type == ash::TYPE_APP_SHORTCUT ||
+          item.type == ash::TYPE_WINDOWED_APP) ? CanPin() : true;
+}
+
+bool ChromeLauncherControllerPerApp::ShouldShowTooltip(
+    const ash::LauncherItem& item) {
+  if (item.type == ash::TYPE_APP_PANEL &&
+      id_to_item_controller_map_[item.id]->IsVisible())
+    return false;
+  return true;
+}
+
 ash::LauncherID ChromeLauncherControllerPerApp::GetIDByWindow(
     aura::Window* window) {
   for (IDToItemControllerMap::const_iterator i =
            id_to_item_controller_map_.begin();
        i != id_to_item_controller_map_.end(); ++i) {
     if (i->second->HasWindow(window)) {
       // Since this might be a reserved index, there might be no item in the
       // launcher for it.
       if (model_->ItemIndexByID(i->first) < 0)
         break;
       return i->first;
     }
   }
   if (window->type() == aura::client::WINDOW_TYPE_NORMAL) {
     // Coming here we are looking for the associated browser item as the
     // default.
     // TODO(flackr): This shouldn't return a default icon if no window is found.
     //    The browser launcher item controller should know which windows it is
     //    managing so that it is identified as the ID in the above loop.
     int browser_index = ash::launcher::GetBrowserItemIndex(*model_);
     // Note that there should always be a browser item in the launcher.
     DCHECK_GE(browser_index, 0);
     return model_->items()[browser_index].id;
   }
   return 0;
 }
 
-bool ChromeLauncherControllerPerApp::IsDraggable(
-    const ash::LauncherItem& item) {
-  return (item.type == ash::TYPE_APP_SHORTCUT ||
-          item.type == ash::TYPE_WINDOWED_APP) ? CanPin() : true;
-}
-
-bool ChromeLauncherControllerPerApp::ShouldShowTooltip(
-    const ash::LauncherItem& item) {
-  if (item.type == ash::TYPE_APP_PANEL &&
-      id_to_item_controller_map_[item.id]->IsVisible())
-    return false;
-  return true;
-}
-
 void ChromeLauncherControllerPerApp::OnLauncherCreated(
     ash::Launcher* launcher) {
   launchers_.insert(launcher);
   launcher->shelf_widget()->shelf_layout_manager()->AddObserver(this);
 }
 
 void ChromeLauncherControllerPerApp::OnLauncherDestroyed(
     ash::Launcher* launcher) {
   launchers_.erase(launcher);
   // RemoveObserver is not called here, since by the time this method is called
   // Launcher is already in its destructor.
 }
 
+// TODO: Remove LauncherItemDelegate interface from
+// ChromeLauncherControllerPerApp. LauncherItemDelegate should be derived by
+// LauncherItemController after removing PerBrowserLauncher.
+ash::LauncherItemDelegate*
+ChromeLauncherControllerPerApp::GetLauncherItemDelegate(
+    const ash::LauncherItem& item) {
+  IDToItemControllerMap::iterator iter =
+      id_to_item_controller_map_.find(item.id);
+  return iter != id_to_item_controller_map_.end() ? this : NULL;
+}
+
+bool ChromeLauncherControllerPerApp::CanPin() const {
+  const PrefService::Preference* pref =
+      profile_->GetPrefs()->FindPreference(prefs::kPinnedLauncherApps);
+  return pref && pref->IsUserModifiable();
+}
+
 void ChromeLauncherControllerPerApp::LauncherItemAdded(int index) {
 }
 
 void ChromeLauncherControllerPerApp::LauncherItemRemoved(
     int index,
     ash::LauncherID id) {
 }
 
 void ChromeLauncherControllerPerApp::LauncherItemMoved(
     int start_index,
     int target_index) {
   ash::LauncherID id = model_->items()[target_index].id;
   if (HasItemController(id) && IsPinned(id))
     PersistPinnedState();
 }
 
 void ChromeLauncherControllerPerApp::LauncherItemChanged(
     int index,
     const ash::LauncherItem& old_item) {
   ash::LauncherID id = model_->items()[index].id;
-  DCHECK(HasItemController(id));
-  id_to_item_controller_map_[id]->LauncherItemChanged(index, old_item);
+  if (HasItemController(id))

[Mr4D (OOO till 08-26), 2013/08/13 18:18:24]

Do you know why this is necessary? We shouldn't come here in the first place if
an item is not valid.

[simonhong_, 2013/08/13 19:54:56]

This is needed because we have two LauncherDelegate.
LauncherModel notifies to its observers regardless of the changing source.
When AshLauncherDelegate change LauncherItem (ex, app list item), this
notification also reaches to here.

[Mr4D (OOO till 08-26), 2013/08/14 15:52:19]

Which 2? Are you talking about the "Ash" and the "Chrome" portion above? If so -
please read my comments above.

[simonhong_, 2013/08/14 23:47:51]

Sorry, I wrote wrong answer.
Yes, currently we can't reach here and also with this CL.
But, think about LauncherItem for dialog (for TaskManager).
When LauncherItem is changed by LauncherModel::Set(), LauncherModel call
LauncherItemChanged() from observer list.
Then, we can reach here.
I think DCHECK should be removed because we will add LauncherItem that not
handled by ChromeLauncherController.

[Mr4D (OOO till 08-26), 2013/08/15 15:53:19]

So we plan that we do not have a ChromeLauncherController for some items? And as
such we will run into many new special cases after you have spent so much time
to make everything nice and clean? Why not?

+    id_to_item_controller_map_[id]->LauncherItemChanged(index, old_item);
 }
 
 void ChromeLauncherControllerPerApp::LauncherStatusChanged() {
 }
 
 void ChromeLauncherControllerPerApp::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
@@ skipping 646 matching lines @@
     browser_to_close.pop_back();
   }
 }
 
 void
 ChromeLauncherControllerPerApp::MoveItemWithoutPinnedStateChangeNotification(
     int source_index, int target_index) {
   base::AutoReset<bool> auto_reset(&ignore_persist_pinned_state_change_, true);
   model_->Move(source_index, target_index);
 }