Implement server-suggested update check backoff

chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
index cc68e6613d0150ec72614748a25ad2855164633c..fd443658789f04fa44882656e04660e584ddf351 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
@@ -61,6 +61,20 @@ public class WebappDataStorage {
     // The number of times that updating a WebAPK in the background has been requested.
     static final String KEY_UPDATE_REQUESTED = "update_requested";
 
+    // Whether to check updates less frequently.
+    static final String KEY_RELAX_UPDATES = "relax_updates";
+
+    // Number of milliseconds between checks for whether the WebAPK's Web Manifest has changed.
+    public static final long UPDATE_INTERVAL = TimeUnit.DAYS.toMillis(3L);
+
+    // Number of milliseconds between checks of updates for a WebAPK that is expected to check
+    // updates less frequently. crbug.com/680128.
+    public static final long RELAXED_UPDATE_INTERVAL = TimeUnit.DAYS.toMillis(30L);
+
+    // Number of milliseconds to wait before re-requesting an updated WebAPK from the WebAPK
+    // server if the previous update attempt failed.
+    public static final long RETRY_UPDATE_DURATION = TimeUnit.HOURS.toMillis(12L);
+
     // Unset/invalid constants for last used times and URLs. 0 is used as the null last used time as
     // WebappRegistry assumes that this is always a valid timestamp.
     static final long LAST_USED_UNSET = 0;
@@ -305,6 +319,7 @@ public class WebappDataStorage {
         editor.remove(KEY_LAST_WEBAPK_UPDATE_REQUEST_COMPLETE_TIME);
         editor.remove(KEY_DID_LAST_WEBAPK_UPDATE_REQUEST_SUCCEED);
         editor.remove(KEY_UPDATE_REQUESTED);
+        editor.remove(KEY_RELAX_UPDATES);
         editor.apply();
     }
 
@@ -390,7 +405,7 @@ public class WebappDataStorage {
     }
 
     /**
-     * Updates the result of whether the last update request to WebAPK Server succeeded.
+     * Updates whether the last update request to WebAPK Server succeeded.
      */
     void updateDidLastWebApkUpdateRequestSucceed(boolean success) {
         mPreferences.edit()
@@ -426,6 +441,48 @@ public class WebappDataStorage {
         return mPreferences.getInt(KEY_UPDATE_REQUESTED, 0);
     }
 
+    /**
+     * Returns whether the previous WebAPK update attempt succeeded. Returns true if there has not
+     * been any update attempts.
+     */
+    boolean didPreviousUpdateSucceed() {

[pkotwicz, 2017/03/04 00:20:46]

Nit: Rename this to didPreviousWebApkUpdateSucceed(). The comment explicitly
mentions WebAPKs

[dominickn, 2017/03/06 02:28:36]

I think we can leave the WebAPK bit out of the method name. WebAPKs are the only
thing that can update right now, and if we have something in the future that
also updates we can differentiate then.

[Xi Han, 2017/03/06 22:14:20]

Acknowledged.

+        long lastUpdateCompletionTime = getLastWebApkUpdateRequestCompletionTime();
+        if (lastUpdateCompletionTime == WebappDataStorage.LAST_USED_INVALID
+                || lastUpdateCompletionTime == WebappDataStorage.LAST_USED_UNSET) {
+            return true;
+        }
+        return getDidLastWebApkUpdateRequestSucceed();
+    }
+
+    /**
+     * Sets whether we should check updates less frequently.

[pkotwicz, 2017/03/04 00:20:46]

"check for updates" here too

I am unsure whether you should have "WebApk" in the new method names e.g.
setRelaxedWebApkUpdates(). I will leave this up to Dominick to decide.

[dominickn, 2017/03/06 02:28:36]

See above comment - we can probably leave WebApk out of the method name since
it's the only thing using this code path.

[Xi Han, 2017/03/06 22:14:20]

Acknowledged.

+     */
+    void setRelaxedUpdates(boolean relaxUpdates) {
+        mPreferences.edit().putBoolean(KEY_RELAX_UPDATES, relaxUpdates).apply();
+    }
+
+    /**

[pkotwicz, 2017/03/04 00:20:46]

Nit:
- "check updates" -> "check for updates"
- I think you can put the comment on one line

[Xi Han, 2017/03/06 22:14:20]

Done.

+     * Returns whether we should check updates less frequently.
+     */
+    boolean shouldRelaxUpdates() {
+        return mPreferences.getBoolean(KEY_RELAX_UPDATES, false);
+    }
+
+    /**
+     * Returns whether we should check update.

[pkotwicz, 2017/03/04 00:20:47]

Nits:
- "check update." -> "check for update."
- You can put the comment on one line /** Returns whether we should check for
update. */

[Xi Han, 2017/03/06 22:14:20]

Done.

+     */
+    boolean shouldUpdate() {

[pkotwicz, 2017/03/04 00:20:46]

Nit: Maybe rename this function to shouldCheckForUpdate()?

[dominickn, 2017/03/06 02:28:36]

shouldCheckForUpdate SGTM

[Xi Han, 2017/03/06 22:14:20]

Done.

+        long checkUpdatesInterval =
+                shouldRelaxUpdates() ? RELAXED_UPDATE_INTERVAL : UPDATE_INTERVAL;
+        long now = sClock.currentTimeMillis();
+        long sinceLastCheckDurationMs = now - getLastCheckForWebManifestUpdateTime();
+        if (sinceLastCheckDurationMs >= checkUpdatesInterval) return true;
+
+        long sinceLastUpdateRequestDurationMs = now - getLastWebApkUpdateRequestCompletionTime();
+        return sinceLastUpdateRequestDurationMs >= WebappDataStorage.RETRY_UPDATE_DURATION
+                && !didPreviousUpdateSucceed();
+    }
+
     protected WebappDataStorage(String webappId) {
         mId = webappId;
         mPreferences = ContextUtils.getApplicationContext().getSharedPreferences(