cros: Add context menu to internal app

This cl adds context menu to internal app at different places:
1. Add context menu to recommended searched results.
2. Add context menu to items in app list.
3. Add context menu to shelf items.

Bug: 824437
Test: new tests in AppContextMenuTest and LauncherContextMenuTest
Change-Id: I46a16b2a2b78d0027a3ce9a4f0afc94581e0c2b9
Reviewed-on: https://chromium-review.googlesource.com/1023042
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553338}

chrome/browser/ui/BUILD.gn

@@ -3654,6 +3654,8 @@ split_static_library("ui") {
         "ash/launcher/crostini_app_window_shelf_controller.h",
         "ash/launcher/crostini_shelf_context_menu.cc",
         "ash/launcher/crostini_shelf_context_menu.h",
+        "ash/launcher/internal_app_shelf_context_menu.cc",
+        "ash/launcher/internal_app_shelf_context_menu.h",
         "ash/launcher/internal_app_window_shelf_controller.cc",
         "ash/launcher/internal_app_window_shelf_controller.h",
         "ash/launcher/launcher_arc_app_updater.cc",

chrome/browser/ui/app_list/app_context_menu.h

@@ -47,6 +47,10 @@ 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;
 
   // Note this could return nullptr if corresponding app item is gone.
@@ -61,11 +65,6 @@ class AppContextMenu : public ui::SimpleMenuModel::Delegate {
   bool GetIconForCommandId(int command_id, gfx::Image* icon) const 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);

chrome/browser/ui/app_list/app_context_menu_unittest.cc

@@ -20,6 +20,8 @@
 #include "chrome/browser/ui/app_list/arc/arc_app_test.h"
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
 #include "chrome/browser/ui/app_list/extension_app_context_menu.h"
+#include "chrome/browser/ui/app_list/internal_app/internal_app_item.h"
+#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "chrome/browser/ui/app_list/test/fake_app_list_model_updater.h"
 #include "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h"
 #include "chrome/test/base/testing_profile.h"
@@ -591,3 +593,19 @@ TEST_F(AppContextMenuTest, CommandIdsMatchEnumsForHistograms) {
   EXPECT_EQ(202, app_list::AppContextMenu::USE_LAUNCH_TYPE_FULLSCREEN);
   EXPECT_EQ(203, app_list::AppContextMenu::USE_LAUNCH_TYPE_WINDOW);
 }
+
+// Tests that internal app's context menu is correct.
+TEST_P(AppContextMenuTest, InternalAppMenu) {
+  for (const auto& internal_app : app_list::GetInternalAppList()) {
+    if (!internal_app.show_in_launcher)
+      continue;
+
+    controller()->SetAppPinnable(internal_app.app_id,
+                                 AppListControllerDelegate::PIN_EDITABLE);
+    InternalAppItem item(profile(), nullptr, internal_app);
+    ui::MenuModel* menu = item.GetContextMenuModel();
+    ASSERT_NE(nullptr, menu);
+    EXPECT_EQ(1, menu->GetItemCount());
+    ValidateItemState(menu, 0, MenuState(app_list::AppContextMenu::TOGGLE_PIN));
+  }
+}

chrome/browser/ui/app_list/internal_app/internal_app_item.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/app_list/internal_app/internal_app_item.h"
 
+#include "chrome/browser/ui/app_list/app_context_menu.h"
 #include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "ui/app_list/app_list_constants.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -25,6 +26,8 @@ InternalAppItem::InternalAppItem(
     SetDefaultPositionIfApplicable();
 }
 
+InternalAppItem::~InternalAppItem() = default;
+
 const char* InternalAppItem::GetItemType() const {
   return InternalAppItem::kItemType;
 }
@@ -32,3 +35,15 @@ const char* InternalAppItem::GetItemType() const {
 void InternalAppItem::Activate(int event_flags) {
   app_list::OpenInternalApp(id(), profile());
 }
+
+ui::MenuModel* InternalAppItem::GetContextMenuModel() {
+  if (!context_menu_) {
+    context_menu_ = std::make_unique<app_list::AppContextMenu>(
+        nullptr, profile(), id(), GetController());
+  }
+  return context_menu_->GetMenuModel();
+}
+
+app_list::AppContextMenu* InternalAppItem::GetAppContextMenu() {
+  return context_menu_.get();
+}

chrome/browser/ui/app_list/internal_app/internal_app_item.h

@@ -9,6 +9,7 @@
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
 
 namespace app_list {
+class AppContextMenu;
 struct InternalApp;
 }
 
@@ -20,13 +21,16 @@ class InternalAppItem : public ChromeAppListItem {
   InternalAppItem(Profile* profile,
                   const app_list::AppListSyncableService::SyncItem* sync_item,
                   const app_list::InternalApp& internal_app);
+  ~InternalAppItem() override;
 
-  ~InternalAppItem() override = default;
-
- private:
   // ChromeAppListItem:
   void Activate(int event_flags) override;
   const char* GetItemType() const override;
+  ui::MenuModel* GetContextMenuModel() override;
+  app_list::AppContextMenu* GetAppContextMenu() override;
+
+ private:
+  std::unique_ptr<app_list::AppContextMenu> context_menu_;
 
   DISALLOW_COPY_AND_ASSIGN(InternalAppItem);
 };

chrome/browser/ui/app_list/internal_app/internal_app_metadata.cc

@@ -19,20 +19,6 @@
 
 namespace app_list {
 
-namespace {
-
-// Returns InternalApp by |app_id|.
-// Returns nullptr if |app_id| does not correspond to an internal app.
-const InternalApp* FindInternalApp(const std::string& app_id) {
-  for (const auto& app : GetInternalAppList()) {
-    if (app_id == app.app_id)
-      return &app;
-  }
-  return nullptr;
-}
-
-}  // namespace
-
 const std::vector<InternalApp>& GetInternalAppList() {
   static const base::NoDestructor<std::vector<InternalApp>> internal_app_list(
       {{kInternalAppIdKeyboardShortcutViewer,
@@ -49,6 +35,14 @@ const std::vector<InternalApp>& GetInternalAppList() {
   return *internal_app_list;
 }
 
+const InternalApp* FindInternalApp(const std::string& app_id) {
+  for (const auto& app : GetInternalAppList()) {
+    if (app_id == app.app_id)
+      return &app;
+  }
+  return nullptr;
+}
+
 bool IsInternalApp(const std::string& app_id) {
   return !!FindInternalApp(app_id);
 }

chrome/browser/ui/app_list/internal_app/internal_app_metadata.h

@@ -38,6 +38,10 @@ struct InternalApp {
 // Returns a list of Chrome OS internal apps, which are searchable in launcher.
 const std::vector<InternalApp>& GetInternalAppList();
 
+// Returns InternalApp by |app_id|.
+// Returns nullptr if |app_id| does not correspond to an internal app.
+const InternalApp* FindInternalApp(const std::string& app_id);
+
 // Returns true if |app_id| corresponds to an internal app.
 bool IsInternalApp(const std::string& app_id);
 

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

@@ -6,6 +6,7 @@
 
 #include "ash/public/cpp/app_list/internal_app_id_constants.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/internal_app/internal_app_metadata.h"
 #include "chrome/browser/ui/app_list/search/search_util.h"
@@ -51,7 +52,16 @@ std::unique_ptr<ChromeSearchResult> InternalAppResult::Duplicate() const {
 }
 
 ui::MenuModel* InternalAppResult::GetContextMenuModel() {
-  return nullptr;
+  const auto* internal_app = app_list::FindInternalApp(id());
+  DCHECK(internal_app);
+  if (!internal_app->show_in_launcher)
+    return nullptr;
+
+  if (!context_menu_) {
+    context_menu_ = std::make_unique<AppContextMenu>(nullptr, profile(), id(),
+                                                     controller());
+  }
+  return context_menu_->GetMenuModel();
 }
 
 }  // namespace app_list

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

@@ -16,6 +16,8 @@ class Profile;
 
 namespace app_list {
 
+class AppContextMenu;
+
 class InternalAppResult : public AppResult {
  public:
   InternalAppResult(Profile* profile,
@@ -33,6 +35,8 @@ class InternalAppResult : public AppResult {
   void ExecuteLaunchCommand(int event_flags) override;
 
  private:
+  std::unique_ptr<AppContextMenu> context_menu_;
+
   DISALLOW_COPY_AND_ASSIGN(InternalAppResult);
 };
 

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

+// Copyright 2018 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/internal_app_shelf_context_menu.h"
+
+#include "ash/public/cpp/shelf_item.h"
+#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
+#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
+#include "chrome/grit/generated_resources.h"
+#include "ui/base/ui_base_features.h"
+
+InternalAppShelfContextMenu::InternalAppShelfContextMenu(
+    ChromeLauncherController* controller,
+    const ash::ShelfItem* item,
+    int64_t display_id)
+    : LauncherContextMenu(controller, item, display_id) {
+  Init();
+}
+
+void InternalAppShelfContextMenu::Init() {
+  const bool app_is_open = controller()->IsOpen(item().id);
+  if (!app_is_open) {
+    AddContextMenuOption(MENU_OPEN_NEW, IDS_APP_CONTEXT_MENU_ACTIVATE_ARC);
+    if (!features::IsTouchableAppContextMenuEnabled())
+      AddSeparator(ui::NORMAL_SEPARATOR);
+  }
+
+  const auto* internal_app = app_list::FindInternalApp(item().id.app_id);
+  DCHECK(internal_app);
+  if (internal_app->show_in_launcher)
+    AddPinMenu();
+
+  if (app_is_open)
+    AddContextMenuOption(MENU_CLOSE, IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
+
+  // Default menu items, e.g. "Autohide shelf" are appended. Adding a separator.
+  if (!features::IsTouchableAppContextMenuEnabled())
+    AddSeparator(ui::NORMAL_SEPARATOR);
+}

chrome/browser/ui/ash/launcher/internal_app_shelf_context_menu.h

+// Copyright 2018 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_ASH_LAUNCHER_INTERNAL_APP_SHELF_CONTEXT_MENU_H_
+#define CHROME_BROWSER_UI_ASH_LAUNCHER_INTERNAL_APP_SHELF_CONTEXT_MENU_H_
+
+#include "base/macros.h"
+#include "chrome/browser/ui/ash/launcher/launcher_context_menu.h"
+
+// Class for context menu which is shown for internal app in the shelf.
+class InternalAppShelfContextMenu : public LauncherContextMenu {
+ public:
+  InternalAppShelfContextMenu(ChromeLauncherController* controller,
+                              const ash::ShelfItem* item,
+                              int64_t display_id);
+
+  ~InternalAppShelfContextMenu() override = default;
+
+ private:
+  void Init();
+
+  DISALLOW_COPY_AND_ASSIGN(InternalAppShelfContextMenu);
+};
+
+#endif  // CHROME_BROWSER_UI_ASH_LAUNCHER_INTERNAL_APP_SHELF_CONTEXT_MENU_H_

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

@@ -13,11 +13,13 @@
 #include "chrome/browser/chromeos/crostini/crostini_registry_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
+#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "chrome/browser/ui/ash/launcher/arc_launcher_context_menu.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller_util.h"
 #include "chrome/browser/ui/ash/launcher/crostini_shelf_context_menu.h"
 #include "chrome/browser/ui/ash/launcher/extension_launcher_context_menu.h"
+#include "chrome/browser/ui/ash/launcher/internal_app_shelf_context_menu.h"
 #include "chrome/browser/ui/ash/tablet_mode_client.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/ui_base_features.h"
@@ -48,6 +50,11 @@ std::unique_ptr<LauncherContextMenu> LauncherContextMenu::Create(
                                                       display_id);
   }
 
+  if (app_list::IsInternalApp(item->id.app_id)) {
+    return std::make_unique<InternalAppShelfContextMenu>(controller, item,
+                                                         display_id);
+  }
+
   // Create an ExtensionLauncherContextMenu for other items.
   return std::make_unique<ExtensionLauncherContextMenu>(controller, item,
                                                         display_id);

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

@@ -19,12 +19,14 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_test.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
+#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "chrome/browser/ui/ash/fake_tablet_mode_controller.h"
 #include "chrome/browser/ui/ash/launcher/arc_app_shelf_id.h"
 #include "chrome/browser/ui/ash/launcher/arc_launcher_context_menu.h"
 #include "chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/extension_launcher_context_menu.h"
+#include "chrome/browser/ui/ash/launcher/internal_app_shelf_context_menu.h"
 #include "chrome/browser/ui/ash/tablet_mode_client.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/arc/test/fake_app_instance.h"
@@ -32,6 +34,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/session_manager/core/session_manager.h"
 #include "ui/aura/window_event_dispatcher.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/display/display.h"
 #include "ui/views/widget/widget.h"
 
@@ -411,4 +414,56 @@ TEST_F(LauncherContextMenuTest, ArcContextMenuOptions) {
   EXPECT_EQ(4, menu->GetItemCount());
 }
 
+// Tests that the context menu of internal app  is correct.
+TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenu) {
+  for (const auto& internal_app : app_list::GetInternalAppList()) {
+    if (!internal_app.show_in_launcher)
+      continue;
+
+    const std::string app_id = internal_app.app_id;
+    const ash::ShelfID shelf_id(app_id);
+    // Pin internal app.
+    controller()->PinAppWithID(app_id);
+    const ash::ShelfItem* item = controller()->GetItem(ash::ShelfID(app_id));
+    ASSERT_TRUE(item);
+    EXPECT_EQ(l10n_util::GetStringUTF16(internal_app.name_string_resource_id),
+              item->title);
+    ash::ShelfItemDelegate* item_delegate =
+        model()->GetShelfItemDelegate(shelf_id);
+    ASSERT_TRUE(item_delegate);
+
+    const int64_t display_id = GetPrimaryDisplay().id();
+    std::unique_ptr<ui::MenuModel> menu =
+        GetContextMenu(item_delegate, display_id);
+    ASSERT_TRUE(menu);
+
+    // Internal app is pinned but not running.
+    EXPECT_TRUE(
+        IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_OPEN_NEW));
+    EXPECT_TRUE(IsItemEnabledInMenu(menu.get(), LauncherContextMenu::MENU_PIN));
+    EXPECT_FALSE(
+        IsItemPresentInMenu(menu.get(), LauncherContextMenu::MENU_CLOSE));
+  }
+}
+
+// Tests that the number of context menu options of internal app is correct.
+TEST_F(LauncherContextMenuTest, InternalAppShelfContextMenuOptionsNumber) {
+  for (const auto& internal_app : app_list::GetInternalAppList()) {
+    const std::string app_id = internal_app.app_id;
+    const ash::ShelfID shelf_id(app_id);
+    // Pin internal app.
+    controller()->PinAppWithID(app_id);
+    const ash::ShelfItem* item = controller()->GetItem(ash::ShelfID(app_id));
+    ASSERT_TRUE(item);
+
+    int64_t primary_id = GetPrimaryDisplay().id();
+    std::unique_ptr<LauncherContextMenu> menu =
+        std::make_unique<InternalAppShelfContextMenu>(controller(), item,
+                                                      primary_id);
+
+    const int expected_options_num = internal_app.show_in_launcher ? 4 : 2;
+    EXPECT_EQ(expected_options_num, menu->GetItemCount());
+  }
+}
+
 }  // namespace