Commit f28f8ee8 authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

desktop-pwas: Store background_color manifest field in WebApp

This CL stores the background_color manifest field in the WebApp
database for use by experimental tabbed web app windows.
This CL has no user visible changes.

Bug: 1104083
Change-Id: I69ad67c17b61161e721a4639340187550e0644b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2291515Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788814}
parent 5789c892
......@@ -38,6 +38,7 @@
#include "chrome/browser/web_applications/components/web_app_provider_base.h"
#include "chrome/browser/web_applications/test/web_app_install_observer.h"
#include "chrome/browser/web_applications/test/web_app_test.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/web_application_info.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/sessions/core/tab_restore_service.h"
......@@ -145,6 +146,24 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, ThemeColor) {
}
}
IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, BackgroundColor) {
// This feature is intentionally not implemented for the obsolete bookmark
// apps.
if (!base::FeatureList::IsEnabled(features::kDesktopPWAsWithoutExtensions))
return;
blink::Manifest manifest;
manifest.start_url = GURL(kExampleURL);
manifest.scope = GURL(kExampleURL);
manifest.background_color = SkColorSetA(SK_ColorBLUE, 0xF0);
auto web_app_info = std::make_unique<WebApplicationInfo>();
web_app::UpdateWebAppInfoFromManifest(manifest, web_app_info.get());
AppId app_id = InstallWebApp(std::move(web_app_info));
auto* provider = WebAppProviderBase::GetProviderBase(profile());
EXPECT_EQ(provider->registrar().GetAppBackgroundColor(app_id), SK_ColorBLUE);
}
// This tests that we don't crash when launching a PWA window with an
// autogenerated user theme set.
IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, AutoGeneratedUserThemeCrash) {
......
......@@ -87,6 +87,8 @@ class AppRegistrar {
virtual std::string GetAppDescription(const AppId& app_id) const = 0;
virtual base::Optional<SkColor> GetAppThemeColor(
const AppId& app_id) const = 0;
virtual base::Optional<SkColor> GetAppBackgroundColor(
const AppId& app_id) const = 0;
virtual const GURL& GetAppLaunchURL(const AppId& app_id) const = 0;
// TODO(crbug.com/910016): Replace uses of this with GetAppScope().
......
......@@ -149,6 +149,11 @@ void UpdateWebAppInfoFromManifest(const blink::Manifest& manifest,
SkColorSetA(*manifest.theme_color, SK_AlphaOPAQUE);
}
if (manifest.background_color) {
web_app_info->background_color =
SkColorSetA(*manifest.background_color, SK_AlphaOPAQUE);
}
if (manifest.display != DisplayMode::kUndefined)
web_app_info->display_mode = manifest.display;
......
......@@ -127,6 +127,12 @@ base::Optional<SkColor> BookmarkAppRegistrar::GetAppThemeColor(
return AppThemeColorInfo::GetThemeColor(extension);
}
base::Optional<SkColor> BookmarkAppRegistrar::GetAppBackgroundColor(
const web_app::AppId& app_id) const {
// Only implemented for WebApp. Bookmark apps are going away.
return base::nullopt;
}
const GURL& BookmarkAppRegistrar::GetAppLaunchURL(
const web_app::AppId& app_id) const {
const Extension* extension = GetBookmarkAppDchecked(app_id);
......
......@@ -42,6 +42,8 @@ class BookmarkAppRegistrar : public web_app::AppRegistrar,
std::string GetAppDescription(const web_app::AppId& app_id) const override;
base::Optional<SkColor> GetAppThemeColor(
const web_app::AppId& app_id) const override;
base::Optional<SkColor> GetAppBackgroundColor(
const web_app::AppId& app_id) const override;
const GURL& GetAppLaunchURL(const web_app::AppId& app_id) const override;
base::Optional<GURL> GetAppScopeInternal(
const web_app::AppId& app_id) const override;
......
......@@ -126,4 +126,6 @@ message WebAppProto {
// A list of icon sizes we successfully downloaded to store on disk.
repeated DownloadedShortcutsMenuIconSizesProto
downloaded_shortcuts_menu_icons_sizes = 20;
optional uint32 background_color = 21;
}
......@@ -98,6 +98,12 @@ base::Optional<SkColor> TestAppRegistrar::GetAppThemeColor(
return base::nullopt;
}
base::Optional<SkColor> TestAppRegistrar::GetAppBackgroundColor(
const AppId& app_id) const {
NOTIMPLEMENTED();
return base::nullopt;
}
const GURL& TestAppRegistrar::GetAppLaunchURL(const AppId& app_id) const {
auto iterator = installed_apps_.find(app_id);
if (iterator == installed_apps_.end())
......
......@@ -56,6 +56,8 @@ class TestAppRegistrar : public AppRegistrar {
std::string GetAppShortName(const AppId& app_id) const override;
std::string GetAppDescription(const AppId& app_id) const override;
base::Optional<SkColor> GetAppThemeColor(const AppId& app_id) const override;
base::Optional<SkColor> GetAppBackgroundColor(
const AppId& app_id) const override;
const GURL& GetAppLaunchURL(const AppId& app_id) const override;
base::Optional<GURL> GetAppScopeInternal(const AppId& app_id) const override;
DisplayMode GetAppDisplayMode(const AppId& app_id) const override;
......
......@@ -131,6 +131,10 @@ void WebApp::SetThemeColor(base::Optional<SkColor> theme_color) {
theme_color_ = theme_color;
}
void WebApp::SetBackgroundColor(base::Optional<SkColor> background_color) {
background_color_ = background_color;
}
void WebApp::SetDisplayMode(DisplayMode display_mode) {
DCHECK_NE(DisplayMode::kUndefined, display_mode);
display_mode_ = display_mode;
......@@ -256,6 +260,8 @@ std::ostream& operator<<(std::ostream& out, const WebApp& app) {
<< " launch_url: " << app.launch_url_ << std::endl
<< " scope: " << app.scope_ << std::endl
<< " theme_color: " << ColorToString(app.theme_color_) << std::endl
<< " background_color: " << ColorToString(app.background_color_)
<< std::endl
<< " display_mode: " << display_mode << std::endl
<< " user_display_mode: " << user_display_mode << std::endl
<< " user_page_ordinal: " << app.user_page_ordinal_.ToDebugString()
......@@ -307,24 +313,26 @@ bool operator!=(const WebApp::SyncFallbackData& sync_fallback_data1,
bool operator==(const WebApp& app1, const WebApp& app2) {
return std::tie(app1.app_id_, app1.sources_, app1.name_, app1.launch_url_,
app1.description_, app1.scope_, app1.theme_color_,
app1.icon_infos_, app1.downloaded_icon_sizes_,
app1.is_generated_icon_, app1.display_mode_,
app1.user_display_mode_, app1.user_page_ordinal_,
app1.user_launch_ordinal_, app1.chromeos_data_,
app1.is_locally_installed_, app1.is_in_sync_install_,
app1.file_handlers_, app1.additional_search_terms_,
app1.protocol_handlers_, app1.sync_fallback_data_,
app1.last_launch_time_, app1.install_time_) ==
app1.background_color_, app1.icon_infos_,
app1.downloaded_icon_sizes_, app1.is_generated_icon_,
app1.display_mode_, app1.user_display_mode_,
app1.user_page_ordinal_, app1.user_launch_ordinal_,
app1.chromeos_data_, app1.is_locally_installed_,
app1.is_in_sync_install_, app1.file_handlers_,
app1.additional_search_terms_, app1.protocol_handlers_,
app1.sync_fallback_data_, app1.last_launch_time_,
app1.install_time_) ==
std::tie(app2.app_id_, app2.sources_, app2.name_, app2.launch_url_,
app2.description_, app2.scope_, app2.theme_color_,
app2.icon_infos_, app2.downloaded_icon_sizes_,
app2.is_generated_icon_, app2.display_mode_,
app2.user_display_mode_, app2.user_page_ordinal_,
app2.user_launch_ordinal_, app2.chromeos_data_,
app2.is_locally_installed_, app2.is_in_sync_install_,
app2.file_handlers_, app2.additional_search_terms_,
app2.protocol_handlers_, app2.sync_fallback_data_,
app2.last_launch_time_, app2.install_time_);
app2.background_color_, app2.icon_infos_,
app2.downloaded_icon_sizes_, app2.is_generated_icon_,
app2.display_mode_, app2.user_display_mode_,
app2.user_page_ordinal_, app2.user_launch_ordinal_,
app2.chromeos_data_, app2.is_locally_installed_,
app2.is_in_sync_install_, app2.file_handlers_,
app2.additional_search_terms_, app2.protocol_handlers_,
app2.sync_fallback_data_, app2.last_launch_time_,
app2.install_time_);
}
bool operator!=(const WebApp& app1, const WebApp& app2) {
......
......@@ -47,6 +47,9 @@ class WebApp {
const GURL& scope() const { return scope_; }
const base::Optional<SkColor>& theme_color() const { return theme_color_; }
const base::Optional<SkColor>& background_color() const {
return background_color_;
}
DisplayMode display_mode() const { return display_mode_; }
......@@ -159,6 +162,7 @@ class WebApp {
void SetLaunchUrl(const GURL& launch_url);
void SetScope(const GURL& scope);
void SetThemeColor(base::Optional<SkColor> theme_color);
void SetBackgroundColor(base::Optional<SkColor> background_color);
void SetDisplayMode(DisplayMode display_mode);
void SetUserDisplayMode(DisplayMode user_display_mode);
void SetUserPageOrdinal(syncer::StringOrdinal page_ordinal);
......@@ -203,6 +207,7 @@ class WebApp {
// is within the scope.
GURL scope_;
base::Optional<SkColor> theme_color_;
base::Optional<SkColor> background_color_;
DisplayMode display_mode_;
DisplayMode user_display_mode_;
syncer::StringOrdinal user_page_ordinal_;
......
......@@ -123,6 +123,8 @@ std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
local_data->set_scope(web_app.scope().spec());
if (web_app.theme_color().has_value())
local_data->set_theme_color(web_app.theme_color().value());
if (web_app.background_color().has_value())
local_data->set_background_color(web_app.background_color().value());
if (!web_app.last_launch_time().is_null()) {
local_data->set_last_launch_time(
syncer::TimeToProtoTime(web_app.last_launch_time()));
......@@ -330,6 +332,9 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(
if (local_data.has_theme_color())
web_app->SetThemeColor(local_data.theme_color());
if (local_data.has_background_color())
web_app->SetBackgroundColor(local_data.background_color());
if (local_data.has_is_in_sync_install())
web_app->SetIsInSyncInstall(local_data.is_in_sync_install());
......
......@@ -143,6 +143,7 @@ class WebAppDatabaseTest : public WebAppTest {
const std::string scope =
base_url + "/scope" + base::NumberToString(suffix);
const base::Optional<SkColor> theme_color = suffix;
const base::Optional<SkColor> background_color = 2 * suffix;
const base::Optional<SkColor> synced_theme_color = suffix ^ UINT_MAX;
auto app = std::make_unique<WebApp>(app_id);
......@@ -165,6 +166,7 @@ class WebAppDatabaseTest : public WebAppTest {
app->SetLaunchUrl(GURL(launch_url));
app->SetScope(GURL(scope));
app->SetThemeColor(theme_color);
app->SetBackgroundColor(background_color);
app->SetIsLocallyInstalled(!(suffix & 2));
app->SetIsInSyncInstall(!(suffix & 4));
app->SetUserDisplayMode((suffix & 1) ? DisplayMode::kBrowser
......@@ -410,6 +412,7 @@ TEST_F(WebAppDatabaseTest, WebAppWithoutOptionalFields) {
EXPECT_TRUE(app->description().empty());
EXPECT_TRUE(app->scope().is_empty());
EXPECT_FALSE(app->theme_color().has_value());
EXPECT_FALSE(app->background_color().has_value());
EXPECT_TRUE(app->icon_infos().empty());
EXPECT_TRUE(app->downloaded_icon_sizes().empty());
EXPECT_FALSE(app->is_generated_icon());
......@@ -460,6 +463,7 @@ TEST_F(WebAppDatabaseTest, WebAppWithoutOptionalFields) {
EXPECT_TRUE(app_copy->description().empty());
EXPECT_TRUE(app_copy->scope().is_empty());
EXPECT_FALSE(app_copy->theme_color().has_value());
EXPECT_FALSE(app_copy->background_color().has_value());
EXPECT_TRUE(app_copy->last_launch_time().is_null());
EXPECT_TRUE(app_copy->install_time().is_null());
EXPECT_TRUE(app_copy->icon_infos().empty());
......
......@@ -434,6 +434,10 @@ void WebAppInstallFinalizer::SetWebAppManifestFieldsAndWriteData(
DCHECK_EQ(SkColorGetA(*web_app_info.theme_color), SK_AlphaOPAQUE);
web_app->SetThemeColor(web_app_info.theme_color);
}
if (web_app_info.background_color) {
DCHECK_EQ(SkColorGetA(*web_app_info.background_color), SK_AlphaOPAQUE);
web_app->SetBackgroundColor(*web_app_info.background_color);
}
WebApp::SyncFallbackData sync_fallback_data;
sync_fallback_data.name = base::UTF16ToUTF8(web_app_info.title);
......
......@@ -80,6 +80,12 @@ base::Optional<SkColor> WebAppRegistrar::GetAppThemeColor(
return web_app ? web_app->theme_color() : base::nullopt;
}
base::Optional<SkColor> WebAppRegistrar::GetAppBackgroundColor(
const AppId& app_id) const {
auto* web_app = GetAppById(app_id);
return web_app ? web_app->background_color() : base::nullopt;
}
const GURL& WebAppRegistrar::GetAppLaunchURL(const AppId& app_id) const {
auto* web_app = GetAppById(app_id);
return web_app ? web_app->launch_url() : GURL::EmptyGURL();
......
......@@ -45,6 +45,8 @@ class WebAppRegistrar : public AppRegistrar, public ProfileManagerObserver {
std::string GetAppShortName(const AppId& app_id) const override;
std::string GetAppDescription(const AppId& app_id) const override;
base::Optional<SkColor> GetAppThemeColor(const AppId& app_id) const override;
base::Optional<SkColor> GetAppBackgroundColor(
const AppId& app_id) const override;
const GURL& GetAppLaunchURL(const AppId& app_id) const override;
base::Optional<GURL> GetAppScopeInternal(const AppId& app_id) const override;
DisplayMode GetAppDisplayMode(const AppId& app_id) const override;
......
......@@ -118,6 +118,10 @@ struct WebApplicationInfo {
// The color to use for the web app frame.
base::Optional<SkColor> theme_color;
// The expected page background color of the web app.
// https://www.w3.org/TR/appmanifest/#background_color-member
base::Optional<SkColor> background_color;
// App preference regarding whether the app should be opened in a tab,
// in a window (with or without minimal-ui buttons), or full screen. Defaults
// to browser display mode as specified in
......
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