Commit ec2d03dc authored by Alexey Baskakov's avatar Alexey Baskakov Committed by Chromium LUCI CQ

dpwa: Implement Web Apps Shortcuts Menu in ChromeOS UI.

WebAppsChromeOs::GetMenuModel() appends the menu items
using manifest resources already stored on disk
(that part is already launched on all OSes)

We override previously introduced
WebAppsChromeOs::ExecuteContextMenuCommand()
to launch the web app with url params appended.

We implement ConvertSquareBitmapsToImageSkia helper function
which converts manifest icons to hi-dpi in-memory representation
needed by our AppService UI code.

Bug: 1129721
Change-Id: Ic6aa0777f74db079c91d0f50f173f334d4be862e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462899
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: default avatarNancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843981}
parent e7cc943c
......@@ -188,7 +188,7 @@ gfx::ImageSkia ExtractSubsetForArcImage(const gfx::ImageSkia& image_skia) {
return subset_image;
}
#endif
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
std::map<std::pair<int, int>, gfx::ImageSkia>& GetResourceIconCache() {
static base::NoDestructor<std::map<std::pair<int, int>, gfx::ImageSkia>>
......@@ -1200,6 +1200,58 @@ void ArcActivityIconsToImageSkias(
base::MakeRefCounted<IconLoadingPipeline>(std::move(callback));
icon_loader->LoadArcActivityIcons(icons);
}
gfx::ImageSkia ConvertSquareBitmapsToImageSkia(
const std::map<SquareSizePx, SkBitmap>& icon_bitmaps,
IconEffects icon_effects,
int size_hint_in_dip) {
if (icon_bitmaps.empty()) {
return gfx::ImageSkia{};
}
gfx::ImageSkia image_skia;
auto it = icon_bitmaps.begin();
for (ui::ScaleFactor scale_factor : ui::GetSupportedScaleFactors()) {
float icon_scale = ui::GetScaleForScaleFactor(scale_factor);
SquareSizePx icon_size_in_px =
gfx::ScaleToFlooredSize(gfx::Size(size_hint_in_dip, size_hint_in_dip),
icon_scale)
.width();
while (it != icon_bitmaps.end() && it->first < icon_size_in_px) {
++it;
}
if (it == icon_bitmaps.end() || it->second.empty()) {
continue;
}
SkBitmap bitmap = it->second;
// Resize |bitmap| to match |icon_scale|.
//
// TODO(crbug.com/1140356): All conversions in app_icon_factory.cc must
// perform CPU-heavy operations off the Browser UI thread.
if (bitmap.width() != icon_size_in_px) {
bitmap = skia::ImageOperations::Resize(
bitmap, skia::ImageOperations::RESIZE_LANCZOS3, icon_size_in_px,
icon_size_in_px);
}
image_skia.AddRepresentation(gfx::ImageSkiaRep(bitmap, icon_scale));
}
if (image_skia.isNull()) {
return gfx::ImageSkia{};
}
image_skia.EnsureRepsForSupportedScales();
ApplyIconEffects(icon_effects, size_hint_in_dip, &image_skia);
return image_skia;
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
void ApplyIconEffects(IconEffects icon_effects,
......
......@@ -109,6 +109,13 @@ void ArcActivityIconsToImageSkias(
const std::vector<arc::mojom::ActivityIconPtr>& icons,
base::OnceCallback<void(const std::vector<gfx::ImageSkia>& icons)>
callback);
// TODO(crbug.com/1140356): Unify this function with IconLoadingPipeline class.
// It's the same as IconLoadingPipeline::OnReadWebAppIcon().
gfx::ImageSkia ConvertSquareBitmapsToImageSkia(
const std::map<SquareSizePx, SkBitmap>& icon_bitmaps,
IconEffects icon_effects,
int size_hint_in_dip);
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
// Modifies |image_skia| to apply icon post-processing effects like badging and
......
......@@ -21,6 +21,7 @@
#include "chrome/browser/web_applications/components/app_registry_controller.h"
#include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "chrome/browser/web_applications/components/web_app_icon_generator.h"
#include "chrome/browser/web_applications/test/test_file_utils.h"
#include "chrome/browser/web_applications/test/test_web_app_registry_controller.h"
#include "chrome/browser/web_applications/test/web_app_icon_test_utils.h"
......@@ -1019,3 +1020,116 @@ TEST_F(WebAppIconFactoryTest, LoadIconFailed) {
VerifyIcon(src_image_skia, dst_image_skia);
}
#if BUILDFLAG(IS_CHROMEOS_ASH)
TEST_F(WebAppIconFactoryTest, ConvertSquareBitmapsToImageSkia_Empty) {
gfx::ImageSkia converted_image = ConvertSquareBitmapsToImageSkia(
/*icon_bitmaps=*/std::map<SquareSizePx, SkBitmap>{},
/*icon_effects=*/apps::IconEffects::kNone,
/*size_hint_in_dip=*/32);
EXPECT_TRUE(converted_image.isNull());
}
TEST_F(WebAppIconFactoryTest,
ConvertSquareBitmapsToImageSkia_OneBigIconForDownscale) {
std::map<SquareSizePx, SkBitmap> icon_bitmaps;
web_app::AddGeneratedIcon(&icon_bitmaps, web_app::icon_size::k512,
SK_ColorYELLOW);
gfx::ImageSkia converted_image = ConvertSquareBitmapsToImageSkia(
icon_bitmaps, /*icon_effects=*/apps::IconEffects::kNone,
/*size_hint_in_dip=*/32);
const std::vector<ui::ScaleFactor>& scale_factors =
ui::GetSupportedScaleFactors();
ASSERT_EQ(2U, scale_factors.size());
for (auto& scale_factor : scale_factors) {
const float scale = ui::GetScaleForScaleFactor(scale_factor);
ASSERT_TRUE(converted_image.HasRepresentation(scale));
EXPECT_EQ(
SK_ColorYELLOW,
converted_image.GetRepresentation(scale).GetBitmap().getColor(0, 0));
}
}
TEST_F(WebAppIconFactoryTest,
ConvertSquareBitmapsToImageSkia_OneSmallIconNoUpscale) {
std::map<SquareSizePx, SkBitmap> icon_bitmaps;
web_app::AddGeneratedIcon(&icon_bitmaps, web_app::icon_size::k16,
SK_ColorMAGENTA);
gfx::ImageSkia converted_image = ConvertSquareBitmapsToImageSkia(
icon_bitmaps, /*icon_effects=*/apps::IconEffects::kNone,
/*size_hint_in_dip=*/32);
EXPECT_TRUE(converted_image.isNull());
}
TEST_F(WebAppIconFactoryTest, ConvertSquareBitmapsToImageSkia_MatchBigger) {
const std::vector<SquareSizePx> sizes_px{
web_app::icon_size::k16, web_app::icon_size::k32, web_app::icon_size::k48,
web_app::icon_size::k64, web_app::icon_size::k128};
const std::vector<SkColor> colors{SK_ColorBLUE, SK_ColorRED, SK_ColorMAGENTA,
SK_ColorGREEN, SK_ColorWHITE};
std::map<SquareSizePx, SkBitmap> icon_bitmaps;
for (size_t i = 0; i < sizes_px.size(); ++i) {
web_app::AddGeneratedIcon(&icon_bitmaps, sizes_px[i], colors[i]);
}
gfx::ImageSkia converted_image = ConvertSquareBitmapsToImageSkia(
icon_bitmaps, /*icon_effects=*/apps::IconEffects::kNone,
/*size_hint_in_dip=*/32);
const std::vector<ui::ScaleFactor>& scale_factors =
ui::GetSupportedScaleFactors();
ASSERT_EQ(2U, scale_factors.size());
// Expects 32px and 64px to be chosen for 32dip-normal and 32dip-hi-DPI (2.0f
// scale).
const std::vector<SkColor> expected_colors{SK_ColorRED, SK_ColorGREEN};
for (int i = 0; i < scale_factors.size(); ++i) {
const float scale = ui::GetScaleForScaleFactor(scale_factors[i]);
ASSERT_TRUE(converted_image.HasRepresentation(scale));
EXPECT_EQ(
expected_colors[i],
converted_image.GetRepresentation(scale).GetBitmap().getColor(0, 0));
}
}
TEST_F(WebAppIconFactoryTest, ConvertSquareBitmapsToImageSkia_StandardEffect) {
const std::vector<SquareSizePx> sizes_px{web_app::icon_size::k48,
web_app::icon_size::k96};
const std::vector<SkColor> colors{SK_ColorBLUE, SK_ColorRED};
std::map<SquareSizePx, SkBitmap> icon_bitmaps;
for (size_t i = 0; i < sizes_px.size(); ++i) {
web_app::AddGeneratedIcon(&icon_bitmaps, sizes_px[i], colors[i]);
}
gfx::ImageSkia converted_image = ConvertSquareBitmapsToImageSkia(
icon_bitmaps, /*icon_effects=*/apps::IconEffects::kCrOsStandardIcon,
/*size_hint_in_dip=*/32);
const std::vector<ui::ScaleFactor>& scale_factors =
ui::GetSupportedScaleFactors();
ASSERT_EQ(2U, scale_factors.size());
for (int i = 0; i < scale_factors.size(); ++i) {
const float scale = ui::GetScaleForScaleFactor(scale_factors[i]);
ASSERT_TRUE(converted_image.HasRepresentation(scale));
// No colour in the upper left corner.
EXPECT_FALSE(
converted_image.GetRepresentation(scale).GetBitmap().getColor(0, 0));
const SquareSizePx center_px = sizes_px[i] / 2;
EXPECT_EQ(colors[i],
converted_image.GetRepresentation(scale).GetBitmap().getColor(
center_px, center_px));
}
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
......@@ -7,6 +7,7 @@
#include <utility>
#include "ash/public/cpp/app_menu_constants.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
......@@ -213,4 +214,21 @@ void PopulateItemFromMojoMenuItems(
}
}
base::StringPiece MenuTypeToString(apps::mojom::MenuType menu_type) {
switch (menu_type) {
case apps::mojom::MenuType::kShelf:
return "shelf";
case apps::mojom::MenuType::kAppList:
return "applist";
}
}
apps::mojom::MenuType MenuTypeFromString(base::StringPiece menu_type) {
if (base::LowerCaseEqualsASCII(menu_type, "shelf"))
return apps::mojom::MenuType::kShelf;
if (base::LowerCaseEqualsASCII(menu_type, "applist"))
return apps::mojom::MenuType::kAppList;
return apps::mojom::MenuType::kShelf;
}
} // namespace apps
......@@ -9,6 +9,7 @@
#include <vector>
#include "base/callback.h"
#include "base/strings/string_piece.h"
#include "chrome/browser/chromeos/arc/app_shortcuts/arc_app_shortcut_item.h"
#include "components/services/app_service/public/mojom/types.mojom.h"
#include "ui/base/models/menu_separator_types.h"
......@@ -85,6 +86,13 @@ void PopulateItemFromMojoMenuItems(
ui::SimpleMenuModel* model,
arc::ArcAppShortcutItems* arc_shortcut_items);
// Convert |menu_type| to string. Useful to pass |menu_type| enum as string id.
base::StringPiece MenuTypeToString(apps::mojom::MenuType menu_type);
// Convert |menu_type| string to enum. Useful to pass |menu_type| enum as string
// id.
apps::mojom::MenuType MenuTypeFromString(base::StringPiece menu_type);
} // namespace apps
#endif // CHROME_BROWSER_APPS_APP_SERVICE_MENU_UTIL_H_
......@@ -234,8 +234,7 @@ void WebAppsBase::Connect(
provider_->on_registry_ready().Post(
FROM_HERE, base::BindOnce(&WebAppsBase::StartPublishingWebApps,
weak_ptr_factory_.GetWeakPtr(),
std::move(subscriber_remote)));
AsWeakPtr(), std::move(subscriber_remote)));
}
void WebAppsBase::LoadIcon(const std::string& app_id,
......
......@@ -44,6 +44,7 @@ struct AppLaunchParams;
// An app publisher (in the App Service sense) of Web Apps.
class WebAppsBase : public apps::PublisherBase,
public base::SupportsWeakPtr<WebAppsBase>,
public web_app::AppRegistrarObserver,
public content_settings::Observer {
public:
......@@ -85,20 +86,16 @@ class WebAppsBase : public apps::PublisherBase,
Profile* profile() const { return profile_; }
web_app::WebAppProvider* provider() const { return provider_; }
base::WeakPtr<WebAppsBase> GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
}
apps_util::IncrementingIconKeyFactory& icon_key_factory() {
return icon_key_factory_;
}
private:
void Initialize(const mojo::Remote<apps::mojom::AppService>& app_service);
// Can return nullptr in tests.
const web_app::WebAppRegistrar* GetRegistrar() const;
private:
void Initialize(const mojo::Remote<apps::mojom::AppService>& app_service);
// apps::mojom::Publisher overrides.
void Connect(mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) override;
......@@ -170,8 +167,6 @@ class WebAppsBase : public apps::PublisherBase,
// app_service_ is owned by the object that owns this object.
apps::mojom::AppService* app_service_;
base::WeakPtrFactory<WebAppsBase> weak_ptr_factory_{this};
};
void PopulateIntentFilters(const web_app::WebApp& web_app,
......
......@@ -18,6 +18,7 @@
#include "base/stl_util.h"
#include "base/strings/string16.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/apps/app_service/app_launch_params.h"
#include "chrome/browser/apps/app_service/app_service_metrics.h"
#include "chrome/browser/apps/app_service/launch_utils.h"
......@@ -36,6 +37,7 @@
#include "chrome/browser/ui/web_applications/web_app_dialog_manager.h"
#include "chrome/browser/ui/web_applications/web_app_launch_manager.h"
#include "chrome/browser/ui/web_applications/web_app_ui_manager_impl.h"
#include "chrome/browser/web_applications/components/app_icon_manager.h"
#include "chrome/browser/web_applications/components/install_finalizer.h"
#include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h"
......@@ -278,6 +280,83 @@ void WebAppsChromeOs::GetMenuModel(const std::string& app_id,
&menu_items);
}
// Read shortcuts menu item icons from disk, if any.
if (base::FeatureList::IsEnabled(
features::kDesktopPWAsAppIconShortcutsMenuUI) &&
!web_app->shortcuts_menu_item_infos().empty()) {
// TODO(crbug.com/1152661): ReadAllShortcutsMenuIcons must support
// IconPurpose::MASKABLE.
provider()->icon_manager().ReadAllShortcutsMenuIcons(
app_id,
base::BindOnce(&WebAppsChromeOs::OnShortcutsMenuIconsRead,
base::AsWeakPtr<WebAppsChromeOs>(this), app_id,
menu_type, std::move(menu_items), std::move(callback)));
} else {
std::move(callback).Run(std::move(menu_items));
}
}
void WebAppsChromeOs::OnShortcutsMenuIconsRead(
const std::string& app_id,
apps::mojom::MenuType menu_type,
apps::mojom::MenuItemsPtr menu_items,
GetMenuModelCallback callback,
ShortcutsMenuIconsBitmaps shortcuts_menu_icons_bitmaps) {
const web_app::WebApp* web_app = GetWebApp(app_id);
if (!web_app) {
std::move(callback).Run(apps::mojom::MenuItems::New());
return;
}
AddSeparator(ui::DOUBLE_SEPARATOR, &menu_items);
int menu_item_index = 0;
for (const WebApplicationShortcutsMenuItemInfo& menu_item_info :
web_app->shortcuts_menu_item_infos()) {
const std::map<SquareSizePx, SkBitmap>* menu_item_icon_bitmaps = nullptr;
if (menu_item_index < shortcuts_menu_icons_bitmaps.size()) {
menu_item_icon_bitmaps = &shortcuts_menu_icons_bitmaps[menu_item_index];
}
if (menu_item_index != 0) {
AddSeparator(ui::PADDED_SEPARATOR, &menu_items);
}
gfx::ImageSkia icon;
if (menu_item_icon_bitmaps) {
// TODO(crbug.com/1140356): Unify this constant with kAppShortcutIconSize
// from ArcAppShortcutsRequest class.
constexpr int kAppShortcutIconSizeDip = 32;
// TODO(crbug.com/1152661): Remove kCrOsStandardIcon and add
// kCrOsStandardBackground|kCrOsStandardMask effects for web app menu
// maskable icons.
IconEffects icon_effects = IconEffects::kNone;
if (base::FeatureList::IsEnabled(features::kAppServiceAdaptiveIcon)) {
icon_effects = IconEffects::kCrOsStandardIcon;
}
icon = ConvertSquareBitmapsToImageSkia(
*menu_item_icon_bitmaps, icon_effects,
/*size_hint_in_dip=*/kAppShortcutIconSizeDip);
}
// Uses integer |command_id| to store menu item index.
const int command_id = ash::LAUNCH_APP_SHORTCUT_FIRST + menu_item_index;
// Passes menu_type argument as shortcut_id to use it in
// ExecuteContextMenuCommand().
std::string shortcut_id{MenuTypeToString(menu_type)};
const std::string label = base::UTF16ToUTF8(menu_item_info.name);
// TODO(crbug.com/1140356): Rename AddArcCommandItem to
// AddPublisherCommandItem.
AddArcCommandItem(command_id, shortcut_id, label, icon, &menu_items);
++menu_item_index;
}
std::move(callback).Run(std::move(menu_items));
}
......@@ -285,8 +364,37 @@ void WebAppsChromeOs::ExecuteContextMenuCommand(const std::string& app_id,
int command_id,
const std::string& shortcut_id,
int64_t display_id) {
// TODO(crbug.com/1129721) Implement it for shortcut menu in web apps.
NOTIMPLEMENTED();
const web_app::WebApp* web_app = GetWebApp(app_id);
if (!web_app) {
return;
}
apps::mojom::LaunchSource launch_source;
// shortcut_id contains menu_type.
switch (MenuTypeFromString(shortcut_id)) {
case apps::mojom::MenuType::kShelf:
launch_source = apps::mojom::LaunchSource::kFromShelf;
break;
case apps::mojom::MenuType::kAppList:
launch_source = apps::mojom::LaunchSource::kFromAppListGridContextMenu;
break;
}
web_app::DisplayMode display_mode =
GetRegistrar()->GetAppEffectiveDisplayMode(app_id);
apps::AppLaunchParams params(
app_id, web_app::ConvertDisplayModeToAppLaunchContainer(display_mode),
WindowOpenDisposition::CURRENT_TAB, GetAppLaunchSource(launch_source),
display_id);
size_t menu_item_index = command_id - ash::LAUNCH_APP_SHORTCUT_FIRST;
if (menu_item_index < web_app->shortcuts_menu_item_infos().size()) {
params.override_url =
web_app->shortcuts_menu_item_infos()[menu_item_index].url;
}
LaunchAppWithParams(std::move(params));
}
void WebAppsChromeOs::OnWebAppWillBeUninstalled(const web_app::AppId& app_id) {
......
......@@ -19,6 +19,7 @@
#include "chrome/browser/notifications/notification_display_service.h"
#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
#include "chrome/browser/web_applications/components/web_app_id.h"
#include "chrome/browser/web_applications/components/web_application_info.h"
#include "components/services/app_service/public/cpp/instance_registry.h"
#include "components/services/app_service/public/mojom/app_service.mojom.h"
#include "components/services/app_service/public/mojom/types.mojom.h"
......@@ -84,6 +85,7 @@ class WebAppsChromeOs : public WebAppsBase,
apps::mojom::MenuType menu_type,
int64_t display_id,
GetMenuModelCallback callback) override;
// menu_type is stored as |shortcut_id|.
void ExecuteContextMenuCommand(const std::string& app_id,
int command_id,
const std::string& shortcut_id,
......@@ -93,7 +95,6 @@ class WebAppsChromeOs : public WebAppsBase,
void OnWebAppWillBeUninstalled(const web_app::AppId& app_id) override;
void OnWebAppDisabledStateChanged(const web_app::AppId& app_id,
bool is_disabled) override;
// TODO(loyso): Implement app->last_launch_time field for the new system.
// ArcAppListPrefs::Observer overrides.
void OnPackageInstalled(
......@@ -111,6 +112,13 @@ class WebAppsChromeOs : public WebAppsBase,
void OnNotificationDisplayServiceDestroyed(
NotificationDisplayService* service) override;
void OnShortcutsMenuIconsRead(
const std::string& app_id,
apps::mojom::MenuType menu_type,
apps::mojom::MenuItemsPtr menu_items,
GetMenuModelCallback callback,
ShortcutsMenuIconsBitmaps shortcuts_menu_icons_bitmaps);
bool MaybeAddNotification(const std::string& app_id,
const std::string& notification_id);
void MaybeAddWebPageNotifications(
......
......@@ -233,6 +233,13 @@ const base::Feature kDesktopCaptureTabSharingInfobar{
const base::Feature kDesktopPWAsAppIconShortcutsMenu{
"DesktopPWAsAppIconShortcutsMenu", base::FEATURE_ENABLED_BY_DEFAULT};
#if BUILDFLAG(IS_CHROMEOS_ASH)
// Enables Desktop PWAs shortcuts menu to be visible and executable in ChromeOS
// UI surfaces.
const base::Feature kDesktopPWAsAppIconShortcutsMenuUI{
"DesktopPWAsAppIconShortcutsMenuUI", base::FEATURE_DISABLED_BY_DEFAULT};
#endif
// When installing default installed PWAs, we wait for service workers
// to cache resources.
const base::Feature kDesktopPWAsCacheDuringDefaultInstall{
......
......@@ -168,6 +168,11 @@ extern const base::Feature kDesktopCaptureTabSharingInfobar;
COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kDesktopPWAsAppIconShortcutsMenu;
#if BUILDFLAG(IS_CHROMEOS_ASH)
COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kDesktopPWAsAppIconShortcutsMenuUI;
#endif
COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kDesktopPWAsCacheDuringDefaultInstall;
......
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