Commit 994334aa authored by Alan Cutter's avatar Alan Cutter Committed by Chromium LUCI CQ

Fix "launch_container": "window" not working for preinstalled web apps that use an offline manifest

The install path used by offline manifest installation was not updating
the WebApplicationInfo according to the InstallParams passed in.
This CL fixes that by putting the user_display_mode setting application
in the shared ApplyParamsToWebApplicationInfo() helper function that
every install path should be using.

Bug: 1159306
Change-Id: Ib632621ac3178dd84bbcd805cffe89e438b8572e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593799
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarGlen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837462}
parent e5caacef
...@@ -293,6 +293,9 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest, ...@@ -293,6 +293,9 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
EXPECT_EQ(registrar().GetAppShortName(app_id), kAppName); EXPECT_EQ(registrar().GetAppShortName(app_id), kAppName);
EXPECT_EQ(registrar().GetAppStartUrl(app_id).spec(), kAppStartUrl); EXPECT_EQ(registrar().GetAppStartUrl(app_id).spec(), kAppStartUrl);
EXPECT_EQ(registrar().GetAppScope(app_id).spec(), kAppScope); EXPECT_EQ(registrar().GetAppScope(app_id).spec(), kAppScope);
EXPECT_EQ(registrar().GetAppUserDisplayMode(app_id),
DisplayMode::kStandalone);
EXPECT_EQ(registrar().GetAppDisplayMode(app_id), DisplayMode::kMinimalUi);
// theme_color must be installed opaque. // theme_color must be installed opaque.
EXPECT_EQ(registrar().GetAppThemeColor(app_id), EXPECT_EQ(registrar().GetAppThemeColor(app_id),
SkColorSetARGB(0xFF, 0xBB, 0xCC, 0xDD)); SkColorSetARGB(0xFF, 0xBB, 0xCC, 0xDD));
...@@ -345,6 +348,9 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest, ...@@ -345,6 +348,9 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
EXPECT_EQ(registrar().GetAppShortName(app_id), "Basic web app"); EXPECT_EQ(registrar().GetAppShortName(app_id), "Basic web app");
EXPECT_EQ(registrar().GetAppStartUrl(app_id).spec(), install_url); EXPECT_EQ(registrar().GetAppStartUrl(app_id).spec(), install_url);
EXPECT_EQ(registrar().GetAppScope(app_id).spec(), scope); EXPECT_EQ(registrar().GetAppScope(app_id).spec(), scope);
EXPECT_EQ(registrar().GetAppUserDisplayMode(app_id),
DisplayMode::kStandalone);
EXPECT_EQ(registrar().GetAppDisplayMode(app_id), DisplayMode::kStandalone);
} }
// Check that offline only installs work offline. // Check that offline only installs work offline.
...@@ -383,6 +389,9 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest, ...@@ -383,6 +389,9 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
EXPECT_EQ(registrar().GetAppShortName(app_id), kAppName); EXPECT_EQ(registrar().GetAppShortName(app_id), kAppName);
EXPECT_EQ(registrar().GetAppStartUrl(app_id).spec(), kAppStartUrl); EXPECT_EQ(registrar().GetAppStartUrl(app_id).spec(), kAppStartUrl);
EXPECT_EQ(registrar().GetAppScope(app_id).spec(), kAppScope); EXPECT_EQ(registrar().GetAppScope(app_id).spec(), kAppScope);
EXPECT_EQ(registrar().GetAppUserDisplayMode(app_id),
DisplayMode::kStandalone);
EXPECT_EQ(registrar().GetAppDisplayMode(app_id), DisplayMode::kMinimalUi);
// theme_color must be installed opaque. // theme_color must be installed opaque.
EXPECT_EQ(registrar().GetAppThemeColor(app_id), EXPECT_EQ(registrar().GetAppThemeColor(app_id),
SkColorSetARGB(0xFF, 0xBB, 0xCC, 0xDD)); SkColorSetARGB(0xFF, 0xBB, 0xCC, 0xDD));
...@@ -432,6 +441,9 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest, ...@@ -432,6 +441,9 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
EXPECT_EQ(registrar().GetAppShortName(app_id), kAppName); EXPECT_EQ(registrar().GetAppShortName(app_id), kAppName);
EXPECT_EQ(registrar().GetAppStartUrl(app_id).spec(), start_url); EXPECT_EQ(registrar().GetAppStartUrl(app_id).spec(), start_url);
EXPECT_EQ(registrar().GetAppScope(app_id).spec(), scope); EXPECT_EQ(registrar().GetAppScope(app_id).spec(), scope);
EXPECT_EQ(registrar().GetAppUserDisplayMode(app_id),
DisplayMode::kStandalone);
EXPECT_EQ(registrar().GetAppDisplayMode(app_id), DisplayMode::kMinimalUi);
// theme_color must be installed opaque. // theme_color must be installed opaque.
EXPECT_EQ(registrar().GetAppThemeColor(app_id), EXPECT_EQ(registrar().GetAppThemeColor(app_id),
SkColorSetARGB(0xFF, 0xBB, 0xCC, 0xDD)); SkColorSetARGB(0xFF, 0xBB, 0xCC, 0xDD));
......
...@@ -145,6 +145,9 @@ void WebAppInstallTask::InstallWebAppFromManifest( ...@@ -145,6 +145,9 @@ void WebAppInstallTask::InstallWebAppFromManifest(
auto web_app_info = std::make_unique<WebApplicationInfo>(); auto web_app_info = std::make_unique<WebApplicationInfo>();
if (install_params_)
ApplyParamsToWebApplicationInfo(*install_params_, *web_app_info);
data_retriever_->CheckInstallabilityAndRetrieveManifest( data_retriever_->CheckInstallabilityAndRetrieveManifest(
web_contents(), bypass_service_worker_check, web_contents(), bypass_service_worker_check,
base::BindOnce(&WebAppInstallTask::OnDidPerformInstallableCheck, base::BindOnce(&WebAppInstallTask::OnDidPerformInstallableCheck,
...@@ -465,6 +468,11 @@ void WebAppInstallTask::OnGetWebApplicationInfo( ...@@ -465,6 +468,11 @@ void WebAppInstallTask::OnGetWebApplicationInfo(
void WebAppInstallTask::ApplyParamsToWebApplicationInfo( void WebAppInstallTask::ApplyParamsToWebApplicationInfo(
const InstallManager::InstallParams& install_params, const InstallManager::InstallParams& install_params,
WebApplicationInfo& web_app_info) { WebApplicationInfo& web_app_info) {
if (install_params.user_display_mode != DisplayMode::kUndefined) {
web_app_info.open_as_window =
install_params.user_display_mode != DisplayMode::kBrowser;
}
// If `additional_search_terms` was a manifest property, it would be // If `additional_search_terms` was a manifest property, it would be
// sanitized while parsing the manifest. Since it's not, we sanitize it // sanitized while parsing the manifest. Since it's not, we sanitize it
// here. // here.
...@@ -751,11 +759,6 @@ void WebAppInstallTask::OnDialogCompleted( ...@@ -751,11 +759,6 @@ void WebAppInstallTask::OnDialogCompleted(
finalize_options.chromeos_data->is_disabled = finalize_options.chromeos_data->is_disabled =
install_params_->is_disabled; install_params_->is_disabled;
} }
if (install_params_->user_display_mode != DisplayMode::kUndefined) {
web_app_info_copy.open_as_window =
install_params_->user_display_mode != DisplayMode::kBrowser;
}
} }
install_finalizer_->FinalizeInstall( install_finalizer_->FinalizeInstall(
......
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