Finalizing the 'OnClick' behavior of the Launcher items.

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 53225faf3d03ad5af1e0ee514c3dd91a91bc04f5..f37d27bf430c4c31f79b0d7a7a5832d43a4a6f78 100644
--- a/ash/launcher/launcher_view.cc
+++ b/ash/launcher/launcher_view.cc
@@ -31,6 +31,7 @@
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animator.h"
 #include "ui/gfx/canvas.h"
+#include "ui/native_theme/native_theme.h"
 #include "ui/views/animation/bounds_animator.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/menu/menu_model_adapter.h"
@@ -67,6 +68,9 @@ const float kReservedNonPanelIconProportion = 0.67f;
 // This is the command id of the menu item which contains the name of the menu.
 const int kCommandIdOfMenuName = 0;
 
+// This is the command id of the active menu item.
+const int kCommandIdOfActiveName = 1;
+
 namespace {
 
 // The MenuModelAdapter gets slightly changed to adapt the menu appearance to
@@ -82,7 +86,9 @@ class LauncherMenuModelAdapter
                                         int icon_size,
                                         int* left_margin,
                                         int* right_margin) const OVERRIDE;
-
+  virtual bool GetBackgroundColor(int command_id,
+                                  bool is_hovered,
+                                  SkColor* override_color) const OVERRIDE;
  private:
 
   DISALLOW_COPY_AND_ASSIGN(LauncherMenuModelAdapter);
@@ -101,6 +107,20 @@ const gfx::Font* LauncherMenuModelAdapter::GetLabelFont(
   return &rb.GetFont(ui::ResourceBundle::BoldFont);
 }
 
+bool LauncherMenuModelAdapter::GetBackgroundColor(
+    int command_id,
+    bool is_hovered,
+    SkColor *override_color) const {
+  if (command_id != kCommandIdOfActiveName)
+    return false;
+
+  ui::NativeTheme* native_theme = ui::NativeTheme::instance();
+  *override_color = native_theme->GetSystemColor(is_hovered ?
+      ui::NativeTheme::kColorId_FocusedSelectedMenuItemBackground :
+      ui::NativeTheme::kColorId_SelectedMenuItemBackground);
+  return true;
+}
+
 void LauncherMenuModelAdapter::GetHorizontalIconMargins(
     int command_id,
     int icon_size,
@@ -1189,71 +1209,46 @@ void LauncherView::ButtonPressed(views::Button* sender,
 
   tooltip_->Close();
 
+  if (event.IsShiftDown())

[sky, 2013/01/31 21:15:02]

This code is fragile (I know I've added some too), can you create a scoping
object whose constructor turns it on and destructor unsets.

[Mr4D (OOO till 08-26), 2013/01/31 22:38:52]

Done.

+    ui::LayerAnimator::set_slow_animation_mode(true);
+
   // Collect usage statistics before we decide what to do with the click.
   switch (model_->items()[view_index].type) {
     case TYPE_APP_SHORTCUT:
     case TYPE_PLATFORM_APP:
       Shell::GetInstance()->delegate()->RecordUserMetricsAction(
           UMA_LAUNCHER_CLICK_ON_APP);
+      delegate_->ItemClicked(model_->items()[view_index], event.flags());
+      // Fallthrough
+    case TYPE_TABBED:
+    case TYPE_APP_PANEL:
       break;
 
     case TYPE_APP_LIST:
       Shell::GetInstance()->delegate()->RecordUserMetricsAction(
           UMA_LAUNCHER_CLICK_ON_APPLIST_BUTTON);
+      Shell::GetInstance()->ToggleAppList(GetWidget()->GetNativeView());
       break;
 
     case TYPE_BROWSER_SHORTCUT:
       // Click on browser icon is counted in app clicks.
       Shell::GetInstance()->delegate()->RecordUserMetricsAction(
           UMA_LAUNCHER_CLICK_ON_APP);
-      break;
-
-    case TYPE_TABBED:
-    case TYPE_APP_PANEL:
+      delegate_->OnBrowserShortcutClicked(event.flags());
       break;
   }
 
-  // If the item is already active we show a menu - otherwise we activate
-  // the item dependent on its type.
-  // Note that the old launcher has no menu and falls back automatically to
-  // the click action.
-  bool call_object_handler = model_->items()[view_index].type == TYPE_APP_LIST;
-  if (!call_object_handler) {
-    call_object_handler =
-        model_->items()[view_index].status != ash::STATUS_ACTIVE;
-    if (!call_object_handler) {
-      // ShowListMenuForView only returns true if the menu was shown.
-      if (ShowListMenuForView(model_->items()[view_index],
-                              sender)) {
-        // When the menu was shown it is possible that this got deleted.
-        return;
-      }
-      call_object_handler = true;
-    }
-  }
+  if (event.IsShiftDown())
+    ui::LayerAnimator::set_slow_animation_mode(false);
 
-  if (call_object_handler) {
-    if (event.IsShiftDown())
-      ui::LayerAnimator::set_slow_animation_mode(true);
-    // The menu was not shown and the objects click handler should be called.
-    switch (model_->items()[view_index].type) {
-      case TYPE_TABBED:
-      case TYPE_APP_PANEL:
-      case TYPE_APP_SHORTCUT:
-      case TYPE_PLATFORM_APP:
-        delegate_->ItemClicked(model_->items()[view_index], event.flags());
-        break;
-      case TYPE_APP_LIST:
-        Shell::GetInstance()->ToggleAppList(GetWidget()->GetNativeView());
-        break;
-      case TYPE_BROWSER_SHORTCUT:
-        delegate_->OnBrowserShortcutClicked(event.flags());
-        break;
+  if (model_->items()[view_index].type != TYPE_APP_LIST) {
+    // ShowListMenuForView only returns true if the menu was shown.
+    if (ShowListMenuForView(model_->items()[view_index],
+                            sender)) {
+      // When the menu was shown it is possible that this got deleted.
+      return;
     }
-    if (event.IsShiftDown())
-      ui::LayerAnimator::set_slow_animation_mode(false);
   }
-
 }
 
 bool LauncherView::ShowListMenuForView(const LauncherItem& item,
@@ -1261,9 +1256,9 @@ bool LauncherView::ShowListMenuForView(const LauncherItem& item,
   scoped_ptr<ui::MenuModel> menu_model;
   menu_model.reset(delegate_->CreateApplicationMenu(item));
 
-  // Make sure we have a menu and it has at least one item in addition to the
-  // application title.
-  if (!menu_model.get() || menu_model->GetItemCount() <= 1)
+  // Make sure we have a menu and it has at least two items in addition to the
+  // application title and the 2 spacing separators.
+  if (!menu_model.get() || menu_model->GetItemCount() <= 4)
     return false;
 
   ShowMenu(menu_model.get(), source, gfx::Point(), false);