Commit 4380e3b0 authored by Christopher Lam's avatar Christopher Lam Committed by Commit Bot

[desktop-pwas] Add a custom menu model for the Hosted App Menu Button.

This CL adds a custom menu model for hosted app which has only the items
that should be shown for a hosted app. It also adds 3 new commands which
are currently disabled and will be implemented in a future CL.

Bug: 762401
Change-Id: I7e53eb3d11042f267567e3773784c8594d7c7bee
TBR: erg@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/721226
Commit-Queue: calamity <calamity@chromium.org>
Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513058}
parent dfd274eb
......@@ -71,6 +71,11 @@
#define IDC_USE_SYSTEM_TITLE_BAR 34051
#endif
// Hosted app commands
#define IDC_COPY_URL 34060
#define IDC_OPEN_IN_CHROME 34061
#define IDC_SITE_SETTINGS 34062
// Page-related commands
#define IDC_BOOKMARK_PAGE 35000
#define IDC_BOOKMARK_ALL_TABS 35001
......
......@@ -606,6 +606,10 @@ Chromium is unable to recover your settings.
Customize and control Chromium
</message>
<message name="IDS_OPEN_IN_CHROME" desc="The text label of the Open in Chrome menu item for the Hosted App app menu">
&amp;Open in Chromium
</message>
<if expr="use_titlecase and not chromeos">
<message name="IDS_ABOUT" desc="In Title Case: The text label of the About Chrome menu item">
About &amp;Chromium
......
......@@ -867,6 +867,12 @@ are declared in build/common.gypi.
<message name="IDS_ZOOM_MINUS2" desc="The text label of the Make Text Smaller menu item in the merged menu">
&#8722;
</message>
<message name="IDS_COPY_URL" desc="The text label of the Copy URL menu item for the Hosted App app menu">
Copy &amp;URL
</message>
<message name="IDS_SITE_SETTINGS" desc="The text label of the Site Settings menu item for the Hosted App app menu">
&amp;Site settings
</message>
</if>
<if expr="use_titlecase">
<message name="IDS_NEW_TAB" desc="In Title Case: The text label of a menu item for opening a new tab">
......@@ -932,6 +938,12 @@ are declared in build/common.gypi.
<message name="IDS_ZOOM_MINUS2" desc="The text label of the Make Text Smaller menu item in the merged menu">
&#8722;
</message>
<message name="IDS_COPY_URL" desc="In Title Case: The text label of the Copy URL menu item for the Hosted App app menu">
Copy &amp;URL
</message>
<message name="IDS_SITE_SETTINGS" desc="In Title Case: The text label of the Site Settings menu item for the Hosted App app menu">
&amp;Site Settings
</message>
</if>
</if>
......
......@@ -616,6 +616,10 @@ Google Chrome is unable to recover your settings.
Customize and control Google Chrome
</message>
<message name="IDS_OPEN_IN_CHROME" desc="The text label of the Open in Chrome menu item for the Hosted App app menu">
&amp;Open in Chrome
</message>
<if expr="use_titlecase and not chromeos">
<message name="IDS_ABOUT" desc="In Title Case: The text label of the About Chrome menu item">
About &amp;Google Chrome
......
......@@ -225,6 +225,7 @@ IN_PROC_BROWSER_TEST_F(ProfileWindowBrowserTest, GuestAppMenuLacksBookmarks) {
EmptyAcceleratorHandler accelerator_handler;
// Verify the normal browser has a bookmark menu.
AppMenuModel model_normal_profile(&accelerator_handler, browser());
model_normal_profile.Init();
EXPECT_NE(-1, model_normal_profile.GetIndexOfCommandId(IDC_BOOKMARKS_MENU));
// Guest browser has no bookmark menu.
......
......@@ -3421,6 +3421,8 @@ split_static_library("ui") {
"extensions/extension_message_bubble_factory.h",
"extensions/hosted_app_browser_controller.cc",
"extensions/hosted_app_browser_controller.h",
"extensions/hosted_app_menu_model.cc",
"extensions/hosted_app_menu_model.h",
"extensions/icon_with_badge_image_source.cc",
"extensions/icon_with_badge_image_source.h",
"extensions/settings_api_bubble_helpers.cc",
......
......@@ -481,6 +481,7 @@ class ToolbarActionsBarObserverHelper : public ToolbarActionsBarObserver {
DCHECK(browser_);
recentTabsMenuModelDelegate_.reset();
appMenuModel_.reset(new AppMenuModel(acceleratorDelegate_.get(), browser_));
appMenuModel_->Init();
[self setModel:appMenuModel_.get()];
buttonViewController_.reset(
......
......@@ -51,7 +51,7 @@ namespace {
class MockAppMenuModel : public AppMenuModel {
public:
MockAppMenuModel() : AppMenuModel() {}
MockAppMenuModel() : AppMenuModel(nullptr, nullptr) {}
~MockAppMenuModel() {}
MOCK_METHOD2(ExecuteCommand, void(int command_id, int event_flags));
};
......
// Copyright 2017 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/extensions/hosted_app_menu_model.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/media/router/media_router_feature.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.h"
#include "components/strings/grit/components_strings.h"
HostedAppMenuModel::HostedAppMenuModel(ui::AcceleratorProvider* provider,
Browser* browser)
: AppMenuModel(provider, browser) {}
HostedAppMenuModel::~HostedAppMenuModel() {}
void HostedAppMenuModel::Build() {
AddItemWithStringId(IDC_COPY_URL, IDS_COPY_URL);
AddItemWithStringId(IDC_OPEN_IN_CHROME, IDS_OPEN_IN_CHROME);
CreateActionToolbarOverflowMenu();
CreateZoomMenu();
AddItemWithStringId(IDC_PRINT, IDS_PRINT);
AddItemWithStringId(IDC_FIND, IDS_FIND);
if (media_router::MediaRouterEnabled(browser()->profile()))
AddItemWithStringId(IDC_ROUTE_MEDIA, IDS_MEDIA_ROUTER_MENU_ITEM_TITLE);
CreateCutCopyPasteMenu();
AddItemWithStringId(IDC_SITE_SETTINGS, IDS_SITE_SETTINGS);
}
// Copyright 2017 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_EXTENSIONS_HOSTED_APP_MENU_MODEL_H_
#define CHROME_BROWSER_UI_EXTENSIONS_HOSTED_APP_MENU_MODEL_H_
#include "base/macros.h"
#include "chrome/browser/ui/toolbar/app_menu_model.h"
// Menu model for the menu button in a hosted app browser window.
class HostedAppMenuModel : public AppMenuModel {
public:
HostedAppMenuModel(ui::AcceleratorProvider* provider, Browser* browser);
~HostedAppMenuModel() override;
private:
// AppMenuModel:
void Build() override;
DISALLOW_COPY_AND_ASSIGN(HostedAppMenuModel);
};
#endif // CHROME_BROWSER_UI_EXTENSIONS_HOSTED_APP_MENU_MODEL_H_
......@@ -216,12 +216,19 @@ AppMenuModel::AppMenuModel(ui::AcceleratorProvider* provider, Browser* browser)
: ui::SimpleMenuModel(this),
uma_action_recorded_(false),
provider_(provider),
browser_(browser) {
browser_(browser) {}
AppMenuModel::~AppMenuModel() {
if (browser_) // Null in Cocoa tests.
browser_->tab_strip_model()->RemoveObserver(this);
}
void AppMenuModel::Init() {
Build();
UpdateZoomControls();
browser_zoom_subscription_ =
zoom::ZoomEventManager::GetForBrowserContext(browser->profile())
zoom::ZoomEventManager::GetForBrowserContext(browser_->profile())
->AddZoomLevelChangedCallback(base::Bind(
&AppMenuModel::OnZoomLevelChanged, base::Unretained(this)));
......@@ -231,11 +238,6 @@ AppMenuModel::AppMenuModel(ui::AcceleratorProvider* provider, Browser* browser)
content::NotificationService::AllSources());
}
AppMenuModel::~AppMenuModel() {
if (browser_) // Null in tests.
browser_->tab_strip_model()->RemoveObserver(this);
}
bool AppMenuModel::DoesCommandIdDismissMenu(int command_id) const {
return command_id != IDC_ZOOM_MINUS && command_id != IDC_ZOOM_PLUS;
}
......@@ -661,21 +663,6 @@ void AppMenuModel::Observe(int type,
UpdateZoomControls();
}
// For testing.
AppMenuModel::AppMenuModel()
: ui::SimpleMenuModel(this),
uma_action_recorded_(false),
provider_(nullptr),
browser_(nullptr) {}
bool AppMenuModel::ShouldShowNewIncognitoWindowMenuItem() {
if (browser_->profile()->IsGuestSession())
return false;
return IncognitoModePrefs::GetAvailability(browser_->profile()->GetPrefs()) !=
IncognitoModePrefs::DISABLED;
}
// Note: When adding new menu items please place under an appropriate section.
// Menu is organised as follows:
// - Extension toolbar overflow.
......@@ -766,32 +753,6 @@ void AppMenuModel::Build() {
uma_action_recorded_ = false;
}
bool AppMenuModel::AddGlobalErrorMenuItems() {
// TODO(sail): Currently we only build the app menu once per browser
// window. This means that if a new error is added after the menu is built
// it won't show in the existing app menu. To fix this we need to some
// how update the menu if new errors are added.
const GlobalErrorService::GlobalErrorList& errors =
GlobalErrorServiceFactory::GetForProfile(browser_->profile())->errors();
bool menu_items_added = false;
for (GlobalErrorService::GlobalErrorList::const_iterator
it = errors.begin(); it != errors.end(); ++it) {
GlobalError* error = *it;
DCHECK(error);
if (error->HasMenuItem()) {
AddItem(error->MenuItemCommandID(), error->MenuItemLabel());
SetIcon(GetIndexOfCommandId(error->MenuItemCommandID()),
error->MenuItemIcon());
menu_items_added = true;
if (IDC_SHOW_SIGNIN_ERROR == error->MenuItemCommandID()) {
base::RecordAction(
base::UserMetricsAction("Signin_Impression_FromMenu"));
}
}
}
return menu_items_added;
}
void AppMenuModel::CreateActionToolbarOverflowMenu() {
// We only add the extensions overflow container if there are any icons that
// aren't shown in the main container.
......@@ -858,6 +819,40 @@ void AppMenuModel::UpdateZoomControls() {
zoom_label_ = base::FormatPercent(zoom_percent);
}
bool AppMenuModel::ShouldShowNewIncognitoWindowMenuItem() {
if (browser_->profile()->IsGuestSession())
return false;
return IncognitoModePrefs::GetAvailability(browser_->profile()->GetPrefs()) !=
IncognitoModePrefs::DISABLED;
}
bool AppMenuModel::AddGlobalErrorMenuItems() {
// TODO(sail): Currently we only build the app menu once per browser
// window. This means that if a new error is added after the menu is built
// it won't show in the existing app menu. To fix this we need to some
// how update the menu if new errors are added.
const GlobalErrorService::GlobalErrorList& errors =
GlobalErrorServiceFactory::GetForProfile(browser_->profile())->errors();
bool menu_items_added = false;
for (GlobalErrorService::GlobalErrorList::const_iterator it = errors.begin();
it != errors.end(); ++it) {
GlobalError* error = *it;
DCHECK(error);
if (error->HasMenuItem()) {
AddItem(error->MenuItemCommandID(), error->MenuItemLabel());
SetIcon(GetIndexOfCommandId(error->MenuItemCommandID()),
error->MenuItemIcon());
menu_items_added = true;
if (IDC_SHOW_SIGNIN_ERROR == error->MenuItemCommandID()) {
base::RecordAction(
base::UserMetricsAction("Signin_Impression_FromMenu"));
}
}
}
return menu_items_added;
}
void AppMenuModel::OnZoomLevelChanged(
const content::HostZoomMap::ZoomLevelChange& change) {
UpdateZoomControls();
......
......@@ -109,9 +109,14 @@ class AppMenuModel : public ui::SimpleMenuModel,
static const int kMinRecentTabsCommandId = 1001;
static const int kMaxRecentTabsCommandId = 1200;
// Creates an app menu model for the given browser. Init() must be called
// before passing this to an AppMenu.
AppMenuModel(ui::AcceleratorProvider* provider, Browser* browser);
~AppMenuModel() override;
// Runs Build() and registers observers.
void Init();
// Overridden for ButtonMenuItemModel::Delegate:
bool DoesCommandIdDismissMenu(int command_id) const override;
......@@ -151,19 +156,9 @@ class AppMenuModel : public ui::SimpleMenuModel,
// Calculates |zoom_label_| in response to a zoom change.
void UpdateZoomControls();
private:
class HelpMenuModel;
// Testing constructor used for mocking.
friend class ::MockAppMenuModel;
AppMenuModel();
void Build();
// Adds actionable global error menu items to the menu.
// Examples: Extension permissions and sign in errors.
// Returns a boolean indicating whether any menu items were added.
bool AddGlobalErrorMenuItems();
protected:
// Builds the menu model, adding appropriate menu items.
virtual void Build();
// Appends everything needed for the clipboard menu: a menu break, the
// clipboard menu content and the finalizing menu break.
......@@ -176,10 +171,19 @@ class AppMenuModel : public ui::SimpleMenuModel,
// menu content and then another menu break.
void CreateZoomMenu();
void OnZoomLevelChanged(const content::HostZoomMap::ZoomLevelChange& change);
private:
class HelpMenuModel;
friend class ::MockAppMenuModel;
bool ShouldShowNewIncognitoWindowMenuItem();
// Adds actionable global error menu items to the menu.
// Examples: Extension permissions and sign in errors.
// Returns a boolean indicating whether any menu items were added.
bool AddGlobalErrorMenuItems();
void OnZoomLevelChanged(const content::HostZoomMap::ZoomLevelChange& change);
// Called when a command is selected.
// Logs UMA metrics about which command was chosen and how long the user
// took to select the command.
......
......@@ -116,6 +116,7 @@ class TestAppMenuModel : public AppMenuModel {
TEST_F(AppMenuModelTest, Basics) {
TestAppMenuModel model(this, browser());
model.Init();
int itemCount = model.GetItemCount();
// Verify it has items. The number varies by platform, so we don't check
......@@ -182,6 +183,7 @@ TEST_F(AppMenuModelTest, GlobalError) {
service->AddGlobalError(base::WrapUnique(error2));
AppMenuModel model(this, browser());
model.Init();
int index1 = model.GetIndexOfCommandId(command1);
EXPECT_GT(index1, -1);
int index2 = model.GetIndexOfCommandId(command2);
......
......@@ -5,7 +5,7 @@
#include "chrome/browser/ui/views/frame/hosted_app_button_container.h"
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/ui/toolbar/app_menu_model.h"
#include "chrome/browser/ui/extensions/hosted_app_menu_model.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/toolbar/app_menu.h"
#include "ui/gfx/color_palette.h"
......@@ -31,9 +31,9 @@ void HostedAppButtonContainer::AppMenuButton::OnMenuButtonClicked(
const gfx::Point& point,
const ui::Event* event) {
Browser* browser = browser_view_->browser();
menu_.reset(new AppMenu(browser, 0));
// TODO(calamity): Use custom menu model here.
menu_model_.reset(new AppMenuModel(browser_view_, browser));
menu_ = std::make_unique<AppMenu>(browser, 0);
menu_model_ = std::make_unique<HostedAppMenuModel>(browser_view_, browser);
menu_model_->Init();
menu_->Init(menu_model_.get());
menu_->RunMenu(this);
......
......@@ -13,7 +13,7 @@
#include "ui/views/view.h"
class AppMenu;
class AppMenuModel;
class HostedAppMenuModel;
class BrowserView;
// A container for hosted app buttons in the title bar.
......@@ -48,7 +48,7 @@ class HostedAppButtonContainer : public views::View {
// App model and menu.
// Note that the menu should be destroyed before the model it uses, so the
// menu should be listed later.
std::unique_ptr<AppMenuModel> menu_model_;
std::unique_ptr<HostedAppMenuModel> menu_model_;
std::unique_ptr<AppMenu> menu_;
DISALLOW_COPY_AND_ASSIGN(AppMenuButton);
......
......@@ -94,6 +94,7 @@ void AppMenuButton::ShowMenu(bool for_drop) {
menu_.reset(new AppMenu(browser, for_drop ? AppMenu::FOR_DROP : 0));
menu_model_.reset(new AppMenuModel(toolbar_view_, browser));
menu_model_->Init();
menu_->Init(menu_model_.get());
for (views::MenuListener& observer : menu_listeners_)
......
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