Implement base class for app launcher context menu.

This additionally moves extension app context menu functionality
to its dedicated class.

BUG=584691
TEST=unit_test passes and no visual menu changes on devices.

Review URL: https://codereview.chromium.org/1673683002

Cr-Commit-Position: refs/heads/master@{#374696}

chrome/browser/ui/app_list/app_context_menu.h

@@ -14,16 +14,14 @@
 class AppListControllerDelegate;
 class Profile;
 
-namespace extensions {
-class ContextMenuMatcher;
-}
-
 namespace app_list {
 
 class AppContextMenuDelegate;
 
+// Base class of all context menus in app list view.
 class AppContextMenu : public ui::SimpleMenuModel::Delegate {
  public:
+  // Defines command ids, used in context menu of all types.
   enum CommandId {
     LAUNCH_NEW = 100,
     TOGGLE_PIN,
@@ -43,28 +41,11 @@ class AppContextMenu : public ui::SimpleMenuModel::Delegate {
     USE_LAUNCH_TYPE_COMMAND_END,
   };
 
-  AppContextMenu(AppContextMenuDelegate* delegate,
-                 Profile* profile,
-                 const std::string& app_id,
-                 AppListControllerDelegate* controller);
   ~AppContextMenu() override;
 
-  static void DisableInstalledExtensionCheckForTesting(bool disable);
-
-  // Note this could return NULL if corresponding extension is gone.
-  ui::MenuModel* GetMenuModel();
+  // Note this could return nullptr if corresponding app item is gone.
+  virtual ui::MenuModel* GetMenuModel();
 
-  void set_is_platform_app(bool is_platform_app) {
-    is_platform_app_ = is_platform_app;
-  }
-  void set_is_search_result(bool is_search_result) {
-    is_search_result_ = is_search_result;
-  }
-  void set_is_in_folder(bool is_in_folder) {
-    is_in_folder_ = is_in_folder;
-  }
-
- private:
   // ui::SimpleMenuModel::Delegate overrides:
   bool IsItemForCommandIdDynamic(int command_id) const override;
   base::string16 GetLabelForCommandId(int command_id) const override;
@@ -74,16 +55,28 @@ class AppContextMenu : public ui::SimpleMenuModel::Delegate {
                                   ui::Accelerator* accelerator) override;
   void ExecuteCommand(int command_id, int event_flags) override;
 
+ protected:
+  AppContextMenu(AppContextMenuDelegate* delegate,
+                 Profile* profile,
+                 const std::string& app_id,
+                 AppListControllerDelegate* controller);
+
+  // Creates default items, derived class may override to add their specific
+  // items.
+  virtual void BuildMenu(ui::SimpleMenuModel* menu_model);
+
+  const std::string& app_id() const { return app_id_; }
+  Profile* profile() const { return profile_; }
+  AppContextMenuDelegate* delegate() const { return delegate_; }
+  AppListControllerDelegate* controller() const { return controller_; }
+
+ private:
   AppContextMenuDelegate* delegate_;
   Profile* profile_;
   const std::string app_id_;
   AppListControllerDelegate* controller_;
-  bool is_platform_app_;
-  bool is_search_result_;
-  bool is_in_folder_;
 
   scoped_ptr<ui::SimpleMenuModel> menu_model_;
-  scoped_ptr<extensions::ContextMenuMatcher> extension_menu_items_;
 
   DISALLOW_COPY_AND_ASSIGN(AppContextMenu);
 };

chrome/browser/ui/app_list/app_context_menu_unittest.cc

@@ -11,10 +11,10 @@
 
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/menu_manager_factory.h"
-#include "chrome/browser/ui/app_list/app_context_menu.h"
 #include "chrome/browser/ui/app_list/app_context_menu_delegate.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
 #include "chrome/browser/ui/app_list/app_list_test_util.h"
+#include "chrome/browser/ui/app_list/extension_app_context_menu.h"
 #include "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -170,10 +170,10 @@ class AppContextMenuTest : public AppListTestBase {
     controller_->SetCanCreateShortcuts(can_create_shortcuts);
     controller_->SetCanShowAppInfo(can_show_app_info);
     controller_->SetExtensionLaunchType(profile(), app_id, launch_type);
-    app_list::AppContextMenu menu(menu_delegate(),
-                                  profile(),
-                                  app_id,
-                                  controller());
+    app_list::ExtensionAppContextMenu menu(menu_delegate(),
+                                           profile(),
+                                           app_id,
+                                           controller());
     menu.set_is_platform_app(platform_app);
     ui::MenuModel* menu_model = menu.GetMenuModel();
     ASSERT_NE(nullptr, menu_model);
@@ -245,10 +245,10 @@ class AppContextMenuTest : public AppListTestBase {
     controller_.reset(new FakeAppListControllerDelegate());
     controller_->SetCanCreateShortcuts(can_create_shortcuts);
     controller_->SetCanShowAppInfo(can_show_app_info);
-    app_list::AppContextMenu menu(menu_delegate(),
-                                  profile(),
-                                  extension_misc::kChromeAppId,
-                                  controller());
+    app_list::ExtensionAppContextMenu menu(menu_delegate(),
+                                           profile(),
+                                           extension_misc::kChromeAppId,
+                                           controller());
     ui::MenuModel* menu_model = menu.GetMenuModel();
     ASSERT_NE(nullptr, menu_model);
 
@@ -273,7 +273,8 @@ class AppContextMenuTest : public AppListTestBase {
 };
 
 TEST_F(AppContextMenuTest, ExtensionApp) {
-  app_list::AppContextMenu::DisableInstalledExtensionCheckForTesting(false);
+  app_list::ExtensionAppContextMenu::DisableInstalledExtensionCheckForTesting(
+      false);
   for (extensions::LaunchType launch_type = extensions::LAUNCH_TYPE_FIRST;
       launch_type < extensions::NUM_LAUNCH_TYPES;
       launch_type = static_cast<extensions::LaunchType>(launch_type+1)) {
@@ -307,7 +308,8 @@ TEST_F(AppContextMenuTest, ExtensionApp) {
 }
 
 TEST_F(AppContextMenuTest, ChromeApp) {
-  app_list::AppContextMenu::DisableInstalledExtensionCheckForTesting(true);
+  app_list::ExtensionAppContextMenu::DisableInstalledExtensionCheckForTesting(
+      true);
   for (size_t combinations = 0; combinations < (1 << 2); ++combinations) {
     TestChromeApp((combinations & (1 << 0)) != 0,
                   (combinations & (1 << 1)) != 0);
@@ -315,11 +317,12 @@ TEST_F(AppContextMenuTest, ChromeApp) {
 }
 
 TEST_F(AppContextMenuTest, NonExistingExtensionApp) {
-  app_list::AppContextMenu::DisableInstalledExtensionCheckForTesting(false);
-  app_list::AppContextMenu menu(menu_delegate(),
-                                profile(),
-                                "some_non_existing_extension_app",
-                                controller());
+  app_list::ExtensionAppContextMenu::DisableInstalledExtensionCheckForTesting(
+      false);
+  app_list::ExtensionAppContextMenu menu(menu_delegate(),
+                                         profile(),
+                                         "some_non_existing_extension_app",
+                                         controller());
   ui::MenuModel* menu_model = menu.GetMenuModel();
   EXPECT_EQ(nullptr, menu_model);
 }

chrome/browser/ui/app_list/extension_app_context_menu.h

+// Copyright 2016 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.
+
+#ifndef CHROME_BROWSER_UI_APP_LIST_EXTENSION_APP_CONTEXT_MENU_H_
+#define CHROME_BROWSER_UI_APP_LIST_EXTENSION_APP_CONTEXT_MENU_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/ui/app_list/app_context_menu.h"
+
+class AppListControllerDelegate;
+class Profile;
+
+namespace extensions {
+class ContextMenuMatcher;
+}
+
+namespace app_list {
+
+class AppContextMenuDelegate;
+
+class ExtensionAppContextMenu : public AppContextMenu {
+ public:
+  ExtensionAppContextMenu(AppContextMenuDelegate* delegate,
+                          Profile* profile,
+                          const std::string& app_id,
+                          AppListControllerDelegate* controller);
+  ~ExtensionAppContextMenu() override;
+
+  static void DisableInstalledExtensionCheckForTesting(bool disable);
+
+  // AppListContextMenu overrides:
+  ui::MenuModel* GetMenuModel() override;
+  void BuildMenu(ui::SimpleMenuModel* menu_model) override;
+
+  // ui::SimpleMenuModel::Delegate overrides:
+  base::string16 GetLabelForCommandId(int command_id) const override;
+  bool IsItemForCommandIdDynamic(int command_id) const override;
+  bool IsCommandIdChecked(int command_id) const override;
+  bool IsCommandIdEnabled(int command_id) const override;
+  void ExecuteCommand(int command_id, int event_flags) override;
+
+  void set_is_platform_app(bool is_platform_app) {
+    is_platform_app_ = is_platform_app;
+  }
+
+ private:
+  bool is_platform_app_ = false;
+
+  scoped_ptr<extensions::ContextMenuMatcher> extension_menu_items_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionAppContextMenu);
+};
+
+}  // namespace app_list
+
+#endif  // CHROME_BROWSER_UI_APP_LIST_EXTENSION_APP_CONTEXT_MENU_H_

chrome/browser/ui/app_list/extension_app_item.cc

@@ -11,8 +11,8 @@
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/app_list/app_context_menu.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
+#include "chrome/browser/ui/app_list/extension_app_context_menu.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_metrics.h"
@@ -311,11 +311,11 @@ void ExtensionAppItem::Activate(int event_flags) {
 }
 
 ui::MenuModel* ExtensionAppItem::GetContextMenuModel() {
-  context_menu_.reset(new app_list::AppContextMenu(
-      this, profile(), extension_id(), GetController()));
+  context_menu_.reset(new app_list::ExtensionAppContextMenu(this,
+                                                            profile(),
+                                                            extension_id(),
+                                                            GetController()));
   context_menu_->set_is_platform_app(is_platform_app_);
-  if (IsInFolder())
-    context_menu_->set_is_in_folder(true);
   return context_menu_->GetMenuModel();
 }
 

chrome/browser/ui/app_list/extension_app_item.h

@@ -21,7 +21,7 @@ class ExtensionEnableFlow;
 class Profile;
 
 namespace app_list {
-class AppContextMenu;
+class ExtensionAppContextMenu;
 }
 
 namespace extensions {
@@ -100,7 +100,7 @@ class ExtensionAppItem : public ChromeAppListItem,
   void UpdatePositionFromOrdering();
 
   scoped_ptr<extensions::IconImage> icon_;
-  scoped_ptr<app_list::AppContextMenu> context_menu_;
+  scoped_ptr<app_list::ExtensionAppContextMenu> context_menu_;
   scoped_ptr<ExtensionEnableFlow> extension_enable_flow_;
   AppListControllerDelegate* extension_enable_flow_controller_;
 

chrome/browser/ui/app_list/search/app_result.cc

@@ -7,8 +7,8 @@
 #include "base/time/time.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/app_list/app_context_menu.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
+#include "chrome/browser/ui/app_list/extension_app_context_menu.h"
 #include "chrome/browser/ui/app_list/search/search_util.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow.h"
 #include "chrome/common/extensions/extension_metrics.h"
@@ -121,10 +121,9 @@ scoped_ptr<SearchResult> AppResult::Duplicate() const {
 
 ui::MenuModel* AppResult::GetContextMenuModel() {
   if (!context_menu_) {
-    context_menu_.reset(new AppContextMenu(
+    context_menu_.reset(new ExtensionAppContextMenu(
         this, profile_, app_id_, controller_));
     context_menu_->set_is_platform_app(is_platform_app_);
-    context_menu_->set_is_search_result(true);
   }
 
   return context_menu_->GetMenuModel();

chrome/browser/ui/app_list/search/app_result.h

@@ -29,7 +29,7 @@ class ExtensionRegistry;
 
 namespace app_list {
 
-class AppContextMenu;
+class ExtensionAppContextMenu;
 
 class AppResult : public SearchResult,
                   public extensions::IconImage::Observer,
@@ -84,7 +84,7 @@ class AppResult : public SearchResult,
 
   bool is_platform_app_;
   scoped_ptr<extensions::IconImage> icon_;
-  scoped_ptr<AppContextMenu> context_menu_;
+  scoped_ptr<ExtensionAppContextMenu> context_menu_;
   scoped_ptr<ExtensionEnableFlow> extension_enable_flow_;
 
   extensions::ExtensionRegistry* extension_registry_;

chrome/chrome_browser_ui.gypi

@@ -2607,6 +2607,8 @@
       'browser/ui/app_list/app_list_view_delegate.h',
       'browser/ui/app_list/chrome_app_list_item.cc',
       'browser/ui/app_list/chrome_app_list_item.h',
+      'browser/ui/app_list/extension_app_context_menu.cc',
+      'browser/ui/app_list/extension_app_context_menu.h',
       'browser/ui/app_list/extension_app_item.cc',
       'browser/ui/app_list/extension_app_item.h',
       'browser/ui/app_list/extension_app_model_builder.cc',