diff --git a/chrome/browser/web_applications/isolated_web_apps/cleanup_orphaned_isolated_web_apps_command.cc b/chrome/browser/web_applications/isolated_web_apps/cleanup_orphaned_isolated_web_apps_command.cc index 864bda5353742f..d3a6fde04c6b03 100644 --- a/chrome/browser/web_applications/isolated_web_apps/cleanup_orphaned_isolated_web_apps_command.cc +++ b/chrome/browser/web_applications/isolated_web_apps/cleanup_orphaned_isolated_web_apps_command.cc @@ -43,7 +43,19 @@ std::set RetrieveAllInstalledIsolatedWebAppsPaths( continue; } - isolated_web_apps_paths.insert(owned_bundle->GetPath(profile.GetPath())); + isolated_web_apps_paths.insert( + owned_bundle->GetPath(profile.GetPath()).DirName()); + + if (const auto& pending_update_info = + isolation_data->pending_update_info()) { + const auto* pending_update_location = + absl::get_if( + &pending_update_info->location.variant()); + if (pending_update_location) { + isolated_web_apps_paths.insert( + pending_update_location->GetPath(profile.GetPath()).DirName()); + } + } } } return isolated_web_apps_paths; @@ -67,8 +79,9 @@ std::set RetrieveAllIsolatedWebAppsDirectories( } bool DeleteOrphanedIsolatedWebApps(std::vector paths) { - return base::ranges::all_of( - paths, [](const base::FilePath& path) { return base::DeleteFile(path); }); + return base::ranges::all_of(paths, [](const base::FilePath& path) { + return base::DeletePathRecursively(path); + }); } base::expected #include "base/base64.h" +#include "base/check_deref.h" +#include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" +#include "base/functional/callback_helpers.h" #include "base/scoped_observation.h" #include "base/strings/strcat.h" #include "base/strings/string_util.h" @@ -107,6 +110,14 @@ constexpr std::string_view kServiceWorkerScript = R"( }); )"; +#if BUILDFLAG(IS_CHROMEOS_ASH) +void CheckBundleExists(Profile* profile, const base::FilePath& directory) { + base::ScopedAllowBlockingForTesting allow_blocking; + EXPECT_TRUE(base::DirectoryExists( + CHECK_DEREF(profile).GetPath().Append(kIwaDirName).Append(directory))); +} +#endif // BUILDFLAG(IS_CHROMEOS_ASH) + class ServiceWorkerVersionStartedRunningWaiter : public content::ServiceWorkerContextObserver { public: @@ -351,6 +362,68 @@ IN_PROC_BROWSER_TEST_F(IsolatedWebAppUpdateManagerBrowserTest, title_watcher.AlsoWaitForTitle(u"3.0.4"); EXPECT_THAT(title_watcher.WaitAndGetTitle(), Eq(u"7.0.6")); } + +IN_PROC_BROWSER_TEST_F(IsolatedWebAppUpdateManagerBrowserTest, + PendingUpdateDoesNotGetCleanedUp) { + profile()->GetPrefs()->SetList( + prefs::kIsolatedWebAppInstallForceList, + base::Value::List().Append( + update_server_mixin_.CreateForceInstallPolicyEntry( + GetWebBundleId()))); + + SessionStartupPref pref(SessionStartupPref::LAST); + SessionStartupPref::SetStartupPref(profile(), pref); + + profile()->GetPrefs()->CommitPendingWrite(); + + web_app::WebAppTestInstallObserver(browser()->profile()) + .BeginListeningAndWait({GetAppId()}); + + // Open the app to prevent the update from being applied. + OpenApp(GetAppId()); + EXPECT_THAT(provider().ui_manager().GetNumWindowsForApp(GetAppId()), Eq(1ul)); + + AddUpdate(); + EXPECT_THAT(provider().iwa_update_manager().DiscoverUpdatesNow(), Eq(1ul)); + + ASSERT_TRUE(base::test::RunUntil([this]() { + const WebApp* app = GetIsolatedWebApp(GetAppId()); + return app->isolation_data()->pending_update_info().has_value(); + })); + + const auto& isolation_data = GetIsolatedWebApp(GetAppId())->isolation_data(); + const auto& app_location = + base::FilePath(absl::get_if( + &isolation_data->location.variant()) + ->dir_name_ascii()); + const auto& app_update_location = base::FilePath( + absl::get_if( + &isolation_data->pending_update_info()->location.variant()) + ->dir_name_ascii()); + + // Check that both IWA directories (currently running instance and the update) + // are there. + CheckBundleExists(profile(), app_location); + CheckBundleExists(profile(), app_update_location); + + // Run the cleanup while both bundles are there. + base::test::TestFuture< + base::expected> + future; + provider().scheduler().CleanupOrphanedIsolatedApps(future.GetCallback()); + const bool command_successful = + future + .Get>() + .has_value(); + ASSERT_TRUE(command_successful); + + // Neither of the bundles should be deleted. + CheckBundleExists(profile(), app_location); + CheckBundleExists(profile(), app_update_location); +} + #endif // !BUILDFLAG(IS_CHROMEOS_LACROS) class IsolatedWebAppUpdateManagerWithKeyRotationBrowserTest diff --git a/chrome/browser/web_applications/isolated_web_apps/policy/isolated_web_app_policy_manager_ash_browsertest.cc b/chrome/browser/web_applications/isolated_web_apps/policy/isolated_web_app_policy_manager_ash_browsertest.cc index 6a0266c6482356..fbd8140533f0e0 100644 --- a/chrome/browser/web_applications/isolated_web_apps/policy/isolated_web_app_policy_manager_ash_browsertest.cc +++ b/chrome/browser/web_applications/isolated_web_apps/policy/isolated_web_app_policy_manager_ash_browsertest.cc @@ -582,10 +582,13 @@ class CleanupOrphanedBundlesTest void SimulateOrphanedBundle(Profile* profile, const std::string& bundle_directory) { base::ScopedAllowBlockingForTesting allow_blocking; - ASSERT_TRUE(base::CreateDirectory(CHECK_DEREF(profile) - .GetPath() - .Append(kIwaDirName) - .Append(bundle_directory))); + auto base_path = CHECK_DEREF(profile) + .GetPath() + .Append(kIwaDirName) + .Append(bundle_directory); + ASSERT_TRUE(base::CreateDirectory(base_path)); + ASSERT_TRUE( + base::WriteFile(base_path.Append("main.swbn"), "Sample content")); } bool CheckBundleDirectoryExists(Profile* profile,