[go: nahoru, domu]

[dPWA] Throttle attempts at fixing generated web app icons to 1 per day

This CL adds a daily throttle to attempts to fix generated web app
icons. Attempts to reschedule fixes within the throttle will have
the fix PostDelayedTask()'d to the next unthrottled time. This means
we don't rely on users restarting Chrome to try again at the next
opportunity.

Along with this throttle is a retry on failure mechanism that
automatically schedules the next attempt once the throttle expires.
A hard coded attempt limit prevents retries from happening
indefinitely.

Bug: 1216965, b/300341813
Change-Id: Ib2cb36c446fd65f24c05e0742c7e891509fdfe2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4899656
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1204516}
diff --git a/chrome/browser/web_applications/generated_icon_fix_util.cc b/chrome/browser/web_applications/generated_icon_fix_util.cc
index b1c220c..3826c80 100644
--- a/chrome/browser/web_applications/generated_icon_fix_util.cc
+++ b/chrome/browser/web_applications/generated_icon_fix_util.cc
@@ -23,6 +23,16 @@
 namespace {
 
 constexpr base::TimeDelta kFixWindowDuration = base::Days(7);
+constexpr base::TimeDelta kFixAttemptThrottleDuration = base::Days(1);
+
+// Capping the number of attempts should be redundant with the throttle + window
+// but, because retries on failure are self propagating, have an explicit
+// attempt count to be extra safe. Ordinarily a constraint like this would be
+// CHECK'd but because these attempts run at start up it wouldn't be good for
+// stability.
+constexpr uint32_t kMaxAttemptCount =
+    kFixWindowDuration.IntDiv(kFixAttemptThrottleDuration);
+static_assert(kMaxAttemptCount == 7u);
 
 absl::optional<base::Time> g_now_override_for_testing_;
 
@@ -63,6 +73,15 @@
   g_now_override_for_testing_ = now;
 }
 
+bool HasRemainingAttempts(const WebApp& app) {
+  const absl::optional<GeneratedIconFix>& generated_icon_fix =
+      app.generated_icon_fix();
+  if (!generated_icon_fix.has_value()) {
+    return true;
+  }
+  return generated_icon_fix->attempt_count() < kMaxAttemptCount;
+}
+
 bool IsWithinFixTimeWindow(const WebApp& app) {
   const absl::optional<GeneratedIconFix>& generated_icon_fix =
       app.generated_icon_fix();
@@ -98,6 +117,23 @@
   return generated_icon_fix;
 }
 
+base::TimeDelta GetThrottleDuration(const WebApp& app) {
+  const absl::optional<GeneratedIconFix> generated_icon_fix =
+      app.generated_icon_fix();
+  if (!generated_icon_fix.has_value() ||
+      !generated_icon_fix->has_last_attempt_time()) {
+    return base::TimeDelta();
+  }
+
+  base::TimeDelta throttle_duration =
+      syncer::ProtoTimeToTime(generated_icon_fix->last_attempt_time()) +
+      kFixAttemptThrottleDuration - Now();
+  // Negative durations could cause us to skip ahead of other tasks already in
+  // the task queue when used in PostDelayedTask() so clamp to 0.
+  return throttle_duration.is_negative() ? base::TimeDelta()
+                                         : throttle_duration;
+}
+
 void RecordFixAttempt(WithAppResources& resources,
                       ScopedRegistryUpdate& update,
                       const webapps::AppId& app_id,