Commit e01f2ab2 authored by Asami Doi's avatar Asami Doi Committed by Chromium LUCI CQ

Consider WARN_NOT_OFFLINE_CAPABLE as not an error.

This CL replaces `data.errors.empty()` with `data.NoBlockingErrors()`
because we don't want to consider the WARN_NOT_OFFLINE_CAPABLE state as
an error. The state just logs a warning message to DevTools and
Application > Manifest > Installability and it should not affect the
behavior.

Bug: 1157809
Change-Id: I03cf02f134ff4f96441723bf893aac380160cced
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2592245Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Asami Doi <asamidoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837869}
parent 807176b3
......@@ -143,7 +143,7 @@ void WebApkUpdateDataFetcher::OnDidGetInstallableData(
// observing too. It is based on our assumption that it is invalid for
// web developers to change the Web Manifest location. When it does
// change, we will treat the new Web Manifest as the one of another WebAPK.
if (!data.errors.empty() || data.manifest->IsEmpty() ||
if (!data.NoBlockingErrors() || data.manifest->IsEmpty() ||
web_manifest_url_ != data.manifest_url ||
!AreWebManifestUrlsWebApkCompatible(*data.manifest)) {
return;
......
......@@ -277,7 +277,7 @@ void AddToHomescreenDataFetcher::OnDidPerformInstallableCheck(
return;
bool webapk_compatible =
(data.errors.empty() && data.valid_manifest && data.has_worker &&
(data.NoBlockingErrors() && data.valid_manifest && data.has_worker &&
AreWebManifestUrlsWebApkCompatible(*data.manifest));
observer_->OnUserTitleAvailable(
webapk_compatible ? shortcut_info_.name : shortcut_info_.user_title,
......
......@@ -320,7 +320,7 @@ void AppBannerManager::OnDidGetManifest(const InstallableData& data) {
if (DidRetryInstallableManagerRequest(data))
return;
UpdateState(State::ACTIVE);
if (!data.errors.empty()) {
if (!data.NoBlockingErrors()) {
Stop(data.errors[0]);
return;
}
......@@ -368,11 +368,8 @@ void AppBannerManager::OnDidPerformInstallableWebAppCheck(
if (data.has_worker && data.valid_manifest)
TrackDisplayEvent(DISPLAY_EVENT_WEB_APP_BANNER_REQUESTED);
auto error = data.errors.empty() ? NO_ERROR_DETECTED : data.errors[0];
// TODO(https://crbug.com/965802): Remove `WARN_NOT_OFFLINE_CAPABLE` once the
// CheckOfflineCapability feature is enabled with 'enforce' mode by default in
// M93.
if (error != NO_ERROR_DETECTED && error != WARN_NOT_OFFLINE_CAPABLE) {
auto error = data.NoBlockingErrors() ? NO_ERROR_DETECTED : data.errors[0];
if (error != NO_ERROR_DETECTED) {
if (error == NO_MATCHING_SERVICE_WORKER)
TrackDisplayEvent(DISPLAY_EVENT_LACKS_SERVICE_WORKER);
......
......@@ -193,7 +193,7 @@ void AppBannerManagerAndroid::PerformInstallableWebAppCheck() {
void AppBannerManagerAndroid::OnDidPerformInstallableWebAppCheck(
const InstallableData& data) {
if (data.errors.empty())
if (data.NoBlockingErrors())
WebApkUkmRecorder::RecordWebApkableVisit(data.manifest_url);
screenshots_ = data.screenshots;
......
......@@ -83,13 +83,13 @@ void TestAppBannerManagerDesktop::OnDidGetManifest(
// AppBannerManagerDesktop does not call |OnDidPerformInstallableCheck| to
// complete the installability check in this case, instead it early exits
// with failure.
if (!result.errors.empty())
if (!result.NoBlockingErrors())
SetInstallable(false);
}
void TestAppBannerManagerDesktop::OnDidPerformInstallableWebAppCheck(
const InstallableData& result) {
AppBannerManagerDesktop::OnDidPerformInstallableWebAppCheck(result);
SetInstallable(result.errors.empty());
SetInstallable(result.NoBlockingErrors());
}
void TestAppBannerManagerDesktop::ResetCurrentPageData() {
......
......@@ -185,7 +185,7 @@ void WebAppDataRetriever::OnDidPerformInstallableCheck(
DCHECK(data.manifest);
DCHECK(data.manifest_url.is_valid() || data.manifest->IsEmpty());
const bool is_installable = data.errors.empty();
const bool is_installable = data.NoBlockingErrors();
DCHECK(!is_installable || data.valid_manifest);
base::Optional<blink::Manifest> opt_manifest;
if (!data.manifest->IsEmpty())
......
......@@ -127,7 +127,7 @@ void ManifestUpdateTask::OnDidGetInstallableData(
const webapps::InstallableData& data) {
DCHECK_EQ(stage_, Stage::kPendingInstallableData);
if (!data.errors.empty()) {
if (!data.NoBlockingErrors()) {
DestroySelf(ManifestUpdateResult::kAppNotEligible);
return;
}
......
......@@ -33,4 +33,9 @@ InstallableData::InstallableData(std::vector<InstallableStatusCode> errors,
InstallableData::~InstallableData() = default;
bool InstallableData::NoBlockingErrors() const {
return errors.empty() ||
(errors.size() == 1 && errors[0] == WARN_NOT_OFFLINE_CAPABLE);
}
} // namespace webapps
......@@ -35,6 +35,14 @@ struct InstallableData {
bool has_worker);
~InstallableData();
// Returns true if `errors` is empty or only has `WARN_NOT_OFFLINE_CAPABLE`.
// `WARN_NOT_OFFLINE_CAPABLE` only logs a warning message in DevTools and
// should not change the behavior.
// TODO(https://crbug.com/965802): Remove `WARN_NOT_OFFLINE_CAPABLE` once the
// CheckOfflineCapability feature is enabled with 'enforce' mode by default in
// M93.
bool NoBlockingErrors() const;
// Contains all errors encountered during the InstallableManager::GetData
// call. Empty if no errors were encountered.
std::vector<InstallableStatusCode> errors;
......
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