Skip to content

Commit

Permalink
dPWA: Introduce UserUninstalledPreinstalledWebAppPrefs
Browse files Browse the repository at this point in the history
This CL introduces the UserUninstalledPreinstalledWebAppPrefs that will contain data about default installed apps after they have been
uninstalled by the user and after the ExternallyInstalledWebAppPrefs
have been migrated to the web_app DB. This data needs to be left over
to ensure that when an user uninstalls a default app,it is not automatically reinstalled by ExternallyManagedAppManager::Synchronize.

Following CLs will perform the following tasks:
1. Write to the new pref and migrate all data from the current
external prefs and the kWasUninstalledByUser pref.
2. Start reading data from the new pref while making installation
decisions from the PreinstalledWebAppManager.

Bug: 1300382
Change-Id: I5d0113f164f94a729f4a37400959d8952dde0e53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3626154
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Dibyajyoti Pal <dibyapal@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002353}
  • Loading branch information
Dibyajyoti Pal authored and Chromium LUCI CQ committed May 11, 2022
1 parent 58c3853 commit 46ebd27
Show file tree
Hide file tree
Showing 8 changed files with 250 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ namespace {
// New fields must be added to BuildIndexJson().
constexpr char kInstalledWebApps[] = "InstalledWebApps";
constexpr char kPreinstalledWebAppConfigs[] = "PreinstalledWebAppConfigs";
constexpr char kPreinstalledAppsUninstalledByUserConfigs[] =
"PreinstalledAppsUninstalledByUserConfigs";
constexpr char kExternallyManagedWebAppPrefs[] = "ExternallyManagedWebAppPrefs";
constexpr char kIconErrorLog[] = "IconErrorLog";
constexpr char kInstallationProcessErrorLog[] = "InstallationProcessErrorLog";
Expand All @@ -61,6 +63,7 @@ base::Value BuildIndexJson() {

index.Append(kInstalledWebApps);
index.Append(kPreinstalledWebAppConfigs);
index.Append(kPreinstalledAppsUninstalledByUserConfigs);
index.Append(kExternallyManagedWebAppPrefs);
index.Append(kIconErrorLog);
index.Append(kInstallationProcessErrorLog);
Expand Down Expand Up @@ -187,6 +190,15 @@ base::Value BuildExternallyManagedWebAppPrefsJson(Profile* profile) {
return root;
}

base::Value BuildPreinstalledAppsUninstalledByUserJson(Profile* profile) {
base::Value::Dict root;
root.Set(kPreinstalledAppsUninstalledByUserConfigs,
profile->GetPrefs()
->GetDictionary(prefs::kUserUninstalledPreinstalledWebAppPref)
->Clone());
return base::Value(std::move(root));
}

base::Value BuildIconErrorLogJson(web_app::WebAppProvider& provider) {
base::Value root(base::Value::Type::DICTIONARY);

Expand Down Expand Up @@ -283,6 +295,7 @@ void BuildWebAppInternalsJson(
root.Append(BuildInstalledWebAppsJson(*provider));
root.Append(BuildPreinstalledWebAppConfigsJson(*provider));
root.Append(BuildExternallyManagedWebAppPrefsJson(profile));
root.Append(BuildPreinstalledAppsUninstalledByUserJson(profile));
root.Append(BuildIconErrorLogJson(*provider));
root.Append(BuildInstallProcessErrorLogJson(*provider));
#if BUILDFLAG(IS_MAC)
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/web_applications/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ source_set("web_applications") {
"system_web_apps/system_web_app_types.h",
"user_display_mode.cc",
"user_display_mode.h",
"user_uninstalled_preinstalled_web_app_prefs.cc",
"user_uninstalled_preinstalled_web_app_prefs.h",
"web_app.cc",
"web_app.h",
"web_app_audio_focus_id_map.cc",
Expand Down Expand Up @@ -492,6 +494,7 @@ source_set("web_applications_unit_tests") {
"preinstalled_web_app_utils_unittest.cc",
"preinstalled_web_apps/preinstalled_web_app_definition_utils_unittest.cc",
"test/web_app_test.h",
"user_uninstalled_preinstalled_web_app_prefs_unittest.cc",
"web_app_command_manager_unittest.cc",
"web_app_constants_unittest.cc",
"web_app_data_retriever_unittest.cc",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/web_applications/user_uninstalled_preinstalled_web_app_prefs.h"

#include <string>
#include <utility>

#include "base/containers/contains.h"
#include "base/strings/string_piece.h"
#include "base/values.h"
#include "content/public/browser/browser_thread.h"

namespace web_app {

UserUninstalledPreinstalledWebAppPrefs::UserUninstalledPreinstalledWebAppPrefs(
PrefService* pref_service)
: pref_service_(pref_service) {}

// static
void UserUninstalledPreinstalledWebAppPrefs::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterDictionaryPref(
prefs::kUserUninstalledPreinstalledWebAppPref);
}

void UserUninstalledPreinstalledWebAppPrefs::Add(
const AppId& app_id,
base::flat_set<GURL> install_urls) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
base::Value::List url_list;

AppendExistingInstallUrlsPerAppId(app_id, install_urls);

for (auto install_url : install_urls)
url_list.Append(install_url.spec());

DictionaryPrefUpdate update(pref_service_,
prefs::kUserUninstalledPreinstalledWebAppPref);
update->SetKey(app_id, base::Value(std::move(url_list)));
}

absl::optional<AppId>
UserUninstalledPreinstalledWebAppPrefs::LookUpAppIdByInstallUrl(
const GURL& url) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
const base::Value* ids_to_urls = pref_service_->GetDictionary(
prefs::kUserUninstalledPreinstalledWebAppPref);

if (!ids_to_urls || !url.is_valid())
return absl::nullopt;

for (auto it : ids_to_urls->DictItems()) {
const base::Value::List* urls = it.second.GetIfList();
if (!urls)
continue;
for (const base::Value& link : *urls) {
GURL install_url(link.GetString());
DCHECK(install_url.is_valid());
if (install_url == url)
return it.first;
}
}

return absl::nullopt;
}

bool UserUninstalledPreinstalledWebAppPrefs::DoesAppIdExist(
const AppId& app_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
const base::Value* ids_to_urls = pref_service_->GetDictionary(
prefs::kUserUninstalledPreinstalledWebAppPref);

if (!ids_to_urls)
return false;

const base::Value::Dict* pref_info = ids_to_urls->GetIfDict();
return pref_info && pref_info->contains(app_id);
}

void UserUninstalledPreinstalledWebAppPrefs::AppendExistingInstallUrlsPerAppId(
const AppId& app_id,
base::flat_set<GURL>& urls) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
const base::Value* ids_to_urls = pref_service_->GetDictionary(
prefs::kUserUninstalledPreinstalledWebAppPref);

if (!ids_to_urls)
return;

const base::Value::Dict* pref_info = ids_to_urls->GetIfDict();
if (!pref_info || !pref_info->contains(app_id))
return;

const base::Value::List* current_list = pref_info->FindList(app_id);
if (!current_list)
return;

for (const base::Value& url : *current_list) {
// This is being done so as to remove duplicate urls from being
// added to the list.
DCHECK(GURL(url.GetString()).is_valid());
urls.emplace(url.GetString());
}
}

} // namespace web_app
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_WEB_APPLICATIONS_USER_UNINSTALLED_PREINSTALLED_WEB_APP_PREFS_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_USER_UNINSTALLED_PREINSTALLED_WEB_APP_PREFS_H_

#include "base/containers/flat_set.h"
#include "base/memory/raw_ptr.h"
#include "chrome/browser/web_applications/web_app_id.h"
#include "chrome/common/pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "url/gurl.h"

class GURL;
class PrefService;

namespace user_prefs {
class PrefRegistrySyncable;
}

namespace web_app {
// A Prefs-backed map from preinstalled web app IDs to their install URLs.
//
// Preinstalled apps are the only type of externally installed apps that
// can be uninstalled by user. So if an user has uninstalled a preinstalled app,
// then it should stay uninstalled on startup.
// TODO(crbug.com/1029410): Ensure PreinstalledWebAppManager relies on
// UserUninstalledPreinstalledWebAppPrefs instead of
// ExternallyInstalledWebAppPrefs once removed.
//
// To prevent that, we keep track of the install URLs of preinstalled apps
// outside of the web_app DB so that on every startup, the WebAppSystem can
// keep the user uninstalled preinstalled apps uninstalled,
// thereby maintaining its synchronization.
//
// The prefs are stored in prefs::kUserUninstalledPreinstalledWebAppPref and
// they are stored as map<AppId, Set<Install URLs>>, e.g.
// {"app_id": {"https://install_url1.com", "https://install_url2.com"}}
//
// They can be seen on chrome://web-app-internals under the
// PreinstalledAppsUninstalledByUserConfigs json for debugging purposes.
class UserUninstalledPreinstalledWebAppPrefs {
public:
explicit UserUninstalledPreinstalledWebAppPrefs(PrefService* pref_service);
UserUninstalledPreinstalledWebAppPrefs(
const UserUninstalledPreinstalledWebAppPrefs&) = delete;
UserUninstalledPreinstalledWebAppPrefs& operator=(
const UserUninstalledPreinstalledWebAppPrefs&) = delete;

static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
void Add(const AppId& app_id, base::flat_set<GURL> install_urls);
absl::optional<AppId> LookUpAppIdByInstallUrl(const GURL& install_url);
bool DoesAppIdExist(const AppId& app_id);
void AppendExistingInstallUrlsPerAppId(const AppId& app_id,
base::flat_set<GURL>& urls);

private:
const raw_ptr<PrefService> pref_service_;
};

} // namespace web_app

#endif // CHROME_BROWSER_WEB_APPLICATIONS_USER_UNINSTALLED_PREINSTALLED_WEB_APP_PREFS_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/web_applications/user_uninstalled_preinstalled_web_app_prefs.h"

#include "base/containers/flat_set.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/web_app_id.h"
#include "url/gurl.h"

namespace web_app {

using UserUninstalledPreinstalledWebAppPrefsUnitTest = WebAppTest;

TEST_F(UserUninstalledPreinstalledWebAppPrefsUnitTest, BasicOperations) {
GURL url1("https://foo.com");
GURL url2("https://bar1.com");
GURL url3("https://bar2.com");
AppId app_id1 = "foo";
AppId app_id2 = "bar";

UserUninstalledPreinstalledWebAppPrefs preinstalled_prefs(
profile()->GetPrefs());
preinstalled_prefs.Add(app_id1, {url1});
preinstalled_prefs.Add(app_id2, {url2});
// To test that url3 gets appended.
preinstalled_prefs.Add(app_id2, {url3});

// Basic checks to verify app id exists in preinstalled prefs or not.
EXPECT_TRUE(preinstalled_prefs.DoesAppIdExist(app_id1));
EXPECT_TRUE(preinstalled_prefs.DoesAppIdExist(app_id2));
EXPECT_FALSE(preinstalled_prefs.DoesAppIdExist("baz"));

// Basic checks to verify if install_urls exist in preinstalled prefs or not.
EXPECT_EQ(app_id1, preinstalled_prefs.LookUpAppIdByInstallUrl(url1));
EXPECT_EQ(app_id2, preinstalled_prefs.LookUpAppIdByInstallUrl(url2));
EXPECT_EQ(app_id2, preinstalled_prefs.LookUpAppIdByInstallUrl(url3));
EXPECT_NE(app_id1, preinstalled_prefs.LookUpAppIdByInstallUrl(url3));
EXPECT_EQ(absl::nullopt, preinstalled_prefs.LookUpAppIdByInstallUrl(GURL()));
EXPECT_EQ(absl::nullopt, preinstalled_prefs.LookUpAppIdByInstallUrl(
GURL("https://baz.com")));
}

} // namespace web_app
2 changes: 2 additions & 0 deletions chrome/browser/web_applications/web_app_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "chrome/browser/web_applications/policy/web_app_policy_manager.h"
#include "chrome/browser/web_applications/preinstalled_web_app_manager.h"
#include "chrome/browser/web_applications/system_web_apps/system_web_app_manager.h"
#include "chrome/browser/web_applications/user_uninstalled_preinstalled_web_app_prefs.h"
#include "chrome/browser/web_applications/web_app_audio_focus_id_map.h"
#include "chrome/browser/web_applications/web_app_command_manager.h"
#include "chrome/browser/web_applications/web_app_database_factory.h"
Expand Down Expand Up @@ -373,6 +374,7 @@ void WebAppProvider::CheckIsConnected() const {
// static
void WebAppProvider::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
UserUninstalledPreinstalledWebAppPrefs::RegisterProfilePrefs(registry);
ExternallyInstalledWebAppPrefs::RegisterProfilePrefs(registry);
PreinstalledWebAppManager::RegisterProfilePrefs(registry);
WebAppPolicyManager::RegisterProfilePrefs(registry);
Expand Down
16 changes: 11 additions & 5 deletions chrome/common/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2057,13 +2057,19 @@ const char kWebAppInstallForceList[] = "profile.web_app.install.forcelist";
// A list of dictionaries for managing Web Apps.
const char kWebAppSettings[] = "profile.web_app.policy_settings";

// A list of dictionaries for managed configurations. Each dictionary contains
// 3 strings -- origin to be configured, link to the configuration, and the
// hashed value to that configuration.
// A map of App ID to install URLs to keep track of preinstalled web apps
// after they have been deleted.
const char kUserUninstalledPreinstalledWebAppPref[] =
"web_app.app_id.install_url";

// A list of dictionaries for managed configurations. Each dictionary
// contains 3 strings -- origin to be configured, link to the configuration,
// and the hashed value to that configuration.
const char kManagedConfigurationPerOrigin[] =
"profile.managed_configuration.list";
// Dictionary that maps the hash of the last downloded managed configuration for
// a particular origin.

// Dictionary that maps the hash of the last downloaded managed configuration
// for a particular origin.
const char kLastManagedConfigurationHashForOrigin[] =
"profile.managed_configuration.last_hash";

Expand Down
1 change: 1 addition & 0 deletions chrome/common/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ extern const char kDevToolsTCPDiscoveryConfig[];
extern const char kDiceSigninUserMenuPromoCount[];
#endif

extern const char kUserUninstalledPreinstalledWebAppPref[];
extern const char kManagedConfigurationPerOrigin[];
extern const char kLastManagedConfigurationHashForOrigin[];

Expand Down

0 comments on commit 46ebd27

Please sign in to comment.