Commit 41aa7347 authored by Fabio Rocha's avatar Fabio Rocha Committed by Chromium LUCI CQ

desktop-pwas: Hook up OSIntegrationManager to ProtocolHandlerManager

This CL adds a couple of hooks for the OSIntegrationManager to call
into the upcoming implementation of the ProtocolHandlerManager. There
should be no observable difference in behavior with this CL.

Bug: 1019239
Change-Id: I88455e0dd868d0b8944de231dd2cbf5f1a89516a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577769
Commit-Queue: Fabio Rocha <fabio.rocha@microsoft.com>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837773}
parent b232d470
...@@ -2202,6 +2202,7 @@ static_library("browser") { ...@@ -2202,6 +2202,7 @@ static_library("browser") {
"//components/security_state/core", "//components/security_state/core",
"//components/send_tab_to_self", "//components/send_tab_to_self",
"//components/services/app_service/public/cpp:intents", "//components/services/app_service/public/cpp:intents",
"//components/services/app_service/public/cpp:protocol_handling",
"//components/services/heap_profiling", "//components/services/heap_profiling",
"//components/services/language_detection/public/cpp", "//components/services/language_detection/public/cpp",
"//components/services/language_detection/public/mojom", "//components/services/language_detection/public/mojom",
...@@ -4175,6 +4176,7 @@ static_library("browser") { ...@@ -4175,6 +4176,7 @@ static_library("browser") {
"//components/services/app_service/public/cpp:icon_loader", "//components/services/app_service/public/cpp:icon_loader",
"//components/services/app_service/public/cpp:intents", "//components/services/app_service/public/cpp:intents",
"//components/services/app_service/public/cpp:preferred_apps", "//components/services/app_service/public/cpp:preferred_apps",
"//components/services/app_service/public/cpp:protocol_handling",
"//components/services/app_service/public/cpp:publisher", "//components/services/app_service/public/cpp:publisher",
"//components/shared_highlighting/core/common", "//components/shared_highlighting/core/common",
"//components/soda:constants", "//components/soda:constants",
......
...@@ -635,6 +635,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientWebAppsBMOSyncTest, NoShortcutsCreatedOnSync) { ...@@ -635,6 +635,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientWebAppsBMOSyncTest, NoShortcutsCreatedOnSync) {
expected_os_hook_requests[OsHookType::kShortcutsMenu] = true; expected_os_hook_requests[OsHookType::kShortcutsMenu] = true;
expected_os_hook_requests[OsHookType::kUninstallationViaOsSettings] = true; expected_os_hook_requests[OsHookType::kUninstallationViaOsSettings] = true;
expected_os_hook_requests[OsHookType::kFileHandlers] = true; expected_os_hook_requests[OsHookType::kFileHandlers] = true;
expected_os_hook_requests[OsHookType::kProtocolHandlers] = true;
EXPECT_EQ(expected_os_hook_requests, last_options->os_hooks); EXPECT_EQ(expected_os_hook_requests, last_options->os_hooks);
EXPECT_TRUE(last_options->add_to_desktop); EXPECT_TRUE(last_options->add_to_desktop);
EXPECT_FALSE(last_options->add_to_quick_launch_bar); EXPECT_FALSE(last_options->add_to_quick_launch_bar);
......
...@@ -208,6 +208,11 @@ void OsIntegrationManager::UninstallOsHooks(const AppId& app_id, ...@@ -208,6 +208,11 @@ void OsIntegrationManager::UninstallOsHooks(const AppId& app_id,
if (os_hooks[OsHookType::kFileHandlers]) if (os_hooks[OsHookType::kFileHandlers])
UnregisterFileHandlers(app_id); UnregisterFileHandlers(app_id);
// TODO(https://crbug.com/1108109) we should return the result of protocol
// handler unregistration and record errors during unregistration.
if (os_hooks[OsHookType::kProtocolHandlers])
UnregisterProtocolHandlers(app_id);
// There is a chance uninstallation point was created with feature flag // There is a chance uninstallation point was created with feature flag
// enabled so we need to clean it up regardless of feature flag state. // enabled so we need to clean it up regardless of feature flag state.
if (os_hooks[OsHookType::kUninstallationViaOsSettings]) if (os_hooks[OsHookType::kUninstallationViaOsSettings])
...@@ -327,6 +332,21 @@ void OsIntegrationManager::RegisterFileHandlers( ...@@ -327,6 +332,21 @@ void OsIntegrationManager::RegisterFileHandlers(
std::move(callback).Run(true); std::move(callback).Run(true);
} }
void OsIntegrationManager::RegisterProtocolHandlers(
const AppId& app_id,
base::OnceCallback<void(bool success)> callback) {
if (!protocol_handler_manager_) {
std::move(callback).Run(true);
return;
}
// TODO(crbug.com/1019239): Call protocol_handler_manager_ implementation.
// TODO(crbug.com/1087219): callback should be run after all hooks are
// deployed, need to refactor protocol_handler_manager to allow this.
std::move(callback).Run(true);
}
void OsIntegrationManager::RegisterShortcutsMenu( void OsIntegrationManager::RegisterShortcutsMenu(
const AppId& app_id, const AppId& app_id,
const std::vector<WebApplicationShortcutsMenuItemInfo>& const std::vector<WebApplicationShortcutsMenuItemInfo>&
...@@ -424,6 +444,13 @@ void OsIntegrationManager::UnregisterFileHandlers(const AppId& app_id) { ...@@ -424,6 +444,13 @@ void OsIntegrationManager::UnregisterFileHandlers(const AppId& app_id) {
file_handler_manager_->DisableAndUnregisterOsFileHandlers(app_id); file_handler_manager_->DisableAndUnregisterOsFileHandlers(app_id);
} }
void OsIntegrationManager::UnregisterProtocolHandlers(const AppId& app_id) {
if (!protocol_handler_manager_)
return;
// TODO(crbug.com/1019239): Call protocol_handler_manager_ implementation.
}
void OsIntegrationManager::UnregisterWebAppOsUninstallation( void OsIntegrationManager::UnregisterWebAppOsUninstallation(
const AppId& app_id) { const AppId& app_id) {
if (ShouldRegisterUninstallationViaOsSettingsWithOs()) if (ShouldRegisterUninstallationViaOsSettingsWithOs())
...@@ -455,6 +482,11 @@ void OsIntegrationManager::OnShortcutsCreated( ...@@ -455,6 +482,11 @@ void OsIntegrationManager::OnShortcutsCreated(
OsHookType::kFileHandlers)); OsHookType::kFileHandlers));
} }
if (options.os_hooks[OsHookType::kProtocolHandlers]) {
RegisterProtocolHandlers(app_id, barrier->CreateBarrierCallbackForType(
OsHookType::kProtocolHandlers));
}
if (options.os_hooks[OsHookType::kShortcuts] && if (options.os_hooks[OsHookType::kShortcuts] &&
options.add_to_quick_launch_bar) { options.add_to_quick_launch_bar) {
AddAppToQuickLaunchBar(app_id); AddAppToQuickLaunchBar(app_id);
......
...@@ -164,6 +164,9 @@ class OsIntegrationManager { ...@@ -164,6 +164,9 @@ class OsIntegrationManager {
virtual void RegisterFileHandlers( virtual void RegisterFileHandlers(
const AppId& app_id, const AppId& app_id,
base::OnceCallback<void(bool success)> callback); base::OnceCallback<void(bool success)> callback);
virtual void RegisterProtocolHandlers(
const AppId& app_id,
base::OnceCallback<void(bool success)> callback);
virtual void RegisterShortcutsMenu( virtual void RegisterShortcutsMenu(
const AppId& app_id, const AppId& app_id,
const std::vector<WebApplicationShortcutsMenuItemInfo>& const std::vector<WebApplicationShortcutsMenuItemInfo>&
...@@ -191,6 +194,7 @@ class OsIntegrationManager { ...@@ -191,6 +194,7 @@ class OsIntegrationManager {
std::unique_ptr<ShortcutInfo> shortcut_info, std::unique_ptr<ShortcutInfo> shortcut_info,
DeleteShortcutsCallback callback); DeleteShortcutsCallback callback);
virtual void UnregisterFileHandlers(const AppId& app_id); virtual void UnregisterFileHandlers(const AppId& app_id);
virtual void UnregisterProtocolHandlers(const AppId& app_id);
virtual void UnregisterWebAppOsUninstallation(const AppId& app_id); virtual void UnregisterWebAppOsUninstallation(const AppId& app_id);
// Utility mathods: // Utility mathods:
......
...@@ -43,6 +43,12 @@ class MockOsIntegrationManager : public OsIntegrationManager { ...@@ -43,6 +43,12 @@ class MockOsIntegrationManager : public OsIntegrationManager {
base::OnceCallback<void(bool success)> callback), base::OnceCallback<void(bool success)> callback),
(override)); (override));
MOCK_METHOD(void,
RegisterProtocolHandlers,
(const AppId& app_id,
base::OnceCallback<void(bool success)> callback),
(override));
MOCK_METHOD(void, MOCK_METHOD(void,
RegisterShortcutsMenu, RegisterShortcutsMenu,
(const AppId& app_id, (const AppId& app_id,
...@@ -92,6 +98,10 @@ class MockOsIntegrationManager : public OsIntegrationManager { ...@@ -92,6 +98,10 @@ class MockOsIntegrationManager : public OsIntegrationManager {
DeleteShortcutsCallback callback), DeleteShortcutsCallback callback),
(override)); (override));
MOCK_METHOD(void, UnregisterFileHandlers, (const AppId& app_id), (override)); MOCK_METHOD(void, UnregisterFileHandlers, (const AppId& app_id), (override));
MOCK_METHOD(void,
UnregisterProtocolHandlers,
(const AppId& app_id),
(override));
MOCK_METHOD(void, MOCK_METHOD(void,
UnregisterWebAppOsUninstallation, UnregisterWebAppOsUninstallation,
(const AppId& app_id), (const AppId& app_id),
...@@ -176,6 +186,7 @@ TEST_F(OsIntegrationManagerTest, InstallOsHooksEverything) { ...@@ -176,6 +186,7 @@ TEST_F(OsIntegrationManagerTest, InstallOsHooksEverything) {
EXPECT_CALL(manager, CreateShortcuts(app_id, true, testing::_)) EXPECT_CALL(manager, CreateShortcuts(app_id, true, testing::_))
.WillOnce(base::test::RunOnceCallback<2>(true)); .WillOnce(base::test::RunOnceCallback<2>(true));
EXPECT_CALL(manager, RegisterFileHandlers(app_id, testing::_)).Times(1); EXPECT_CALL(manager, RegisterFileHandlers(app_id, testing::_)).Times(1);
EXPECT_CALL(manager, RegisterProtocolHandlers(app_id, testing::_)).Times(1);
EXPECT_CALL(manager, AddAppToQuickLaunchBar(app_id)).Times(1); EXPECT_CALL(manager, AddAppToQuickLaunchBar(app_id)).Times(1);
EXPECT_CALL(manager, ReadAllShortcutsMenuIconsAndRegisterShortcutsMenu( EXPECT_CALL(manager, ReadAllShortcutsMenuIconsAndRegisterShortcutsMenu(
app_id, testing::_)) app_id, testing::_))
...@@ -190,6 +201,7 @@ TEST_F(OsIntegrationManagerTest, InstallOsHooksEverything) { ...@@ -190,6 +201,7 @@ TEST_F(OsIntegrationManagerTest, InstallOsHooksEverything) {
run_loop.Run(); run_loop.Run();
EXPECT_TRUE(install_results[OsHookType::kShortcuts]); EXPECT_TRUE(install_results[OsHookType::kShortcuts]);
EXPECT_TRUE(install_results[OsHookType::kFileHandlers]); EXPECT_TRUE(install_results[OsHookType::kFileHandlers]);
EXPECT_TRUE(install_results[OsHookType::kProtocolHandlers]);
EXPECT_TRUE(install_results[OsHookType::kRunOnOsLogin]); EXPECT_TRUE(install_results[OsHookType::kRunOnOsLogin]);
// Note: We asked for these to be installed, but their methods were not // Note: We asked for these to be installed, but their methods were not
// called. This is because the features are turned off. We only set these // called. This is because the features are turned off. We only set these
...@@ -223,6 +235,7 @@ TEST_F(OsIntegrationManagerTest, UninstallOsHooksEverything) { ...@@ -223,6 +235,7 @@ TEST_F(OsIntegrationManagerTest, UninstallOsHooksEverything) {
testing::_, testing::_)) testing::_, testing::_))
.WillOnce(base::test::RunOnceCallback<3>(true)); .WillOnce(base::test::RunOnceCallback<3>(true));
EXPECT_CALL(manager, UnregisterFileHandlers(app_id)).Times(1); EXPECT_CALL(manager, UnregisterFileHandlers(app_id)).Times(1);
EXPECT_CALL(manager, UnregisterProtocolHandlers(app_id)).Times(1);
EXPECT_CALL(manager, UnregisterWebAppOsUninstallation(app_id)).Times(1); EXPECT_CALL(manager, UnregisterWebAppOsUninstallation(app_id)).Times(1);
EXPECT_CALL(manager, UnregisterShortcutsMenu(app_id)) EXPECT_CALL(manager, UnregisterShortcutsMenu(app_id))
.WillOnce(testing::Return(true)); .WillOnce(testing::Return(true));
...@@ -231,6 +244,7 @@ TEST_F(OsIntegrationManagerTest, UninstallOsHooksEverything) { ...@@ -231,6 +244,7 @@ TEST_F(OsIntegrationManagerTest, UninstallOsHooksEverything) {
run_loop.Run(); run_loop.Run();
EXPECT_TRUE(uninstall_results[OsHookType::kShortcuts]); EXPECT_TRUE(uninstall_results[OsHookType::kShortcuts]);
EXPECT_TRUE(uninstall_results[OsHookType::kFileHandlers]); EXPECT_TRUE(uninstall_results[OsHookType::kFileHandlers]);
EXPECT_TRUE(uninstall_results[OsHookType::kProtocolHandlers]);
EXPECT_TRUE(uninstall_results[OsHookType::kRunOnOsLogin]); EXPECT_TRUE(uninstall_results[OsHookType::kRunOnOsLogin]);
EXPECT_TRUE(uninstall_results[OsHookType::kShortcutsMenu]); EXPECT_TRUE(uninstall_results[OsHookType::kShortcutsMenu]);
EXPECT_TRUE(uninstall_results[OsHookType::kUninstallationViaOsSettings]); EXPECT_TRUE(uninstall_results[OsHookType::kUninstallationViaOsSettings]);
......
...@@ -46,9 +46,10 @@ enum Type { ...@@ -46,9 +46,10 @@ enum Type {
kShortcutsMenu, kShortcutsMenu,
kUninstallationViaOsSettings, kUninstallationViaOsSettings,
kFileHandlers, kFileHandlers,
kMaxValue = kFileHandlers, kProtocolHandlers,
kMaxValue = kProtocolHandlers,
}; };
} } // namespace OsHookType
// The result of an attempted web app installation, uninstallation or update. // The result of an attempted web app installation, uninstallation or update.
// //
......
...@@ -272,6 +272,7 @@ void PendingAppInstallTask::OnWebAppInstalled(bool is_placeholder, ...@@ -272,6 +272,7 @@ void PendingAppInstallTask::OnWebAppInstalled(bool is_placeholder,
// TODO(crbug.com/1087219): Determine if |register_file_handlers| should be // TODO(crbug.com/1087219): Determine if |register_file_handlers| should be
// configured from somewhere else rather than always true. // configured from somewhere else rather than always true.
options.os_hooks[OsHookType::kFileHandlers] = true; options.os_hooks[OsHookType::kFileHandlers] = true;
options.os_hooks[OsHookType::kProtocolHandlers] = true;
options.os_hooks[OsHookType::kUninstallationViaOsSettings] = true; options.os_hooks[OsHookType::kUninstallationViaOsSettings] = true;
os_integration_manager_->InstallOsHooks( os_integration_manager_->InstallOsHooks(
......
...@@ -817,6 +817,7 @@ void WebAppInstallTask::OnInstallFinalizedCreateShortcuts( ...@@ -817,6 +817,7 @@ void WebAppInstallTask::OnInstallFinalizedCreateShortcuts(
// TODO(crbug.com/1087219): Determine if file handlers should be // TODO(crbug.com/1087219): Determine if file handlers should be
// configured from somewhere else rather than always true. // configured from somewhere else rather than always true.
options.os_hooks[OsHookType::kFileHandlers] = true; options.os_hooks[OsHookType::kFileHandlers] = true;
options.os_hooks[OsHookType::kProtocolHandlers] = true;
options.os_hooks[OsHookType::kUninstallationViaOsSettings] = true; options.os_hooks[OsHookType::kUninstallationViaOsSettings] = true;
if (install_source_ == webapps::WebappInstallSource::SYNC) if (install_source_ == webapps::WebappInstallSource::SYNC)
......
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