Commit 22f03e26 authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Fix ExternalWebAppMigrationBrowserTest.UserUninstalledExtensionApp

This CL updates the preinstalled web app to exclude configs that want to
uninstall_and_replace preinstalled Chrome apps that have been
uninstalled by the user.

Bug: 1123435
Change-Id: I5522523680d4a5a20fa3f1f615bd6b2824692e9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425604
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarGlen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811176}
parent 31304689
......@@ -110,6 +110,7 @@ source_set("web_applications") {
"//components/services/app_service/public/cpp:protocol_handling",
"//components/sync",
"//content/public/browser",
"//extensions/browser",
"//skia",
]
......
......@@ -21,6 +21,7 @@
#include "base/metrics/histogram_functions.h"
#include "base/no_destructor.h"
#include "base/path_service.h"
#include "base/stl_util.h"
#include "base/task/post_task.h"
#include "base/task/thread_pool.h"
#include "base/threading/scoped_blocking_call.h"
......@@ -35,6 +36,9 @@
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_paths.h"
#include "content/public/browser/browser_thread.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_system.h"
#include "url/gurl.h"
#if defined(OS_CHROMEOS)
......@@ -236,13 +240,40 @@ void ExternalWebAppManager::ParseConfigs(ConsumeParsedConfigs callback,
void ExternalWebAppManager::PostProcessConfigs(ConsumeInstallOptions callback,
ParsedConfigs parsed_configs) {
// Add hard coded configs.
PreinstalledWebApps preinstalled_web_apps = GetPreinstalledWebApps();
for (ExternalInstallOptions& options : preinstalled_web_apps.options)
parsed_configs.options_list.push_back(std::move(options));
parsed_configs.disabled_count += preinstalled_web_apps.disabled_count;
// Save this as we may remove apps due to user uninstall (not the same as
// being disabled).
int enabled_count = parsed_configs.options_list.size();
// Remove web apps whose replace target was uninstalled.
if (extensions::ExtensionSystem::Get(profile_)) {
auto* extension_prefs = extensions::ExtensionPrefs::Get(profile_);
auto* extension_registry = extensions::ExtensionRegistry::Get(profile_);
base::EraseIf(
parsed_configs.options_list,
[&](const ExternalInstallOptions& options) {
for (const AppId& app_id : options.uninstall_and_replace) {
if (extension_registry->GetInstalledExtension(app_id))
return false;
}
for (const AppId& app_id : options.uninstall_and_replace) {
if (extension_prefs->IsExternalExtensionUninstalled(app_id))
return true;
}
return false;
});
}
base::UmaHistogramCounts100(ExternalWebAppManager::kHistogramEnabledCount,
parsed_configs.options_list.size());
enabled_count);
base::UmaHistogramCounts100(ExternalWebAppManager::kHistogramDisabledCount,
parsed_configs.disabled_count);
base::UmaHistogramCounts100(ExternalWebAppManager::kHistogramConfigErrorCount,
......
......@@ -411,17 +411,8 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppMigrationBrowserTest,
SyncExternalExtensions();
EXPECT_FALSE(IsExtensionAppInstalled());
// The web app should *not* be installed, since the user chose to uninstall
// the extension app that it's replacing. Unfortunately, this doesn't
// currently work.
// TODO(alancutter): Fix this so that default web apps that are replacing
// extensions are only installed if the corresponding extension is still
// installed.
// SyncExternalWebApps(/*expect_install=*/false,
// /*expect_uninstall=*/false); EXPECT_FALSE(IsWebAppInstalled());
SyncExternalWebApps(/*expect_install=*/true, /*expect_uninstall=*/false);
EXPECT_TRUE(IsWebAppInstalled());
SyncExternalWebApps(/*expect_install=*/false, /*expect_uninstall=*/false);
EXPECT_FALSE(IsWebAppInstalled());
}
}
......
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