Commit ef2601fc authored by Alexey Baskakov's avatar Alexey Baskakov Committed by Chromium LUCI CQ

webapps: Plumb the reinstall milestone config parameter for default apps.

Background: We update default web apps icons only if user opens
the web app url (DidFinishNavigation, throttled at daily basis).

This new force_reinstall_for_milestone parameter in the external .json
config allows to trigger force reinstall on a particular milestone.

Bug: 1150687
Change-Id: I86956537e933d34ba6746ea3d7fa44dfe5ff1a0f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2620921
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842414}
parent 40bcc6d3
......@@ -61,6 +61,7 @@ bool ExternalInstallOptions::operator==(
options.bypass_service_worker_check,
options.require_manifest,
options.force_reinstall,
options.force_reinstall_for_milestone,
options.wait_for_windows_closed,
options.install_placeholder,
options.reinstall_placeholder,
......@@ -133,6 +134,8 @@ std::ostream& operator<<(std::ostream& out,
<< install_options.bypass_service_worker_check
<< "\n require_manifest: " << install_options.require_manifest
<< "\n force_reinstall: " << install_options.force_reinstall
<< "\n force_reinstall_for_milestone: "
<< install_options.force_reinstall_for_milestone
<< "\n wait_for_windows_closed: "
<< install_options.wait_for_windows_closed
<< "\n install_placeholder: " << install_options.install_placeholder
......
......@@ -109,6 +109,12 @@ struct ExternalInstallOptions {
// Whether the app should be reinstalled even if it is already installed.
bool force_reinstall = false;
// Whether we should update the app if the browser's binary milestone number
// goes from less the milestone specified to greater or equal than the
// milestone specified. For example, if this value is 89 then we update the
// app on all browser upgrades from <89 to >=89. The update happens only once.
base::Optional<int> force_reinstall_for_milestone;
// Whether we should wait for all app windows being closed before reinstalling
// the placeholder.
bool wait_for_windows_closed = false;
......
......@@ -392,6 +392,16 @@ void ExternalWebAppManager::PostProcessConfigs(ConsumeInstallOptions callback,
debug_info_->enabled_configs = parsed_configs.options_list;
}
// Triggers |force_reinstall| in PendingAppManager if milestone increments
// across |force_reinstall_for_milestone|.
for (ExternalInstallOptions& options : parsed_configs.options_list) {
if (options.force_reinstall_for_milestone &&
IsReinstallPastMilestoneNeededSinceLastSync(
options.force_reinstall_for_milestone.value())) {
options.force_reinstall = true;
}
}
UMA_HISTOGRAM_COUNTS_100(kHistogramEnabledCount,
parsed_configs.options_list.size());
UMA_HISTOGRAM_COUNTS_100(kHistogramDisabledCount, disabled_count);
......@@ -417,8 +427,8 @@ void ExternalWebAppManager::OnExternalWebAppsSynchronized(
PendingAppManager::SynchronizeCallback callback,
std::map<GURL, PendingAppManager::InstallResult> install_results,
std::map<GURL, bool> uninstall_results) {
// Note that we are storing the Chrome version instead of a "has synchronised"
// bool in order to do version update specific logic in the future.
// Note that we are storing the Chrome version (milestone number) instead of a
// "has synchronised" bool in order to do version update specific logic.
profile_->GetPrefs()->SetString(
prefs::kWebAppsLastPreinstallSynchronizeVersion,
version_info::GetMajorVersionNumber());
......@@ -491,6 +501,17 @@ bool ExternalWebAppManager::IsNewUser() {
return ExternallyInstalledWebAppPrefs(prefs).HasNoApps();
}
bool ExternalWebAppManager::IsReinstallPastMilestoneNeededSinceLastSync(
int force_reinstall_for_milestone) {
PrefService* prefs = profile_->GetPrefs();
std::string last_preinstall_synchronize_milestone =
prefs->GetString(prefs::kWebAppsLastPreinstallSynchronizeVersion);
return IsReinstallPastMilestoneNeeded(last_preinstall_synchronize_milestone,
version_info::GetMajorVersionNumber(),
force_reinstall_for_milestone);
}
ExternalWebAppManager::DebugInfo::DebugInfo() = default;
ExternalWebAppManager::DebugInfo::~DebugInfo() = default;
......
......@@ -119,6 +119,11 @@ class ExternalWebAppManager {
// profile.
bool IsNewUser();
// |force_reinstall_for_milestone| is a major version number. See
// components/version_info/version_info.h.
bool IsReinstallPastMilestoneNeededSinceLastSync(
int force_reinstall_for_milestone);
PendingAppManager* pending_app_manager_ = nullptr;
Profile* const profile_;
......
......@@ -148,6 +148,10 @@ constexpr char kOfflineManifestIconAnyPngs[] = "icon_any_pngs";
// "theme_color": "red"
constexpr char kOfflineManifestThemeColorArgbHex[] = "theme_color_argb_hex";
// Contains numeric milestone number M like 89 (the Chrome version). The app
// gets updated if browser's binary milestone number goes from <M to >=M.
constexpr char kForceReinstallForMilestone[] = "force_reinstall_for_milestone";
} // namespace
OptionsOrError ParseConfig(FileUtilsWrapper& file_utils,
......@@ -356,6 +360,16 @@ OptionsOrError ParseConfig(FileUtilsWrapper& file_utils,
" set with no ", kOfflineManifest, " available"});
}
// force_reinstall_for_milestone
value = app_config.FindKey(kForceReinstallForMilestone);
if (value) {
if (!value->is_int()) {
return base::StrCat({file.AsUTF8Unsafe(), " had an invalid ",
kForceReinstallForMilestone});
}
options.force_reinstall_for_milestone = value->GetInt();
}
return options;
}
......@@ -495,4 +509,23 @@ WebApplicationInfoFactoryOrError ParseOfflineManifest(
std::move(app_info));
}
bool IsReinstallPastMilestoneNeeded(
base::StringPiece last_preinstall_synchronize_milestone_str,
base::StringPiece current_milestone_str,
int force_reinstall_for_milestone) {
int last_preinstall_synchronize_milestone = 0;
if (!base::StringToInt(last_preinstall_synchronize_milestone_str,
&last_preinstall_synchronize_milestone)) {
return false;
}
int current_milestone = 0;
if (!base::StringToInt(current_milestone_str, &current_milestone))
return false;
return last_preinstall_synchronize_milestone <
force_reinstall_for_milestone &&
current_milestone >= force_reinstall_for_milestone;
}
} // namespace web_app
......@@ -8,6 +8,7 @@
#include <string>
#include "base/optional.h"
#include "base/strings/string_piece.h"
#include "chrome/browser/web_applications/components/external_install_options.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
......@@ -36,6 +37,16 @@ WebApplicationInfoFactoryOrError ParseOfflineManifest(
const base::FilePath& file,
const base::Value& offline_manifest);
// Returns true if we need to update the app. |current_milestone_str| is the
// browser's binary milestone number.
// |last_preinstall_synchronize_milestone_str| is a previous milestone for the
// user's data state. For example, if |force_reinstall_for_milestone| value is
// 89 then we need to update the app on all browser upgrades from <89 to >=89.
bool IsReinstallPastMilestoneNeeded(
base::StringPiece last_preinstall_synchronize_milestone_str,
base::StringPiece current_milestone_str,
int force_reinstall_for_milestone);
} // namespace web_app
#endif // CHROME_BROWSER_WEB_APPLICATIONS_EXTERNAL_WEB_APP_UTILS_H_
......@@ -443,4 +443,51 @@ TEST_F(ExternalWebAppUtilsTest, OfflineManifestThemeColorArgbHex) {
)")) << "theme_color_argb_hex is valid";
}
TEST_F(ExternalWebAppUtilsTest, ForceReinstallForMilestone) {
base::Optional<ExternalInstallOptions> non_number = ParseConfig(R"(
{
"app_url": "https://test.org",
"launch_container": "window",
"force_reinstall_for_milestone": "error",
"user_type": ["test"]
}
)");
EXPECT_FALSE(non_number.has_value());
base::Optional<ExternalInstallOptions> number = ParseConfig(R"(
{
"app_url": "https://test.org",
"launch_container": "window",
"force_reinstall_for_milestone": 89,
"user_type": ["test"]
}
)");
EXPECT_TRUE(number.has_value());
EXPECT_EQ(89, number->force_reinstall_for_milestone);
}
TEST_F(ExternalWebAppUtilsTest, IsReinstallPastMilestoneNeeded) {
// Arguments: last_preinstall_synchronize_milestone, current_milestone,
// force_reinstall_for_milestone.
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("87", "87", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("87", "88", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("88", "88", 89));
EXPECT_TRUE(IsReinstallPastMilestoneNeeded("88", "89", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("89", "89", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("89", "90", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("90", "90", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("90", "91", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("91", "91", 89));
// Long jumps:
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("80", "85", 89));
EXPECT_TRUE(IsReinstallPastMilestoneNeeded("80", "100", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("90", "95", 89));
// Wrong input:
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("error", "90", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("88", "error", 89));
EXPECT_FALSE(IsReinstallPastMilestoneNeeded("error", "error", 0));
}
} // namespace web_app
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