Commit a3df8027 authored by Jesse McKenna's avatar Jesse McKenna Committed by Commit Bot

desktop-pwas: fix PWA file-handler icon paths

This change fixes an issue where PWA custom icons were not showing up
in the Open With menu on Windows when a PWA is registered as a file
handler on Windows.

A PWA's custom icon is located at:
<user data>/<profile>/Web Applications/<app ID>/<app name>.ico

Currently, the value of DefaultIcon as registered with Windows is:
<user data>/<profile>/Web Applications/<app ID>/<app name>.exe/<app name>.ico

Windows fails to find the icon file and instead displays a generic
icon. This is caused by GetIconFilePath() being passed
|app_launcher_path| (which has value <app name>.exe) as its
|web_app_path| parameter. This change replaces that argument with the
path to <user data>/<profile>/Web Applications/<app ID>.

Bug: 1143966
Change-Id: I4744b9866c85f570c3e9bd794d83a6abb6cd97eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508648Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Cr-Commit-Position: refs/heads/master@{#823194}
parent 8bf42472
...@@ -31,8 +31,10 @@ void RegisterFileHandlersWithOsTask( ...@@ -31,8 +31,10 @@ void RegisterFileHandlersWithOsTask(
const base::FilePath& profile_path, const base::FilePath& profile_path,
const std::set<base::string16>& file_extensions, const std::set<base::string16>& file_extensions,
const base::string16& app_name_extension) { const base::string16& app_name_extension) {
const base::FilePath web_app_path =
GetOsIntegrationResourcesDirectoryForApp(profile_path, app_id, GURL());
base::Optional<base::FilePath> app_specific_launcher_path = base::Optional<base::FilePath> app_specific_launcher_path =
CreateAppLauncherFile(app_id, app_name, app_name_extension, profile_path); CreateAppLauncherFile(app_name, app_name_extension, web_app_path);
if (!app_specific_launcher_path.has_value()) if (!app_specific_launcher_path.has_value())
return; return;
...@@ -42,8 +44,7 @@ void RegisterFileHandlersWithOsTask( ...@@ -42,8 +44,7 @@ void RegisterFileHandlersWithOsTask(
base::string16 user_visible_app_name(app_name); base::string16 user_visible_app_name(app_name);
user_visible_app_name.append(app_name_extension); user_visible_app_name.append(app_name_extension);
base::FilePath icon_path = base::FilePath icon_path = internals::GetIconFilePath(web_app_path, app_name);
internals::GetIconFilePath(app_specific_launcher_path.value(), app_name);
bool result = ShellUtil::AddFileAssociations( bool result = ShellUtil::AddFileAssociations(
GetProgIdForApp(profile_path, app_id), app_specific_launcher_command, GetProgIdForApp(profile_path, app_id), app_specific_launcher_command,
......
...@@ -103,8 +103,10 @@ void UpdateAppRegistration(const web_app::AppId& app_id, ...@@ -103,8 +103,10 @@ void UpdateAppRegistration(const web_app::AppId& app_id,
user_visible_app_name.append(app_name_extension); user_visible_app_name.append(app_name_extension);
base::Optional<base::FilePath> app_launcher_path = base::Optional<base::FilePath> app_launcher_path =
web_app::CreateAppLauncherFile(app_id, app_name, app_name_extension, web_app::CreateAppLauncherFile(
profile_path); app_name, app_name_extension,
web_app::GetOsIntegrationResourcesDirectoryForApp(profile_path,
app_id, GURL()));
if (!app_launcher_path) if (!app_launcher_path)
return; return;
...@@ -205,12 +207,9 @@ base::string16 GetProgIdForApp(const base::FilePath& profile_path, ...@@ -205,12 +207,9 @@ base::string16 GetProgIdForApp(const base::FilePath& profile_path,
} }
base::Optional<base::FilePath> CreateAppLauncherFile( base::Optional<base::FilePath> CreateAppLauncherFile(
const AppId& app_id,
const base::string16& app_name, const base::string16& app_name,
const base::string16& app_name_extension, const base::string16& app_name_extension,
const base::FilePath& profile_path) { const base::FilePath& web_app_path) {
base::FilePath web_app_path =
GetOsIntegrationResourcesDirectoryForApp(profile_path, app_id, GURL());
if (!base::CreateDirectory(web_app_path)) { if (!base::CreateDirectory(web_app_path)) {
DPLOG(ERROR) << "Unable to create web app dir"; DPLOG(ERROR) << "Unable to create web app dir";
RecordRegistration(RegistrationResult::kFailToCopyFromGenericLauncher); RecordRegistration(RegistrationResult::kFailToCopyFromGenericLauncher);
......
...@@ -33,13 +33,12 @@ base::string16 GetProgIdForApp(const base::FilePath& profile_path, ...@@ -33,13 +33,12 @@ base::string16 GetProgIdForApp(const base::FilePath& profile_path,
const AppId& app_id); const AppId& app_id);
// Makes an app-specific copy of chrome_pwa_launcher.exe that lives in the web // Makes an app-specific copy of chrome_pwa_launcher.exe that lives in the web
// application directory. Returns path of the launcher file if successful, // application directory |web_app_path|. Returns path of the launcher file if
// base::nullopt otherwise. // successful, base::nullopt otherwise.
base::Optional<base::FilePath> CreateAppLauncherFile( base::Optional<base::FilePath> CreateAppLauncherFile(
const AppId& app_id,
const base::string16& app_name, const base::string16& app_name,
const base::string16& app_name_extension, const base::string16& app_name_extension,
const base::FilePath& profile_path); const base::FilePath& web_app_path);
// Checks if there is an installation of this app in another profile that needs // Checks if there is an installation of this app in another profile that needs
// to be updated with a profile specific name and executes required update. // to be updated with a profile specific name and executes required update.
......
...@@ -59,7 +59,8 @@ class WebAppHandlerRegistrationUtilsWinTest : public testing::Test { ...@@ -59,7 +59,8 @@ class WebAppHandlerRegistrationUtilsWinTest : public testing::Test {
const base::string16& app_name_extension, const base::string16& app_name_extension,
const base::FilePath& profile_path) { const base::FilePath& profile_path) {
base::Optional<base::FilePath> launcher_path = CreateAppLauncherFile( base::Optional<base::FilePath> launcher_path = CreateAppLauncherFile(
app_id, app_name, app_name_extension, profile_path); app_name, app_name_extension,
GetOsIntegrationResourcesDirectoryForApp(profile_path, app_id, GURL()));
ASSERT_TRUE(launcher_path.has_value()); ASSERT_TRUE(launcher_path.has_value());
base::CommandLine launcher_command = base::CommandLine launcher_command =
...@@ -225,8 +226,10 @@ TEST_F(WebAppHandlerRegistrationUtilsWinTest, ...@@ -225,8 +226,10 @@ TEST_F(WebAppHandlerRegistrationUtilsWinTest,
TEST_F(WebAppHandlerRegistrationUtilsWinTest, CreateAppLauncherFile) { TEST_F(WebAppHandlerRegistrationUtilsWinTest, CreateAppLauncherFile) {
base::string16 app_name_extension = L" extension"; base::string16 app_name_extension = L" extension";
base::Optional<base::FilePath> launcher_path = CreateAppLauncherFile( base::Optional<base::FilePath> launcher_path =
app_id(), app_name(), app_name_extension, profile()->GetPath()); CreateAppLauncherFile(app_name(), app_name_extension,
GetOsIntegrationResourcesDirectoryForApp(
profile()->GetPath(), app_id(), GURL()));
EXPECT_TRUE(launcher_path.has_value()); EXPECT_TRUE(launcher_path.has_value());
EXPECT_TRUE(base::PathExists(launcher_path.value())); EXPECT_TRUE(base::PathExists(launcher_path.value()));
......
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