Commit 679f10eb authored by Christopher Lam's avatar Christopher Lam Committed by Commit Bot

[System Web Apps] Remove context menu options from Launcher and Shelf.

This CL removes the 'Open as tab/window' and 'App Info' options for
System PWAs in the App Launcher and Shelf.

Bug: 836128
Change-Id: I80dd5046d49e709c766a893d581298eb176e4bdc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1557478Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarAlexey Baskakov <loyso@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649771}
parent f8764798
......@@ -12,6 +12,8 @@
#include "chrome/browser/profiles/profile.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/web_applications/system_web_app_manager.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.h"
#include "content/public/common/context_menu_params.h"
......@@ -86,8 +88,12 @@ void ExtensionAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
profile(), this, menu_model,
base::Bind(MenuItemHasLauncherContext));
bool is_system_web_app = web_app::WebAppProvider::Get(profile())
->system_web_app_manager()
.IsSystemWebApp(app_id());
// First, add the primary actions.
if (!is_platform_app_)
if (!is_platform_app_ && !is_system_web_app)
CreateOpenNewSubmenu(menu_model);
// Create default items.
......@@ -107,7 +113,7 @@ void ExtensionAppContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
is_platform_app_ ? IDS_APP_LIST_UNINSTALL_ITEM
: IDS_APP_LIST_EXTENSIONS_UNINSTALL);
if (controller()->CanDoShowAppInfoFlow()) {
if (controller()->CanDoShowAppInfoFlow() && !is_system_web_app) {
AddContextMenuOption(menu_model, ash::SHOW_APP_INFO,
IDS_APP_CONTEXT_MENU_SHOW_INFO);
}
......
......@@ -17,6 +17,8 @@
#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/browser_commands.h"
#include "chrome/browser/web_applications/system_web_app_manager.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/common/extensions/extension_constants.h"
#include "chrome/grit/generated_resources.h"
#include "content/public/common/context_menu_params.h"
......@@ -170,12 +172,17 @@ void ExtensionLauncherContextMenu::CreateOpenNewSubmenu(
}
void ExtensionLauncherContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
Profile* profile = controller()->profile();
extension_items_.reset(new extensions::ContextMenuMatcher(
controller()->profile(), this, menu_model,
base::Bind(MenuItemHasLauncherContext)));
profile, this, menu_model,
base::BindRepeating(MenuItemHasLauncherContext)));
if (item().type == ash::TYPE_PINNED_APP || item().type == ash::TYPE_APP) {
// V1 apps can be started from the menu - but V2 apps should not.
if (!controller()->IsPlatformApp(item().id))
// V1 apps can be started from the menu - but V2 apps and system web apps
// should not.
bool is_system_web_app = web_app::WebAppProvider::Get(profile)
->system_web_app_manager()
.IsSystemWebApp(item().id.app_id);
if (!controller()->IsPlatformApp(item().id) && !is_system_web_app)
CreateOpenNewSubmenu(menu_model);
AddPinMenu(menu_model);
......@@ -186,7 +193,7 @@ void ExtensionLauncherContextMenu::BuildMenu(ui::SimpleMenuModel* menu_model) {
} else if (item().type == ash::TYPE_BROWSER_SHORTCUT) {
AddContextMenuOption(menu_model, ash::MENU_NEW_WINDOW,
IDS_APP_LIST_NEW_WINDOW);
if (!controller()->profile()->IsGuestSession()) {
if (!profile->IsGuestSession()) {
AddContextMenuOption(menu_model, ash::MENU_NEW_INCOGNITO_WINDOW,
IDS_APP_LIST_NEW_INCOGNITO_WINDOW);
}
......
......@@ -196,6 +196,9 @@ IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTest, Install) {
WebAppProvider::Get(browser()->profile())
->system_web_app_manager()
.GetAppIdForSystemApp(web_app::SystemAppType::SETTINGS));
EXPECT_TRUE(WebAppProvider::Get(browser()->profile())
->system_web_app_manager()
.IsSystemWebApp(app->id()));
}
} // namespace web_app
......@@ -14,6 +14,7 @@
#include "base/containers/flat_map.h"
#include "base/macros.h"
#include "chrome/browser/web_applications/components/install_options.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "url/gurl.h"
namespace web_app {
......@@ -104,7 +105,13 @@ class PendingAppManager {
SynchronizeCallback callback);
// Returns the app id for |url| if the PendingAppManager is aware of it.
virtual base::Optional<std::string> LookupAppId(const GURL& url) const = 0;
virtual base::Optional<AppId> LookupAppId(const GURL& url) const = 0;
// Returns whether the PendingAppManager has installed an app with |app_id|
// from |install_source|.
virtual bool HasAppIdWithInstallSource(
const AppId& app_id,
web_app::InstallSource install_source) const = 0;
private:
struct SynchronizeRequest {
......
......@@ -91,9 +91,15 @@ std::vector<GURL> TestPendingAppManager::GetInstalledAppUrls(
return urls;
}
base::Optional<std::string> TestPendingAppManager::LookupAppId(
base::Optional<AppId> TestPendingAppManager::LookupAppId(
const GURL& url) const {
return base::Optional<std::string>();
}
bool TestPendingAppManager::HasAppIdWithInstallSource(
const AppId& app_id,
web_app::InstallSource install_source) const {
return false;
}
} // namespace web_app
......@@ -56,7 +56,10 @@ class TestPendingAppManager : public PendingAppManager {
OnceInstallCallback callback) override;
std::vector<GURL> GetInstalledAppUrls(
InstallSource install_source) const override;
base::Optional<std::string> LookupAppId(const GURL& url) const override;
base::Optional<AppId> LookupAppId(const GURL& url) const override;
bool HasAppIdWithInstallSource(
const AppId& app_id,
web_app::InstallSource install_source) const override;
private:
void DoInstall(InstallOptions install_options, OnceInstallCallback callback);
......
......@@ -131,11 +131,18 @@ std::vector<GURL> PendingBookmarkAppManager::GetInstalledAppUrls(
install_source);
}
base::Optional<std::string> PendingBookmarkAppManager::LookupAppId(
base::Optional<web_app::AppId> PendingBookmarkAppManager::LookupAppId(
const GURL& url) const {
return extension_ids_map_.LookupExtensionId(url);
}
bool PendingBookmarkAppManager::HasAppIdWithInstallSource(
const web_app::AppId& app_id,
web_app::InstallSource install_source) const {
return web_app::ExtensionIdsMap::HasExtensionIdWithInstallSource(
profile_->GetPrefs(), app_id, install_source);
}
void PendingBookmarkAppManager::SetTaskFactoryForTesting(
TaskFactory task_factory) {
task_factory_ = std::move(task_factory);
......
......@@ -67,7 +67,10 @@ class PendingBookmarkAppManager final : public web_app::PendingAppManager {
OnceInstallCallback callback) override;
std::vector<GURL> GetInstalledAppUrls(
web_app::InstallSource install_source) const override;
base::Optional<std::string> LookupAppId(const GURL& url) const override;
base::Optional<web_app::AppId> LookupAppId(const GURL& url) const override;
bool HasAppIdWithInstallSource(
const web_app::AppId& app_id,
web_app::InstallSource install_source) const override;
void SetTaskFactoryForTesting(TaskFactory task_factory);
void SetUninstallerForTesting(
......
......@@ -77,6 +77,11 @@ base::Optional<std::string> SystemWebAppManager::GetAppIdForSystemApp(
return pending_app_manager_->LookupAppId(app->second);
}
bool SystemWebAppManager::IsSystemWebApp(const AppId& app_id) const {
return pending_app_manager_->HasAppIdWithInstallSource(
app_id, InstallSource::kSystemInstalled);
}
void SystemWebAppManager::SetSystemAppsForTesting(
base::flat_map<SystemAppType, GURL> system_app_urls) {
system_app_urls_ = std::move(system_app_urls);
......
......@@ -40,6 +40,9 @@ class SystemWebAppManager {
// Returns the app id for the given System App |id|.
base::Optional<std::string> GetAppIdForSystemApp(SystemAppType id) const;
// Returns whether |app_id| points to an installed System App.
bool IsSystemWebApp(const AppId& app_id) const;
protected:
void SetSystemAppsForTesting(
base::flat_map<SystemAppType, GURL> system_app_urls);
......
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