[dPWA] Migrate Update<OsHook> functions to use Synchronize()
This CL updates all os integration registration/unregistration callsites
that are called outside of InstallOsHooks() / Uninstall(All)OsHooks()
and UpdateOsHooks(). This also updates tests that test the flow to
account for cases where OS integration sub managers are enabled or
disabled.
The os_integration_manager_unittest.cc file is not updated because that
uses the MockOsIntegrationManager which will be retired at the
end of this project.
This CL also does a small code health update where the test name
generation function is moved to web_app_test_utils.h and reused instead
of being defined everywhere else.
Bug: b/262399316
Change-Id: I2fa5ea07f7541a57a012e86f99684bc37f22ad0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4104720
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Mike Wasserman <msw@chromium.org>
Commit-Queue: Dibyajyoti Pal <dibyapal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1083487}
diff --git a/chrome/browser/ui/web_applications/web_app_browsertest.cc b/chrome/browser/ui/web_applications/web_app_browsertest.cc
index 3b38c43a..889ddc5 100644
--- a/chrome/browser/ui/web_applications/web_app_browsertest.cc
+++ b/chrome/browser/ui/web_applications/web_app_browsertest.cc
@@ -5,9 +5,11 @@
#include <codecvt>
#include <string>
+#include "base/barrier_callback.h"
#include "base/containers/contains.h"
#include "base/feature_list.h"
#include "base/files/file_util.h"
+#include "base/functional/callback.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h"
@@ -1602,9 +1604,28 @@
EXPECT_EQ(browser()->tab_strip_model()->count(), 1);
}
-using WebAppBrowserTest_UpdateShortcuts = WebAppBrowserTest;
+class WebAppBrowserTestUpdateShortcutResult
+ : public WebAppBrowserTest,
+ public ::testing::WithParamInterface<OsIntegrationSubManagersState> {
+ public:
+ WebAppBrowserTestUpdateShortcutResult() {
+ if (GetParam() == OsIntegrationSubManagersState::kEnabled) {
+ scoped_feature_list_.InitWithFeaturesAndParameters(
+ {{features::kOsIntegrationSubManagers, {{"stage", "write_config"}}}},
+ /*disabled_features=*/{});
+ } else {
+ scoped_feature_list_.InitWithFeatures(
+ {}, {features::kOsIntegrationSubManagers});
+ }
+ }
-IN_PROC_BROWSER_TEST_F(WebAppBrowserTest_UpdateShortcuts, UpdateShortcut) {
+ ~WebAppBrowserTestUpdateShortcutResult() override = default;
+
+ private:
+ base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_P(WebAppBrowserTestUpdateShortcutResult, UpdateShortcut) {
#if BUILDFLAG(IS_MAC)
base::AutoReset<bool> scope_shortcut_app_update(
&g_app_shims_allow_update_and_launch_in_tests, true);
@@ -1639,8 +1660,26 @@
base::HistogramTester tester;
base::test::TestFuture<Result> result;
+
+ auto synchronize_barrier = base::BarrierCallback<Result>(
+ /*num_callbacks=*/2,
+ base::BindOnce(
+ [&](base::OnceCallback<void(Result)> result_callback,
+ std::vector<Result> final_results) {
+ DCHECK_EQ(2u, final_results.size());
+ Result final_result = Result::kOk;
+ if (final_results[0] == Result::kError ||
+ final_results[1] == Result::kError) {
+ final_result = Result::kError;
+ }
+ std::move(result_callback).Run(final_result);
+ },
+ result.GetCallback()));
+
provider->os_integration_manager().UpdateShortcuts(
- app_id, "Manifest test app", result.GetCallback());
+ app_id, "Manifest test app", synchronize_barrier);
+ provider->os_integration_manager().Synchronize(
+ app_id, base::BindOnce(synchronize_barrier, Result::kOk));
ASSERT_TRUE(result.Wait());
EXPECT_THAT(result.Get(), testing::Eq(Result::kOk));
@@ -1663,6 +1702,13 @@
EXPECT_EQ(shortcut_info->title, u"test_app_2");
}
+INSTANTIATE_TEST_SUITE_P(
+ All,
+ WebAppBrowserTestUpdateShortcutResult,
+ ::testing::Values(OsIntegrationSubManagersState::kEnabled,
+ OsIntegrationSubManagersState::kDisabled),
+ test::GetOsIntegrationSubManagersTestName);
+
// Tests that reparenting a display: browser app tab results in a minimal-ui
// app window.
IN_PROC_BROWSER_TEST_F(WebAppBrowserTest, ReparentDisplayBrowserApp) {
diff --git a/chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc b/chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc
index ea28fe7..993eddc 100644
--- a/chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc
+++ b/chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc
@@ -349,6 +349,8 @@
options.add_to_desktop = locations.on_desktop;
options.add_to_quick_launch_bar = locations.in_quick_launch_bar;
options.os_hooks[OsHookType::kRunOnOsLogin] = locations.in_startup;
+ // TODO(crbug.com/1401125): Remove InstallOsHooks() once OS integration
+ // sub managers have been implemented.
os_integration_manager_->InstallOsHooks(app_id, base::DoNothing(), nullptr,
options);
os_integration_manager_->Synchronize(app_id, base::DoNothing());
diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
index 2ddb1036..fbe510a 100644
--- a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
+++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
@@ -1467,6 +1467,8 @@
base::BindOnce(&AppLauncherHandler::OnOsHooksInstalled,
weak_ptr_factory_.GetWeakPtr(), app_id));
+ // TODO(crbug.com/1401125): Remove InstallOsHooks() once OS integration
+ // sub managers have been implemented.
web_app_provider_->os_integration_manager().InstallOsHooks(
app_id, os_hooks_barrier, /*web_app_info=*/nullptr, std::move(options));
web_app_provider_->os_integration_manager().Synchronize(
diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler_unittest.cc b/chrome/browser/ui/webui/ntp/app_launcher_handler_unittest.cc
index 4bd03b2..c939e224 100644
--- a/chrome/browser/ui/webui/ntp/app_launcher_handler_unittest.cc
+++ b/chrome/browser/ui/webui/ntp/app_launcher_handler_unittest.cc
@@ -275,11 +275,4 @@
AppLauncherHandlerTest,
::testing::Values(OsIntegrationSubManagersState::kEnabled,
OsIntegrationSubManagersState::kDisabled),
- [](const ::testing::TestParamInfo<OsIntegrationSubManagersState>& info) {
- switch (info.param) {
- case OsIntegrationSubManagersState::kEnabled:
- return "OSIntegrationSubManagers_Enabled";
- case OsIntegrationSubManagersState::kDisabled:
- return "OSIntegrationSubManagers_Disabled";
- }
- });
+ web_app::test::GetOsIntegrationSubManagersTestName);
diff --git a/chrome/browser/ui/webui/settings/protocol_handlers_handler.cc b/chrome/browser/ui/webui/settings/protocol_handlers_handler.cc
index c605911..b0ff859c 100644
--- a/chrome/browser/ui/webui/settings/protocol_handlers_handler.cc
+++ b/chrome/browser/ui/webui/settings/protocol_handlers_handler.cc
@@ -355,9 +355,13 @@
handler.web_app_id().value(), handler.protocol());
// Update registration with the OS.
+ // TODO(crbug.com/1401125): Remove UpdateProtocolHandlers() once OS
+ // integration sub managers have been implemented.
lock.os_integration_manager().UpdateProtocolHandlers(
handler.web_app_id().value(),
/*force_shortcut_updates_if_needed=*/true, base::DoNothing());
+ lock.os_integration_manager().Synchronize(
+ handler.web_app_id().value(), base::DoNothing());
},
std::move(handler)));
diff --git a/chrome/browser/web_applications/commands/manifest_update_data_fetch_command.cc b/chrome/browser/web_applications/commands/manifest_update_data_fetch_command.cc
index b80ba8c..4a02627 100644
--- a/chrome/browser/web_applications/commands/manifest_update_data_fetch_command.cc
+++ b/chrome/browser/web_applications/commands/manifest_update_data_fetch_command.cc
@@ -9,6 +9,7 @@
#include <utility>
#include <vector>
+#include "base/barrier_callback.h"
#include "base/feature_list.h"
#include "base/memory/weak_ptr.h"
#include "base/metrics/histogram_macros.h"
@@ -576,11 +577,26 @@
return;
}
- lock_->os_integration_manager().UpdateUrlHandlers(
- app_id_,
+ auto manifest_update_origin_update_callback = base::BindOnce(
+ &ManifestUpdateDataFetchCommand::OnWebAppOriginAssociationsUpdated,
+ AsWeakPtr());
+ auto synchronize_callback = base::BarrierCallback<bool>(
+ /*num_callbacks = */ 2,
base::BindOnce(
- &ManifestUpdateDataFetchCommand::OnWebAppOriginAssociationsUpdated,
- AsWeakPtr()));
+ [&](base::OnceCallback<void(bool)> final_callback,
+ std::vector<bool> final_results) {
+ DCHECK_EQ(2u, final_results.size());
+ bool final_result = final_results[0] && final_results[1];
+ std::move(final_callback).Run(final_result);
+ },
+ std::move(manifest_update_origin_update_callback)));
+
+ // TODO(crbug.com/1401125): Remove UpdateUrlHandlers() once OS integration
+ // sub managers have been implemented.
+ lock_->os_integration_manager().UpdateUrlHandlers(app_id_,
+ synchronize_callback);
+ lock_->os_integration_manager().Synchronize(
+ app_id_, base::BindOnce(synchronize_callback, true));
}
void ManifestUpdateDataFetchCommand::OnWebAppOriginAssociationsUpdated(
diff --git a/chrome/browser/web_applications/commands/run_on_os_login_command.cc b/chrome/browser/web_applications/commands/run_on_os_login_command.cc
index 5f97687..c192884 100644
--- a/chrome/browser/web_applications/commands/run_on_os_login_command.cc
+++ b/chrome/browser/web_applications/commands/run_on_os_login_command.cc
@@ -193,6 +193,9 @@
return;
}
+ // TODO(crbug.com/1401125): Remove InstallOsHooks() and UninstallOsHooks()
+ // once OS integration
+ // sub managers have been implemented.
if (login_mode_.value() == RunOnOsLoginMode::kNotRun) {
web_app::OsHooksOptions os_hooks;
os_hooks[web_app::OsHookType::kRunOnOsLogin] = true;
diff --git a/chrome/browser/web_applications/commands/run_on_os_login_command_unittest.cc b/chrome/browser/web_applications/commands/run_on_os_login_command_unittest.cc
index 86c6a80..1cee1ad 100644
--- a/chrome/browser/web_applications/commands/run_on_os_login_command_unittest.cc
+++ b/chrome/browser/web_applications/commands/run_on_os_login_command_unittest.cc
@@ -513,13 +513,6 @@
RunOnOsLoginCommandTest,
::testing::Values(OsIntegrationSubManagersState::kEnabled,
OsIntegrationSubManagersState::kDisabled),
- [](const ::testing::TestParamInfo<OsIntegrationSubManagersState>& info) {
- switch (info.param) {
- case OsIntegrationSubManagersState::kEnabled:
- return "OSIntegrationSubManagers_Enabled";
- case OsIntegrationSubManagersState::kDisabled:
- return "OSIntegrationSubManagers_Disabled";
- }
- });
+ test::GetOsIntegrationSubManagersTestName);
} // namespace web_app
diff --git a/chrome/browser/web_applications/commands/update_file_handler_command.cc b/chrome/browser/web_applications/commands/update_file_handler_command.cc
index 9052041..ad50b97 100644
--- a/chrome/browser/web_applications/commands/update_file_handler_command.cc
+++ b/chrome/browser/web_applications/commands/update_file_handler_command.cc
@@ -7,7 +7,8 @@
#include <memory>
#include <utility>
-#include "base/callback.h"
+#include "base/barrier_callback.h"
+#include "base/functional/callback.h"
#include "base/memory/ptr_util.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/web_applications/locks/app_lock.h"
@@ -19,6 +20,27 @@
namespace web_app {
+namespace {
+
+base::RepeatingCallback<void(Result)> CreateBarrierForSynchronizeWithResult(
+ base::OnceCallback<void(Result)> final_callback) {
+ return base::BarrierCallback<Result>(
+ /*num_callbacks=*/2, base::BindOnce(
+ [](base::OnceCallback<void(Result)> callback,
+ std::vector<Result> combined_results) {
+ DCHECK_EQ(2u, combined_results.size());
+ Result final_result = Result::kOk;
+ if (combined_results[0] == Result::kError ||
+ combined_results[1] == Result::kError) {
+ final_result = Result::kError;
+ }
+ std::move(callback).Run(final_result);
+ },
+ std::move(final_callback)));
+}
+
+} // namespace
+
// static
std::unique_ptr<UpdateFileHandlerCommand>
UpdateFileHandlerCommand::CreateForPersistUserChoice(
@@ -93,6 +115,10 @@
debug_info_.Set("was_update_required", true);
+ auto callback_for_synchronize = CreateBarrierForSynchronizeWithResult(
+ base::BindOnce(&UpdateFileHandlerCommand::OnFileHandlerUpdated,
+ weak_factory_.GetWeakPtr(), file_handling_enabled));
+
#if BUILDFLAG(IS_MAC)
// On Mac, the file handlers are encoded in the app shortcut. First
// unregister the file handlers (verifying that it finishes
@@ -105,25 +131,29 @@
&unregister_file_handlers_result));
DCHECK_EQ(Result::kOk, unregister_file_handlers_result);
+ lock_->os_integration_manager().Synchronize(
+ app_id_, base::BindOnce(callback_for_synchronize, Result::kOk));
+
// If we're enabling file handling, yet this app does not have any file
// handlers there is no need to update the shortcut, as doing so would be a
// no-op anyway.
const apps::FileHandlers* handlers =
lock_->registrar().GetAppFileHandlers(app_id_);
+
if (file_handling_enabled && (!handlers || handlers->empty())) {
- OnFileHandlerUpdated(file_handling_enabled, Result::kOk);
+ callback_for_synchronize.Run(Result::kOk);
} else {
- lock_->os_integration_manager().UpdateShortcuts(
- app_id_, /*old_name=*/{},
- base::BindOnce(&UpdateFileHandlerCommand::OnFileHandlerUpdated,
- weak_factory_.GetWeakPtr(), file_handling_enabled));
+ // TODO(crbug.com/1401125): Remove UpdateFileHandlers() and
+ // UpdateShortcuts() once OS integration sub managers have been implemented.
+ lock_->os_integration_manager().UpdateShortcuts(app_id_, /*old_name=*/{},
+ callback_for_synchronize);
}
#else
+ lock_->os_integration_manager().UpdateFileHandlers(app_id_, action,
+ callback_for_synchronize);
+ lock_->os_integration_manager().Synchronize(
+ app_id_, base::BindOnce(callback_for_synchronize, Result::kOk));
- lock_->os_integration_manager().UpdateFileHandlers(
- app_id_, action,
- base::BindOnce(&UpdateFileHandlerCommand::OnFileHandlerUpdated,
- weak_factory_.GetWeakPtr(), file_handling_enabled));
#endif
}
diff --git a/chrome/browser/web_applications/commands/update_file_handler_command_unittest.cc b/chrome/browser/web_applications/commands/update_file_handler_command_unittest.cc
index 93cf33e..adea45a 100644
--- a/chrome/browser/web_applications/commands/update_file_handler_command_unittest.cc
+++ b/chrome/browser/web_applications/commands/update_file_handler_command_unittest.cc
@@ -10,8 +10,10 @@
#include "chrome/browser/web_applications/test/fake_web_app_provider.h"
#include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
#include "chrome/browser/web_applications/test/web_app_test.h"
+#include "chrome/browser/web_applications/test/web_app_test_utils.h"
#include "chrome/browser/web_applications/web_app_command_scheduler.h"
#include "chrome/browser/web_applications/web_app_constants.h"
+#include "chrome/common/chrome_features.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/features.h"
#include "url/gurl.h"
@@ -19,14 +21,24 @@
namespace web_app {
namespace {
-class UpdateFileHandlerCommandTest : public WebAppTest {
+class UpdateFileHandlerCommandTest
+ : public WebAppTest,
+ public ::testing::WithParamInterface<OsIntegrationSubManagersState> {
public:
const char* kTestAppName = "test app";
const GURL kTestAppUrl = GURL("https://example.com");
UpdateFileHandlerCommandTest() {
- scoped_feature_list_.InitAndEnableFeature(
- blink::features::kFileHandlingAPI);
+ if (GetParam() == OsIntegrationSubManagersState::kEnabled) {
+ scoped_feature_list_.InitWithFeaturesAndParameters(
+ {{features::kOsIntegrationSubManagers, {{"stage", "write_config"}}},
+ {blink::features::kFileHandlingAPI, {}}},
+ /*disabled_features=*/{});
+ } else {
+ scoped_feature_list_.InitWithFeatures(
+ {blink::features::kFileHandlingAPI},
+ {features::kOsIntegrationSubManagers});
+ }
}
~UpdateFileHandlerCommandTest() override = default;
@@ -70,7 +82,7 @@
base::test::ScopedFeatureList scoped_feature_list_;
};
-TEST_F(UpdateFileHandlerCommandTest, UserChoiceAllowPersisted) {
+TEST_P(UpdateFileHandlerCommandTest, UserChoiceAllowPersisted) {
const AppId app_id =
test::InstallDummyWebApp(profile(), kTestAppName, kTestAppUrl);
EXPECT_EQ(
@@ -90,7 +102,7 @@
app_id));
}
-TEST_F(UpdateFileHandlerCommandTest, UserChoiceDisallowPersisted) {
+TEST_P(UpdateFileHandlerCommandTest, UserChoiceDisallowPersisted) {
const AppId app_id =
test::InstallDummyWebApp(profile(), kTestAppName, kTestAppUrl);
EXPECT_EQ(
@@ -110,7 +122,7 @@
app_id));
}
-TEST_F(UpdateFileHandlerCommandTest, UpdateFileHandler) {
+TEST_P(UpdateFileHandlerCommandTest, UpdateFileHandler) {
const AppId app_id =
test::InstallDummyWebApp(profile(), kTestAppName, kTestAppUrl);
EXPECT_EQ(
@@ -142,5 +154,12 @@
app_id));
}
+INSTANTIATE_TEST_SUITE_P(
+ All,
+ UpdateFileHandlerCommandTest,
+ ::testing::Values(OsIntegrationSubManagersState::kEnabled,
+ OsIntegrationSubManagersState::kDisabled),
+ test::GetOsIntegrationSubManagersTestName);
+
} // namespace
} // namespace web_app
diff --git a/chrome/browser/web_applications/commands/update_protocol_handler_approval_command_browsertest.cc b/chrome/browser/web_applications/commands/update_protocol_handler_approval_command_browsertest.cc
index b5f7980d..2a90937b 100644
--- a/chrome/browser/web_applications/commands/update_protocol_handler_approval_command_browsertest.cc
+++ b/chrome/browser/web_applications/commands/update_protocol_handler_approval_command_browsertest.cc
@@ -323,7 +323,8 @@
All,
UpdateProtocolHandlerApprovalCommandTest,
::testing::Values(OsIntegrationSubManagersState::kEnabled,
- OsIntegrationSubManagersState::kDisabled));
+ OsIntegrationSubManagersState::kDisabled),
+ test::GetOsIntegrationSubManagersTestName);
} // namespace
} // namespace web_app
diff --git a/chrome/browser/web_applications/manifest_update_manager_browsertest.cc b/chrome/browser/web_applications/manifest_update_manager_browsertest.cc
index c5047973..6f37712 100644
--- a/chrome/browser/web_applications/manifest_update_manager_browsertest.cc
+++ b/chrome/browser/web_applications/manifest_update_manager_browsertest.cc
@@ -3455,7 +3455,28 @@
}
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
-IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest_UrlHandlers,
+class ManifestUpdateBrowserTestUrlHandlerSynchronize
+ : public ManifestUpdateManagerBrowserTest_UrlHandlers,
+ public ::testing::WithParamInterface<OsIntegrationSubManagersState> {
+ public:
+ ManifestUpdateBrowserTestUrlHandlerSynchronize() {
+ if (GetParam() == OsIntegrationSubManagersState::kEnabled) {
+ scoped_feature_list_.InitWithFeaturesAndParameters(
+ {{features::kOsIntegrationSubManagers, {{"stage", "write_config"}}}},
+ /*disabled_features=*/{});
+ } else {
+ scoped_feature_list_.InitWithFeatures(
+ {}, {features::kOsIntegrationSubManagers});
+ }
+ }
+
+ ~ManifestUpdateBrowserTestUrlHandlerSynchronize() override = default;
+
+ private:
+ base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_P(ManifestUpdateBrowserTestUrlHandlerSynchronize,
NoHandlersChangeUpdateAssociations) {
constexpr char kManifestTemplate[] = R"(
{
@@ -3515,6 +3536,13 @@
matches = UrlHandlerManagerImpl::GetUrlHandlerMatches(cmd);
ASSERT_EQ(matches.size(), 0u);
}
+
+INSTANTIATE_TEST_SUITE_P(
+ All,
+ ManifestUpdateBrowserTestUrlHandlerSynchronize,
+ ::testing::Values(OsIntegrationSubManagersState::kEnabled,
+ OsIntegrationSubManagersState::kDisabled),
+ test::GetOsIntegrationSubManagersTestName);
#endif
IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
diff --git a/chrome/browser/web_applications/os_integration/os_integration_manager.h b/chrome/browser/web_applications/os_integration/os_integration_manager.h
index ffea2b58..813c54d 100644
--- a/chrome/browser/web_applications/os_integration/os_integration_manager.h
+++ b/chrome/browser/web_applications/os_integration/os_integration_manager.h
@@ -114,8 +114,12 @@
virtual void Start();
- // Start OS Integration synchronization from external points. This should be
- // the only point of call into OsIntegrationManager from external places.
+ // Start OS Integration synchronization from external callsites. This should
+ // be the only point of call into OsIntegrationManager from external places
+ // after the OS integration sub managers have been implemented.
+ // TODO(crbug.com/1401125): Remove all install, uninstall and update functions
+ // from this file once all OS Integration sub managers have been implemented,
+ // connected to the web_app system and tested.
virtual void Synchronize(const AppId& app_id, base::OnceClosure callback);
// Install all needed OS hooks for the web app.
diff --git a/chrome/browser/web_applications/os_integration/protocol_handling_sub_manager_unittest.cc b/chrome/browser/web_applications/os_integration/protocol_handling_sub_manager_unittest.cc
index 612d567..fc40959a 100644
--- a/chrome/browser/web_applications/os_integration/protocol_handling_sub_manager_unittest.cc
+++ b/chrome/browser/web_applications/os_integration/protocol_handling_sub_manager_unittest.cc
@@ -229,14 +229,7 @@
ProtocolHandlingSubManagerTest,
::testing::Values(OsIntegrationSubManagersState::kEnabled,
OsIntegrationSubManagersState::kDisabled),
- [](const ::testing::TestParamInfo<OsIntegrationSubManagersState>& info) {
- switch (info.param) {
- case OsIntegrationSubManagersState::kEnabled:
- return "OSIntegrationSubManagers_Enabled";
- case OsIntegrationSubManagersState::kDisabled:
- return "OSIntegrationSubManagers_Disabled";
- }
- });
+ test::GetOsIntegrationSubManagersTestName);
} // namespace
diff --git a/chrome/browser/web_applications/os_integration/shortcut_handling_sub_manager_browsertest.cc b/chrome/browser/web_applications/os_integration/shortcut_handling_sub_manager_browsertest.cc
index d282f57f..b17a8c8 100644
--- a/chrome/browser/web_applications/os_integration/shortcut_handling_sub_manager_browsertest.cc
+++ b/chrome/browser/web_applications/os_integration/shortcut_handling_sub_manager_browsertest.cc
@@ -156,14 +156,7 @@
ShortcutHandlingSubManagerBrowserTest,
::testing::Values(OsIntegrationSubManagersState::kEnabled,
OsIntegrationSubManagersState::kDisabled),
- [](const ::testing::TestParamInfo<OsIntegrationSubManagersState>& info) {
- switch (info.param) {
- case OsIntegrationSubManagersState::kEnabled:
- return "OSIntegrationSubManagers_Enabled";
- case OsIntegrationSubManagersState::kDisabled:
- return "OSIntegrationSubManagers_Disabled";
- }
- });
+ test::GetOsIntegrationSubManagersTestName);
} // namespace
diff --git a/chrome/browser/web_applications/test/web_app_test_utils.cc b/chrome/browser/web_applications/test/web_app_test_utils.cc
index 124d749..eac7334 100644
--- a/chrome/browser/web_applications/test/web_app_test_utils.cc
+++ b/chrome/browser/web_applications/test/web_app_test_utils.cc
@@ -326,6 +326,16 @@
}
}
+std::string GetOsIntegrationSubManagersTestName(
+ const ::testing::TestParamInfo<OsIntegrationSubManagersState>& info) {
+ switch (info.param) {
+ case OsIntegrationSubManagersState::kEnabled:
+ return "OSIntegrationSubManagers_Enabled";
+ case OsIntegrationSubManagersState::kDisabled:
+ return "OSIntegrationSubManagers_Disabled";
+ }
+}
+
std::unique_ptr<WebApp> CreateWebApp(const GURL& start_url,
WebAppManagement::Type source_type) {
const AppId app_id = GenerateAppId(/*manifest_id=*/absl::nullopt, start_url);
diff --git a/chrome/browser/web_applications/test/web_app_test_utils.h b/chrome/browser/web_applications/test/web_app_test_utils.h
index f29083fd..36e3196 100644
--- a/chrome/browser/web_applications/test/web_app_test_utils.h
+++ b/chrome/browser/web_applications/test/web_app_test_utils.h
@@ -52,6 +52,9 @@
std::string GetExternalPrefMigrationTestName(
const ::testing::TestParamInfo<ExternalPrefMigrationTestCases>& info);
+std::string GetOsIntegrationSubManagersTestName(
+ const ::testing::TestParamInfo<OsIntegrationSubManagersState>& info);
+
// Do not use this for installation! Instead, use the utilities in
// web_app_install_test_util.h.
std::unique_ptr<WebApp> CreateWebApp(
diff --git a/chrome/browser/web_applications/web_app_install_finalizer.cc b/chrome/browser/web_applications/web_app_install_finalizer.cc
index 7ec1f16..67ee012 100644
--- a/chrome/browser/web_applications/web_app_install_finalizer.cc
+++ b/chrome/browser/web_applications/web_app_install_finalizer.cc
@@ -549,6 +549,8 @@
&WebAppInstallFinalizer::OnInstallHooksFinished,
weak_ptr_factory_.GetWeakPtr(), std::move(callback), app_id));
+ // TODO(crbug.com/1401125): Remove InstallOsHooks() once OS integration
+ // sub managers have been implemented.
os_integration_manager_->InstallOsHooks(
app_id, os_hooks_barrier, /*web_app_info=*/nullptr, hooks_options);
os_integration_manager_->Synchronize(
@@ -608,6 +610,8 @@
weak_ptr_factory_.GetWeakPtr(), std::move(callback),
app_id, old_name));
+ // TODO(crbug.com/1401125): Remove UpdateOsHooks() once OS integration
+ // sub managers have been implemented.
os_integration_manager_->UpdateOsHooks(app_id, old_name,
file_handlers_need_os_update,
web_app_info, os_hooks_barrier);
diff --git a/chrome/browser/web_applications/web_app_install_finalizer_unittest.cc b/chrome/browser/web_applications/web_app_install_finalizer_unittest.cc
index 1c7f8375..2d95d009 100644
--- a/chrome/browser/web_applications/web_app_install_finalizer_unittest.cc
+++ b/chrome/browser/web_applications/web_app_install_finalizer_unittest.cc
@@ -462,13 +462,6 @@
WebAppInstallFinalizerUnitTest,
::testing::Values(OsIntegrationSubManagersState::kEnabled,
OsIntegrationSubManagersState::kDisabled),
- [](const ::testing::TestParamInfo<OsIntegrationSubManagersState>& info) {
- switch (info.param) {
- case OsIntegrationSubManagersState::kEnabled:
- return "OSIntegrationSubManagers_Enabled";
- case OsIntegrationSubManagersState::kDisabled:
- return "OSIntegrationSubManagers_Disabled";
- }
- });
+ test::GetOsIntegrationSubManagersTestName);
} // namespace web_app
diff --git a/chrome/browser/web_applications/web_app_install_utils.cc b/chrome/browser/web_applications/web_app_install_utils.cc
index e685e4e..efb20f6 100644
--- a/chrome/browser/web_applications/web_app_install_utils.cc
+++ b/chrome/browser/web_applications/web_app_install_utils.cc
@@ -944,6 +944,8 @@
options.os_hooks[OsHookType::kUninstallationViaOsSettings] = true;
auto os_hooks_barrier =
OsIntegrationManager::GetBarrierForSynchronize(std::move(callback));
+ // TODO(crbug.com/1401125): Remove InstallOsHooks() once OS integration
+ // sub managers have been implemented.
os_integration_manager.InstallOsHooks(web_app->app_id(), os_hooks_barrier,
nullptr, options);
os_integration_manager.Synchronize(
@@ -969,6 +971,8 @@
if (user_installable_before_install && !user_installable_after_install) {
OsHooksOptions options;
options[OsHookType::kUninstallationViaOsSettings] = true;
+ // TODO(crbug.com/1401125): Remove UninstallOsHooks() once OS integration
+ // sub managers have been implemented.
os_integration_manager.UninstallOsHooks(web_app->app_id(), options,
base::DoNothing());
os_integration_manager.Synchronize(web_app->app_id(), base::DoNothing());
diff --git a/chrome/browser/web_applications/web_app_uninstall_job.cc b/chrome/browser/web_applications/web_app_uninstall_job.cc
index 04661cd..7b3c736b4 100644
--- a/chrome/browser/web_applications/web_app_uninstall_job.cc
+++ b/chrome/browser/web_applications/web_app_uninstall_job.cc
@@ -90,6 +90,9 @@
auto synchronize_barrier = OsIntegrationManager::GetBarrierForSynchronize(
base::BindOnce(&WebAppUninstallJob::OnOsHooksUninstalled,
weak_ptr_factory_.GetWeakPtr()));
+
+ // TODO(crbug.com/1401125): Remove UninstallAllOsHooks() once OS integration
+ // sub managers have been implemented.
os_integration_manager.UninstallAllOsHooks(app_id_, synchronize_barrier);
os_integration_manager.Synchronize(
app_id_, base::BindOnce(synchronize_barrier, OsHooksErrors()));