Commit 05a36dae authored by Xiyuan Xia's avatar Xiyuan Xia Committed by Commit Bot

app_list: Pass ui::SimpleMenuModel instead of mojo menus.

Use ui::SimpleMenuModel instead of mojo menus in AppListClient
and relevant app context menu code.

LauncherContextMenu (context menu of shelf items) is unchanged
and still uses mojo menus and ui::MenuModel. Deferring that
since it might change with shelf mojo changes.

Bug: 958134, 958208
Change-Id: I0e798f3b9fd6f7b23b604ff2f421bda8dd9e1485
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1616161Reviewed-by: default avatarAlex Newcomer <newcomer@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660876}
parent e26f73bb
......@@ -13,7 +13,6 @@
#include "ash/public/cpp/ash_public_export.h"
#include "ash/public/interfaces/app_list.mojom.h"
#include "ash/public/interfaces/app_list_view.mojom.h"
#include "ash/public/interfaces/menu.mojom.h"
#include "base/callback_forward.h"
#include "base/strings/string16.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
......@@ -25,6 +24,7 @@
namespace ui {
class GestureEvent;
class SimpleMenuModel;
} // namespace ui
namespace app_list {
......@@ -88,7 +88,7 @@ class ASH_PUBLIC_EXPORT AppListViewDelegate {
// or NULL if there is currently no menu for the result.
// Note the returned menu model is owned by that result.
using GetContextMenuModelCallback =
base::OnceCallback<void(std::vector<ash::mojom::MenuItemPtr>)>;
base::OnceCallback<void(std::unique_ptr<ui::SimpleMenuModel>)>;
virtual void GetSearchResultContextMenuModel(
const std::string& result_id,
GetContextMenuModelCallback callback) = 0;
......
......@@ -46,15 +46,13 @@ void AppListTestModel::AppListTestItem::Activate(int event_flags) {
model_->ItemActivated(this);
}
ui::MenuModel* AppListTestModel::AppListTestItem::GetContextMenuModel() {
if (menu_model_)
return menu_model_.get();
menu_model_ = std::make_unique<ui::SimpleMenuModel>(
std::unique_ptr<ui::SimpleMenuModel>
AppListTestModel::AppListTestItem::CreateContextMenuModel() {
auto menu_model = std::make_unique<ui::SimpleMenuModel>(
nullptr /*no SimpleMenuModelDelegate for tests*/);
menu_model_->AddItem(0, base::ASCIIToUTF16("0"));
menu_model_->AddItem(1, base::ASCIIToUTF16("1"));
return menu_model_.get();
menu_model->AddItem(0, base::ASCIIToUTF16("0"));
menu_model->AddItem(1, base::ASCIIToUTF16("1"));
return menu_model;
}
const char* AppListTestModel::AppListTestItem::GetItemType() const {
......
......@@ -14,7 +14,6 @@
#include "base/macros.h"
namespace ui {
class MenuModel;
class SimpleMenuModel;
} // namespace ui
......@@ -30,16 +29,13 @@ class AppListTestModel : public AppListModel {
AppListTestItem(const std::string& id, AppListTestModel* model);
~AppListTestItem() override;
void Activate(int event_flags);
ui::MenuModel* GetContextMenuModel();
std::unique_ptr<ui::SimpleMenuModel> CreateContextMenuModel();
const char* GetItemType() const override;
void SetPosition(const syncer::StringOrdinal& new_position);
private:
AppListTestModel* model_;
// The menu that holds context menu options.
std::unique_ptr<ui::SimpleMenuModel> menu_model_;
AppListTestModel* const model_;
DISALLOW_COPY_AND_ASSIGN(AppListTestItem);
};
......
......@@ -11,7 +11,6 @@
#include "ash/app_list/model/app_list_model.h"
#include "ash/public/cpp/app_list/app_list_features.h"
#include "ash/public/cpp/app_list/app_list_switches.h"
#include "ash/public/cpp/menu_utils.h"
#include "base/callback.h"
#include "base/files/file_path.h"
#include "base/strings/utf_string_conversions.h"
......@@ -22,8 +21,7 @@ namespace test {
AppListTestViewDelegate::AppListTestViewDelegate()
: model_(std::make_unique<AppListTestModel>()),
search_model_(std::make_unique<SearchModel>()),
search_result_context_menu_model_(this) {}
search_model_(std::make_unique<SearchModel>()) {}
AppListTestViewDelegate::~AppListTestViewDelegate() {}
......@@ -102,12 +100,12 @@ void AppListTestViewDelegate::GetContextMenuModel(
GetContextMenuModelCallback callback) {
app_list::AppListItem* item = model_->FindItem(id);
// TODO(stevenjb/jennyz): Implement this for folder items
ui::MenuModel* menu = nullptr;
std::unique_ptr<ui::SimpleMenuModel> menu_model;
if (item && !item->is_folder()) {
menu = static_cast<AppListTestModel::AppListTestItem*>(item)
->GetContextMenuModel();
menu_model = static_cast<AppListTestModel::AppListTestItem*>(item)
->CreateContextMenuModel();
}
std::move(callback).Run(ash::menu_utils::GetMojoMenuItemsFromModel(menu));
std::move(callback).Run(std::move(menu_model));
}
void AppListTestViewDelegate::ShowWallpaperContextMenu(
......@@ -134,13 +132,12 @@ void AppListTestViewDelegate::GetNavigableContentsFactory(
void AppListTestViewDelegate::GetSearchResultContextMenuModel(
const std::string& result_id,
GetContextMenuModelCallback callback) {
ui::SimpleMenuModel* menu = &search_result_context_menu_model_;
menu->Clear();
auto menu = std::make_unique<ui::SimpleMenuModel>(this);
// Change items if needed.
int command_id = 0;
menu->AddItem(command_id++, base::ASCIIToUTF16("Item0"));
menu->AddItem(command_id++, base::ASCIIToUTF16("Item1"));
std::move(callback).Run(ash::menu_utils::GetMojoMenuItemsFromModel(menu));
std::move(callback).Run(std::move(menu));
}
ash::AssistantViewDelegate*
......
......@@ -138,7 +138,6 @@ class AppListTestViewDelegate : public AppListViewDelegate,
std::unique_ptr<AppListTestModel> model_;
std::unique_ptr<SearchModel> search_model_;
std::vector<SkColor> wallpaper_prominent_colors_;
ui::SimpleMenuModel search_result_context_menu_model_;
content::FakeNavigableContentsFactory fake_navigable_contents_factory_;
DISALLOW_COPY_AND_ASSIGN(AppListTestViewDelegate);
......
......@@ -14,14 +14,14 @@ TestAppListClient::~TestAppListClient() = default;
void TestAppListClient::GetSearchResultContextMenuModel(
const std::string& result_id,
GetContextMenuModelCallback callback) {
std::move(callback).Run({});
std::move(callback).Run(nullptr);
}
void TestAppListClient::GetContextMenuModel(
int profile_id,
const std::string& id,
GetContextMenuModelCallback callback) {
std::move(callback).Run({});
std::move(callback).Run(nullptr);
}
} // namespace ash
......@@ -448,9 +448,9 @@ void AppListItemView::SetItemPercentDownloaded(int percent_downloaded) {
void AppListItemView::OnContextMenuModelReceived(
const gfx::Point& point,
ui::MenuSourceType source_type,
std::vector<ash::mojom::MenuItemPtr> menu) {
std::unique_ptr<ui::SimpleMenuModel> menu_model) {
waiting_for_context_menu_options_ = false;
if (menu.empty() || (context_menu_ && context_menu_->IsShowingMenu()))
if (!menu_model || (context_menu_ && context_menu_->IsShowingMenu()))
return;
// GetContextMenuModel is asynchronous and takes a nontrivial amount of time
......@@ -480,12 +480,11 @@ void AppListItemView::OnContextMenuModelReceived(
views::View::ConvertRectToScreen(apps_grid_view_, &anchor_rect);
context_menu_ = std::make_unique<AppListMenuModelAdapter>(
item_weak_->GetMetadata()->id, GetWidget(), source_type, this,
AppListMenuModelAdapter::FULLSCREEN_APP_GRID,
item_weak_->GetMetadata()->id, std::move(menu_model), GetWidget(),
source_type, this, AppListMenuModelAdapter::FULLSCREEN_APP_GRID,
base::BindOnce(&AppListItemView::OnMenuClosed,
weak_ptr_factory_.GetWeakPtr()),
apps_grid_view_->IsTabletMode());
context_menu_->Build(std::move(menu));
context_menu_->Run(anchor_rect, views::MenuAnchorPosition::kBubbleRight,
run_types);
apps_grid_view_->SetSelectedView(this);
......
......@@ -13,7 +13,6 @@
#include "ash/app_list/app_list_export.h"
#include "ash/app_list/model/app_list_item_observer.h"
#include "ash/app_list/views/app_list_menu_model_adapter.h"
#include "ash/public/interfaces/menu.mojom.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/strings/string16.h"
......@@ -169,9 +168,10 @@ class APP_LIST_EXPORT AppListItemView
// Callback invoked when a context menu is received after calling
// |AppListViewDelegate::GetContextMenuModel|.
void OnContextMenuModelReceived(const gfx::Point& point,
ui::MenuSourceType source_type,
std::vector<ash::mojom::MenuItemPtr> menu);
void OnContextMenuModelReceived(
const gfx::Point& point,
ui::MenuSourceType source_type,
std::unique_ptr<ui::SimpleMenuModel> menu_model);
// views::ContextMenuController overrides:
void ShowContextMenuForViewImpl(views::View* source,
......
......@@ -7,7 +7,6 @@
#include <utility>
#include "ash/public/cpp/app_menu_constants.h"
#include "ash/public/cpp/menu_utils.h"
#include "base/metrics/histogram_macros.h"
#include "ui/views/controls/menu/menu_runner.h"
......@@ -15,6 +14,7 @@ namespace app_list {
AppListMenuModelAdapter::AppListMenuModelAdapter(
const std::string& app_id,
std::unique_ptr<ui::SimpleMenuModel> menu_model,
views::Widget* widget_owner,
ui::MenuSourceType source_type,
Delegate* delegate,
......@@ -22,7 +22,7 @@ AppListMenuModelAdapter::AppListMenuModelAdapter(
base::OnceClosure on_menu_closed_callback,
bool is_tablet_mode)
: ash::AppMenuModelAdapter(app_id,
std::make_unique<ui::SimpleMenuModel>(nullptr),
std::move(menu_model),
widget_owner,
source_type,
std::move(on_menu_closed_callback),
......@@ -35,15 +35,6 @@ AppListMenuModelAdapter::AppListMenuModelAdapter(
AppListMenuModelAdapter::~AppListMenuModelAdapter() = default;
void AppListMenuModelAdapter::Build(
std::vector<ash::mojom::MenuItemPtr> items) {
DCHECK(!items.empty() && !IsShowingMenu());
ash::menu_utils::PopulateMenuFromMojoMenuItems(model(), nullptr, items,
&submenu_models_);
menu_items_ = std::move(items);
}
void AppListMenuModelAdapter::RecordHistogramOnMenuClosed() {
const base::TimeDelta user_journey_time =
base::TimeTicks::Now() - menu_open_time();
......@@ -149,18 +140,14 @@ void AppListMenuModelAdapter::RecordHistogramOnMenuClosed() {
}
}
bool AppListMenuModelAdapter::IsItemChecked(int id) const {
return ash::menu_utils::GetMenuItemByCommandId(menu_items_, id)->checked;
}
bool AppListMenuModelAdapter::IsCommandEnabled(int id) const {
// NOTIFICATION_CONTAINER is always enabled. It is added to this model by
// NotificationMenuController, but it is not added to |menu_items_|, so check
// for it first.
// NotificationMenuController. It is not known by model()'s delegate (i.e.
// an instance of AppContextMenu). Check for it first.
if (id == ash::NOTIFICATION_CONTAINER)
return true;
return ash::menu_utils::GetMenuItemByCommandId(menu_items_, id)->enabled;
return ash::AppMenuModelAdapter::IsCommandEnabled(id);
}
void AppListMenuModelAdapter::ExecuteCommand(int id, int mouse_event_flags) {
......
......@@ -11,7 +11,6 @@
#include "ash/app_list/app_list_export.h"
#include "ash/app_menu/app_menu_model_adapter.h"
#include "ash/public/interfaces/menu.mojom.h"
#include "ui/base/models/simple_menu_model.h"
#include "ui/base/ui_base_types.h"
#include "ui/views/controls/menu/menu_types.h"
......@@ -44,6 +43,7 @@ class APP_LIST_EXPORT AppListMenuModelAdapter
};
AppListMenuModelAdapter(const std::string& app_id,
std::unique_ptr<ui::SimpleMenuModel> menu_model,
views::Widget* widget_owner,
ui::MenuSourceType source_type,
Delegate* delegate,
......@@ -52,14 +52,10 @@ class APP_LIST_EXPORT AppListMenuModelAdapter
bool is_tablet_mode);
~AppListMenuModelAdapter() override;
// Builds the menu model from |items|.
void Build(std::vector<ash::mojom::MenuItemPtr> items);
// Overridden from AppMenuModelAdapter:
void RecordHistogramOnMenuClosed() override;
// Overridden from views::MenuModelAdapter:
bool IsItemChecked(int id) const override;
bool IsCommandEnabled(int id) const override;
void ExecuteCommand(int id, int mouse_event_flags) override;
......@@ -70,10 +66,6 @@ class APP_LIST_EXPORT AppListMenuModelAdapter
// The type of app which is using this object to show a menu.
const AppListViewAppType type_;
// The mojo version of the model of items which are shown in a menu.
std::vector<ash::mojom::MenuItemPtr> menu_items_;
std::vector<std::unique_ptr<ui::MenuModel>> submenu_models_;
DISALLOW_COPY_AND_ASSIGN(AppListMenuModelAdapter);
};
......
......@@ -36,7 +36,8 @@ class AppListMenuModelAdapterTest : public views::ViewsTestBase {
mock_app_list_menu_model_adapter_delegate_ =
std::make_unique<MockAppListMenuModelAdapterDelegate>();
app_list_menu_model_adapter_ = std::make_unique<AppListMenuModelAdapter>(
"test-app-id", nullptr, ui::MenuSourceType::MENU_SOURCE_TYPE_LAST,
"test-app-id", std::make_unique<ui::SimpleMenuModel>(nullptr), nullptr,
ui::MenuSourceType::MENU_SOURCE_TYPE_LAST,
mock_app_list_menu_model_adapter_delegate_.get(),
AppListMenuModelAdapter::FULLSCREEN_APP_GRID, base::OnceClosure(),
false /* is_tablet_mode */);
......
......@@ -354,8 +354,8 @@ void SearchResultTileItemView::OnGetContextMenuModel(
views::View* source,
const gfx::Point& point,
ui::MenuSourceType source_type,
std::vector<ash::mojom::MenuItemPtr> menu) {
if (menu.empty() || (context_menu_ && context_menu_->IsShowingMenu()))
std::unique_ptr<ui::SimpleMenuModel> menu_model) {
if (!menu_model || (context_menu_ && context_menu_->IsShowingMenu()))
return;
gfx::Rect anchor_rect = source->GetBoundsInScreen();
......@@ -363,11 +363,11 @@ void SearchResultTileItemView::OnGetContextMenuModel(
anchor_rect.ClampToCenteredSize(AppListConfig::instance().grid_focus_size());
context_menu_ = std::make_unique<AppListMenuModelAdapter>(
result()->id(), GetWidget(), source_type, this, GetAppType(),
result()->id(), std::move(menu_model), GetWidget(), source_type, this,
GetAppType(),
base::BindOnce(&SearchResultTileItemView::OnMenuClosed,
weak_ptr_factory_.GetWeakPtr()),
view_delegate_->GetSearchModel()->tablet_mode());
context_menu_->Build(std::move(menu));
context_menu_->Run(anchor_rect, views::MenuAnchorPosition::kBubbleRight,
views::MenuRunner::HAS_MNEMONICS |
views::MenuRunner::USE_TOUCHABLE_LAYOUT |
......
......@@ -11,7 +11,6 @@
#include "ash/app_list/app_list_export.h"
#include "ash/app_list/views/app_list_menu_model_adapter.h"
#include "ash/app_list/views/search_result_base_view.h"
#include "ash/public/interfaces/menu.mojom.h"
#include "base/macros.h"
#include "ui/views/context_menu_controller.h"
......@@ -81,7 +80,7 @@ class APP_LIST_EXPORT SearchResultTileItemView
void OnGetContextMenuModel(views::View* source,
const gfx::Point& point,
ui::MenuSourceType source_type,
std::vector<ash::mojom::MenuItemPtr> menu);
std::unique_ptr<ui::SimpleMenuModel> menu_model);
// The callback used when a menu closes.
void OnMenuClosed();
......
......@@ -454,15 +454,14 @@ void SearchResultView::OnGetContextMenu(
views::View* source,
const gfx::Point& point,
ui::MenuSourceType source_type,
std::vector<ash::mojom::MenuItemPtr> menu) {
if (menu.empty() || context_menu_->IsShowingMenu())
std::unique_ptr<ui::SimpleMenuModel> menu_model) {
if (!menu_model || context_menu_->IsShowingMenu())
return;
context_menu_ = std::make_unique<AppListMenuModelAdapter>(
std::string(), GetWidget(), source_type, this,
std::string(), std::move(menu_model), GetWidget(), source_type, this,
AppListMenuModelAdapter::SEARCH_RESULT, base::OnceClosure(),
view_delegate_->GetSearchModel()->tablet_mode());
context_menu_->Build(std::move(menu));
context_menu_->Run(gfx::Rect(point, gfx::Size()),
views::MenuAnchorPosition::kTopLeft,
views::MenuRunner::HAS_MNEMONICS);
......
......@@ -15,7 +15,6 @@
#include "ash/app_list/views/app_list_menu_model_adapter.h"
#include "ash/app_list/views/search_result_actions_view_delegate.h"
#include "ash/app_list/views/search_result_base_view.h"
#include "ash/public/interfaces/menu.mojom.h"
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
......@@ -105,7 +104,7 @@ class APP_LIST_EXPORT SearchResultView
void OnGetContextMenu(views::View* source,
const gfx::Point& point,
ui::MenuSourceType source_type,
std::vector<ash::mojom::MenuItemPtr> menu);
std::unique_ptr<ui::SimpleMenuModel> menu_model);
// SearchResultObserver overrides:
void OnMetadataChanged() override;
......
......@@ -12,12 +12,10 @@
#include "ash/public/cpp/ash_public_export.h"
#include "ash/public/interfaces/app_list.mojom.h"
// TODO(crbug.com/958208): Remove menu.mojom.h
#include "ash/public/interfaces/menu.mojom.h"
#include "base/callback_forward.h"
#include "base/strings/string16.h"
#include "components/sync/model/string_ordinal.h"
#include "ui/base/models/menu_model.h"
#include "ui/base/models/simple_menu_model.h"
namespace app_list {
......@@ -64,7 +62,7 @@ class ASH_PUBLIC_EXPORT AppListClient {
// Returns the context menu model for the search result with |result_id|, or
// an empty array if there is currently no menu for the result.
using GetSearchResultContextMenuModelCallback =
base::OnceCallback<void(std::vector<::ash::mojom::MenuItemPtr>)>;
base::OnceCallback<void(std::unique_ptr<ui::SimpleMenuModel>)>;
virtual void GetSearchResultContextMenuModel(
const std::string& result_id,
GetSearchResultContextMenuModelCallback callback) = 0;
......@@ -98,7 +96,7 @@ class ASH_PUBLIC_EXPORT AppListClient {
// Returns the context menu model for the item with |id|, or an empty array if
// there is currently no menu for the item (e.g. during install).
using GetContextMenuModelCallback =
base::OnceCallback<void(std::vector<::ash::mojom::MenuItemPtr>)>;
base::OnceCallback<void(std::unique_ptr<ui::SimpleMenuModel>)>;
virtual void GetContextMenuModel(int profile_id,
const std::string& id,
GetContextMenuModelCallback callback) = 0;
......
......@@ -60,7 +60,7 @@ class ArcAppShortcutsMenuBuilderTest : public testing::Test {
TEST_F(ArcAppShortcutsMenuBuilderTest, Basic) {
base::RunLoop run_loop;
std::unique_ptr<ui::MenuModel> menu;
std::unique_ptr<ui::SimpleMenuModel> menu;
auto simple_menu_model = std::make_unique<ui::SimpleMenuModel>(nullptr);
const base::string16 first_item_label = base::UTF8ToUTF16("FirstItemLabel");
simple_menu_model->AddItem(1, first_item_label);
......
......@@ -10,7 +10,6 @@
#include <vector>
#include "ash/public/cpp/app_list/app_list_controller.h"
#include "ash/public/cpp/menu_utils.h"
#include "ash/public/interfaces/constants.mojom.h"
#include "base/bind.h"
#include "base/metrics/histogram_functions.h"
......@@ -149,19 +148,18 @@ void AppListClientImpl::GetSearchResultContextMenuModel(
const std::string& result_id,
GetContextMenuModelCallback callback) {
if (!search_controller_) {
std::move(callback).Run(std::vector<ash::mojom::MenuItemPtr>());
std::move(callback).Run(nullptr);
return;
}
ChromeSearchResult* result = search_controller_->FindSearchResult(result_id);
if (!result) {
std::move(callback).Run(std::vector<ash::mojom::MenuItemPtr>());
std::move(callback).Run(nullptr);
return;
}
result->GetContextMenuModel(base::BindOnce(
[](GetContextMenuModelCallback callback,
std::unique_ptr<ui::SimpleMenuModel> menu_model) {
std::move(callback).Run(
ash::menu_utils::GetMojoMenuItemsFromModel(menu_model.get()));
std::move(callback).Run(std::move(menu_model));
},
std::move(callback)));
}
......@@ -226,18 +224,16 @@ void AppListClientImpl::GetContextMenuModel(
auto* requested_model_updater = profile_model_mappings_[profile_id];
if (requested_model_updater != current_model_updater_ ||
!requested_model_updater) {
std::move(callback).Run(std::vector<ash::mojom::MenuItemPtr>());
std::move(callback).Run(nullptr);
return;
}
requested_model_updater->GetContextMenuModel(
id,
base::BindOnce(
[](GetContextMenuModelCallback callback,
std::unique_ptr<ui::SimpleMenuModel> menu_model) {
std::move(callback).Run(
ash::menu_utils::GetMojoMenuItemsFromModel(menu_model.get()));
},
std::move(callback)));
id, base::BindOnce(
[](GetContextMenuModelCallback callback,
std::unique_ptr<ui::SimpleMenuModel> menu_model) {
std::move(callback).Run(std::move(menu_model));
},
std::move(callback)));
}
void AppListClientImpl::ContextMenuItemSelected(int profile_id,
......
......@@ -53,7 +53,7 @@
#include "extensions/browser/extension_prefs.h"
#include "extensions/common/constants.h"
#include "ui/aura/window.h"
#include "ui/base/models/menu_model.h"
#include "ui/base/models/simple_menu_model.h"
// Browser Test for AppListClientImpl.
using AppListClientImplBrowserTest = extensions::PlatformAppBrowserTest;
......
......@@ -14,7 +14,7 @@
#include "ash/public/interfaces/app_list.mojom.h"
#include "base/macros.h"
#include "chrome/browser/ui/app_list/app_list_model_updater.h"
#include "ui/base/models/menu_model.h"
#include "ui/base/models/simple_menu_model.h"
namespace app_list {
class AppContextMenu;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment