From ac40cb4c3207c32482449bee0112440a39b871ff Mon Sep 17 00:00:00 2001 From: Phillis Tang Date: Wed, 25 Jan 2023 21:55:04 +0000 Subject: [PATCH] apphome: support install locally - Add install locally as a menu item to the app home frontend. - Add `is_locally_installed` field to `AppInfo`. This controls a set of menu items visibility, including `open_in_window`, so `may_show_open_in_window` is not needed anymore. - Listens to `OnWebAppInstallLocallyWithOsHooks` and update frontend. Bug: 1350406 Change-Id: I9c2aca4af65da865a1d09ff570210358b715b820 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4193146 Reviewed-by: Daniel Murphy Reviewed-by: Dibyajyoti Pal Reviewed-by: Camden King Commit-Queue: Phillis Tang Reviewed-by: Mustafa Emre Acer Cr-Commit-Position: refs/heads/main@{#1097019} --- chrome/app/generated_resources.grd | 4 + .../IDS_APP_HOME_INSTALL_LOCALLY.png.sha1 | 1 + .../browser/resources/app_home/app_item.html | 2 +- chrome/browser/resources/app_home/app_item.ts | 11 ++ .../browser/resources/app_home/app_list.html | 13 +- chrome/browser/resources/app_home/app_list.ts | 14 ++- .../browser/ui/webui/app_home/app_home.mojom | 4 +- .../webui/app_home/app_home_page_handler.cc | 13 +- .../ui/webui/app_home/app_home_page_handler.h | 5 + .../browser/ui/webui/app_home/app_home_ui.cc | 1 + .../test/data/webui/app_home/app_list_test.ts | 112 ++++++++++++++++-- .../app_home/test_app_home_browser_proxy.ts | 18 ++- 12 files changed, 173 insertions(+), 25 deletions(-) create mode 100644 chrome/app/generated_resources_grd/IDS_APP_HOME_INSTALL_LOCALLY.png.sha1 diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 89328d2c8cc7a..b279f2e9e2387 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -5908,6 +5908,10 @@ Keep your key file in a safe place. You will need it to create new versions of y Create shortcut + + Install on this device + + Uninstall diff --git a/chrome/app/generated_resources_grd/IDS_APP_HOME_INSTALL_LOCALLY.png.sha1 b/chrome/app/generated_resources_grd/IDS_APP_HOME_INSTALL_LOCALLY.png.sha1 new file mode 100644 index 0000000000000..cd4b270daa6be --- /dev/null +++ b/chrome/app/generated_resources_grd/IDS_APP_HOME_INSTALL_LOCALLY.png.sha1 @@ -0,0 +1 @@ +6bc27550b41495ff6b48e18599d57e9b4f9d6d4d \ No newline at end of file diff --git a/chrome/browser/resources/app_home/app_item.html b/chrome/browser/resources/app_home/app_item.html index 54c5cc5bee665..4913e58ea7762 100644 --- a/chrome/browser/resources/app_home/app_item.html +++ b/chrome/browser/resources/app_home/app_item.html @@ -32,6 +32,6 @@
- App's icon + App's icon
[[appInfo.name]]
diff --git a/chrome/browser/resources/app_home/app_item.ts b/chrome/browser/resources/app_home/app_item.ts index 81c80b8167276..0775601d042b8 100644 --- a/chrome/browser/resources/app_home/app_item.ts +++ b/chrome/browser/resources/app_home/app_item.ts @@ -43,6 +43,17 @@ export class AppItemElement extends PolymerElement { this.dispatchEvent( new CustomEvent(eventName, {bubbles: true, composed: true, detail})); } + + private getIconUrl_() { + const url = new URL(this.appInfo.iconUrl.url); + // For web app, the backend serves grayscale image when the app is not + // locally installed automatically and doesn't recognize this query param, + // but we add a query param here to force browser to refetch the image. + if (!this.appInfo.isLocallyInstalled) { + url.searchParams.append('grayscale', 'true'); + } + return url; + } } declare global { diff --git a/chrome/browser/resources/app_home/app_list.html b/chrome/browser/resources/app_home/app_list.html index 23b4004a07ceb..12f3867759768 100644 --- a/chrome/browser/resources/app_home/app_list.html +++ b/chrome/browser/resources/app_home/app_list.html @@ -41,7 +41,7 @@ diff --git a/chrome/browser/resources/app_home/app_list.ts b/chrome/browser/resources/app_home/app_list.ts index 3acfd1dad8ced..37f204675150e 100644 --- a/chrome/browser/resources/app_home/app_list.ts +++ b/chrome/browser/resources/app_home/app_list.ts @@ -111,10 +111,10 @@ export class AppListElement extends PolymerElement { } } - private isOpenInWindowHidden_() { + private isLocallyInstalled_() { return this.selectedActionMenuModel_ ? - !this.selectedActionMenuModel_.appInfo.mayShowOpenInWindow : - true; + this.selectedActionMenuModel_.appInfo.isLocallyInstalled : + false; } private isLaunchOnStartupHidden_() { @@ -173,6 +173,14 @@ export class AppListElement extends PolymerElement { this.closeMenu_(); } + private onInstallLocallyItemClick_() { + if (this.selectedActionMenuModel_?.appInfo.id) { + BrowserProxy.getInstance().handler.installAppLocally( + this.selectedActionMenuModel_?.appInfo.id); + } + this.closeMenu_(); + } + private onUninstallItemClick_() { if (this.selectedActionMenuModel_?.appInfo.id) { BrowserProxy.getInstance().handler.uninstallApp( diff --git a/chrome/browser/ui/webui/app_home/app_home.mojom b/chrome/browser/ui/webui/app_home/app_home.mojom index 5d66b59275c6e..5795831123f4f 100644 --- a/chrome/browser/ui/webui/app_home/app_home.mojom +++ b/chrome/browser/ui/webui/app_home/app_home.mojom @@ -37,8 +37,8 @@ struct AppInfo { // The app's `RunOnOsLoginMode`, including `RunOnOsLoginModeNotRun` and // `RunOnOsLoginModeWindowed`. RunOnOsLoginMode run_on_os_login_mode; - // Whether to show `open_in_window` menu item. - bool may_show_open_in_window; + // Whether the app is installed locally. + bool is_locally_installed; // Whether the app open in a app window or as a browser tab. bool open_in_window; diff --git a/chrome/browser/ui/webui/app_home/app_home_page_handler.cc b/chrome/browser/ui/webui/app_home/app_home_page_handler.cc index 07c23a28c384f..5c1d1c34bbd83 100644 --- a/chrome/browser/ui/webui/app_home/app_home_page_handler.cc +++ b/chrome/browser/ui/webui/app_home/app_home_page_handler.cc @@ -31,6 +31,7 @@ #include "chrome/browser/ui/web_applications/web_app_ui_manager_impl.h" #include "chrome/browser/ui/webui/extensions/extension_icon_source.h" #include "chrome/browser/web_applications/extension_status_utils.h" +#include "chrome/browser/web_applications/extensions/bookmark_app_util.h" #include "chrome/browser/web_applications/locks/app_lock.h" #include "chrome/browser/web_applications/mojom/user_display_mode.mojom.h" #include "chrome/browser/web_applications/web_app.h" @@ -332,7 +333,7 @@ app_home::mojom::AppInfoPtr AppHomePageHandler::CreateAppInfoPtrFromWebApp( app_info->may_toggle_run_on_os_login_mode = login_mode.user_controllable; app_info->run_on_os_login_mode = login_mode.value; - app_info->may_show_open_in_window = is_locally_installed; + app_info->is_locally_installed = is_locally_installed; // Treat all other types of display mode as "open as window". app_info->open_in_window = registrar.GetAppEffectiveDisplayMode(app_id) != blink::mojom::DisplayMode::kBrowser; @@ -357,7 +358,10 @@ app_home::mojom::AppInfoPtr AppHomePageHandler::CreateAppInfoPtrFromExtension( app_info->may_show_run_on_os_login_mode = false; app_info->may_toggle_run_on_os_login_mode = false; - app_info->may_show_open_in_window = false; + app_info->is_locally_installed = + !extension->is_hosted_app() || + extensions::BookmarkAppIsLocallyInstalled(extension_service_->profile(), + extension); return app_info; } @@ -564,6 +568,11 @@ void AppHomePageHandler::OnWebAppUserDisplayModeChanged( page_->AddApp(CreateAppInfoPtrFromWebApp(app_id)); } +void AppHomePageHandler::OnWebAppInstalledWithOsHooks( + const web_app::AppId& app_id) { + page_->AddApp(CreateAppInfoPtrFromWebApp(app_id)); +} + void AppHomePageHandler::OnAppRegistrarDestroyed() { web_app_registrar_observation_.Reset(); } diff --git a/chrome/browser/ui/webui/app_home/app_home_page_handler.h b/chrome/browser/ui/webui/app_home/app_home_page_handler.h index 060625dfdea97..040f1baf5092c 100644 --- a/chrome/browser/ui/webui/app_home/app_home_page_handler.h +++ b/chrome/browser/ui/webui/app_home/app_home_page_handler.h @@ -60,7 +60,12 @@ class AppHomePageHandler ~AppHomePageHandler() override; // web_app::WebAppInstallManagerObserver: + // Listens to both `OnWebAppInstalled` and `OnWebAppInstalledWithOsHooks` as + // some type of installs, e.g. sync install only trigger `OnWebAppInstalled`. + // `OnWebAppInstalledWithOsHooks` also gets fired when an installed app gets + // locally installed. void OnWebAppInstalled(const web_app::AppId& app_id) override; + void OnWebAppInstalledWithOsHooks(const web_app::AppId& app_id) override; void OnWebAppWillBeUninstalled(const web_app::AppId& app_id) override; void OnWebAppInstallManagerDestroyed() override; diff --git a/chrome/browser/ui/webui/app_home/app_home_ui.cc b/chrome/browser/ui/webui/app_home/app_home_ui.cc index e6d5ac62b7b83..11a918eafe316 100644 --- a/chrome/browser/ui/webui/app_home/app_home_ui.cc +++ b/chrome/browser/ui/webui/app_home/app_home_ui.cc @@ -28,6 +28,7 @@ void AddAppHomeLocalizedStrings(content::WebUIDataSource* ui_source) { {"appLaunchAtStartupCheckboxLabel", IDS_ACCNAME_APP_HOME_LAUNCH_AT_STARTUP_CHECKBOX}, {"createShortcutForAppLabel", IDS_APP_HOME_CREATE_SHORTCUT}, + {"installLocallyLabel", IDS_APP_HOME_INSTALL_LOCALLY}, {"uninstallAppLabel", IDS_APP_HOME_UNINSTALL_APP}, {"appSettingsLabel", IDS_APP_HOME_APP_SETTINGS}, }; diff --git a/chrome/test/data/webui/app_home/app_list_test.ts b/chrome/test/data/webui/app_home/app_list_test.ts index 6fb2b8090bdc3..79989a21c80b6 100644 --- a/chrome/test/data/webui/app_home/app_list_test.ts +++ b/chrome/test/data/webui/app_home/app_list_test.ts @@ -40,7 +40,7 @@ suite('AppListTest', () => { mayShowRunOnOsLoginMode: true, mayToggleRunOnOsLoginMode: false, runOnOsLoginMode: RunOnOsLoginMode.kNotRun, - mayShowOpenInWindow: true, + isLocallyInstalled: true, openInWindow: false, }, { @@ -54,7 +54,7 @@ suite('AppListTest', () => { mayShowRunOnOsLoginMode: false, mayToggleRunOnOsLoginMode: false, runOnOsLoginMode: RunOnOsLoginMode.kNotRun, - mayShowOpenInWindow: false, + isLocallyInstalled: false, openInWindow: false, }, ], @@ -70,7 +70,7 @@ suite('AppListTest', () => { mayShowRunOnOsLoginMode: false, mayToggleRunOnOsLoginMode: false, runOnOsLoginMode: RunOnOsLoginMode.kNotRun, - mayShowOpenInWindow: false, + isLocallyInstalled: true, openInWindow: false, }; testBrowserProxy = new TestAppHomeBrowserProxy(apps); @@ -104,7 +104,7 @@ suite('AppListTest', () => { assertEquals( appItems[1]!.shadowRoot! .querySelector('.icon-container img')!.src, - apps.appList[1]!.iconUrl.url); + apps.appList[1]!.iconUrl.url + '?grayscale=true'); }); test('add/remove app', async () => { @@ -131,7 +131,7 @@ suite('AppListTest', () => { testAppInfo.name)); }); - test('context menu', () => { + test('context menu locally installed', () => { // Get the first app item. const appItem = appListElement.shadowRoot!.querySelector('app-item'); assertTrue(!!appItem); @@ -150,7 +150,7 @@ suite('AppListTest', () => { const openInWindow = contextMenu.querySelector('#open-in-window'); assertTrue(!!openInWindow); - assertEquals(openInWindow.hidden, !appInfo.mayShowOpenInWindow); + assertEquals(openInWindow.hidden, !appInfo.isLocallyInstalled); assertEquals( openInWindow.querySelector('cr-checkbox')!.checked, appInfo.openInWindow); @@ -170,6 +170,42 @@ suite('AppListTest', () => { assertTrue(!!contextMenu.querySelector('#create-shortcut')); assertTrue(!!contextMenu.querySelector('#uninstall')); assertTrue(!!contextMenu.querySelector('#app-settings')); + assertTrue(!!contextMenu.querySelector('#install-locally')); + + assertFalse( + contextMenu.querySelector('#create-shortcut')!.hidden); + assertFalse(contextMenu.querySelector('#uninstall')!.hidden); + assertFalse( + contextMenu.querySelector('#app-settings')!.hidden); + assertTrue( + contextMenu.querySelector('#install-locally')!.hidden); + }); + + test('context menu not locally installed', () => { + // Get the second app item that's not locally installed. + const appList = appListElement.shadowRoot!.querySelectorAll('app-item'); + assertEquals(appList.length, 2); + const appItem = appList[1]; + assertTrue(!!appItem); + + const contextMenu = + appListElement.shadowRoot!.querySelector('cr-action-menu'); + assertTrue(!!contextMenu); + assertTrue(contextMenu.hidden); + + appItem.dispatchEvent(new CustomEvent('contextmenu')); + assertFalse(contextMenu.hidden); + + assertTrue( + contextMenu.querySelector('#open-in-window')!.hidden); + assertTrue( + contextMenu.querySelector('#launch-on-startup')!.hidden); + assertTrue( + contextMenu.querySelector('#create-shortcut')!.hidden); + assertTrue(contextMenu.querySelector('#app-settings')!.hidden); + assertFalse(contextMenu.querySelector('#uninstall')!.hidden); + assertFalse( + contextMenu.querySelector('#install-locally')!.hidden); }); test('toggle open in window', () => { @@ -239,7 +275,7 @@ suite('AppListTest', () => { assertEquals(appInfo.runOnOsLoginMode, RunOnOsLoginMode.kWindowed); }); - test('click uninstall', () => { + test('click uninstall', async () => { const appItem = appListElement.shadowRoot!.querySelector('app-item'); assertTrue(!!appItem); @@ -250,11 +286,11 @@ suite('AppListTest', () => { assertTrue(!!uninstall); uninstall.click(); - testBrowserProxy.fakeHandler.whenCalled('uninstallApp') + await testBrowserProxy.fakeHandler.whenCalled('uninstallApp') .then((appId: string) => assertEquals(appId, apps.appList[0]!.id)); }); - test('click app settings', () => { + test('click app settings', async () => { const appItem = appListElement.shadowRoot!.querySelector('app-item'); assertTrue(!!appItem); @@ -265,11 +301,11 @@ suite('AppListTest', () => { assertTrue(!!appSettings); appSettings.click(); - testBrowserProxy.fakeHandler.whenCalled('showAppSettings') + await testBrowserProxy.fakeHandler.whenCalled('showAppSettings') .then((appId: string) => assertEquals(appId, apps.appList[0]!.id)); }); - test('click create shortcut', () => { + test('click create shortcut', async () => { const appItem = appListElement.shadowRoot!.querySelector('app-item'); assertTrue(!!appItem); @@ -281,8 +317,60 @@ suite('AppListTest', () => { assertTrue(!!createShortcut); createShortcut.click(); - testBrowserProxy.fakeHandler.whenCalled('createAppShortcut') + await testBrowserProxy.fakeHandler.whenCalled('createAppShortcut') .then((appId: string) => assertEquals(appId, apps.appList[0]!.id)); }); + test('click install locally', async () => { + const appItem = appListElement.shadowRoot!.querySelectorAll('app-item')[1]; + assertTrue(!!appItem); + + assertEquals( + appItem.shadowRoot! + .querySelector('.icon-container img')!.src, + apps.appList[1]!.iconUrl.url + '?grayscale=true'); + + appItem.dispatchEvent(new CustomEvent('contextmenu')); + + const contextMenu = + appListElement.shadowRoot!.querySelector('cr-action-menu'); + assertTrue(!!contextMenu); + + assertTrue( + contextMenu.querySelector('#open-in-window')!.hidden); + assertTrue( + contextMenu.querySelector('#create-shortcut')!.hidden); + assertTrue(contextMenu.querySelector('#app-settings')!.hidden); + assertFalse(contextMenu.querySelector('#uninstall')!.hidden); + + const installLocally = + appListElement.shadowRoot!.querySelector( + '#install-locally'); + assertTrue(!!installLocally); + assertFalse(installLocally.hidden); + + installLocally.click(); + await testBrowserProxy.fakeHandler.whenCalled('installAppLocally') + .then((appId: string) => assertEquals(appId, apps.appList[1]!.id)); + + await callbackRouterRemote.$.flushForTesting(); + flush(); + assertEquals( + appItem.shadowRoot! + .querySelector('.icon-container img')!.src, + apps.appList[1]!.iconUrl.url); + + appItem.dispatchEvent(new CustomEvent('contextmenu')); + + assertFalse( + contextMenu.querySelector('#open-in-window')!.hidden); + assertFalse( + contextMenu.querySelector('#create-shortcut')!.hidden); + assertFalse( + contextMenu.querySelector('#app-settings')!.hidden); + assertFalse(contextMenu.querySelector('#uninstall')!.hidden); + assertTrue( + contextMenu.querySelector('#install-locally')!.hidden); + }); + }); diff --git a/chrome/test/data/webui/app_home/test_app_home_browser_proxy.ts b/chrome/test/data/webui/app_home/test_app_home_browser_proxy.ts index 6909820b8d8ac..9b4226c3e0dd7 100644 --- a/chrome/test/data/webui/app_home/test_app_home_browser_proxy.ts +++ b/chrome/test/data/webui/app_home/test_app_home_browser_proxy.ts @@ -16,7 +16,12 @@ export class FakePageHandler extends TestBrowserProxy implements private callbackRouterRemote_: PageRemote; constructor(apps: AppList, callbackRouterRemote: PageRemote) { - super(['uninstallApp', 'showAppSettings', 'createAppShortcut']); + super([ + 'uninstallApp', + 'showAppSettings', + 'createAppShortcut', + 'installAppLocally', + ]); this.apps_ = apps; this.callbackRouterRemote_ = callbackRouterRemote; } @@ -52,7 +57,16 @@ export class FakePageHandler extends TestBrowserProxy implements launchDeprecatedAppDialog() {} - installAppLocally(_appId: string) {} + installAppLocally(appId: string) { + this.methodCalled('installAppLocally', appId); + for (const app of this.apps_.appList) { + if (app.id === appId) { + app.isLocallyInstalled = true; + this.callbackRouterRemote_.addApp(app); + break; + } + } + } setUserDisplayMode(appId: string, userDisplayMode: UserDisplayMode) { for (const app of this.apps_.appList) {