Implement server-suggested update check backoff

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

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
index 68943599223d5846481232b1b6c1e9e9712ad2d2..b646a0b43caea089397eeb9d9e69977d731bc7bf 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
@@ -22,7 +22,6 @@ import org.chromium.chrome.browser.util.UrlUtilities;
 import org.chromium.webapk.lib.client.WebApkVersion;
 
 import java.util.Map;
-import java.util.concurrent.TimeUnit;
 
 /**
  * WebApkUpdateManager manages when to check for updates to the WebAPK's Web Manifest, and sends
@@ -31,15 +30,6 @@ import java.util.concurrent.TimeUnit;
 public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
     private static final String TAG = "WebApkUpdateManager";
 
-    /** Number of milliseconds between checks for whether the WebAPK's Web Manifest has changed. */
-    public static final long FULL_CHECK_UPDATE_INTERVAL = TimeUnit.DAYS.toMillis(3L);
-
-    /**
-     * 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);
-
     /**
      * Number of times to wait for updating the WebAPK after it is moved to the background prior
      * to doing the update while the WebAPK is in the foreground.
@@ -89,8 +79,8 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
     }
 
     /**
-     * Checks whether the WebAPK's Web Manifest has changed. Requests an updated WebAPK if the
-     * Web Manifest has changed. Skips the check if the check was done recently.
+     * Checks whether the WebAPK's Web Manifest has changed. Requests an updated WebAPK if the Web
+     * Manifest has changed. Skips the check if the check was done recently.
      * @param tab  The tab of the WebAPK.
      * @param info The WebApkInfo of the WebAPK.
      */
@@ -98,9 +88,9 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
         mInfo = info;
 
         WebappDataStorage storage = WebappRegistry.getInstance().getWebappDataStorage(mInfo.id());
-        mPreviousUpdateSucceeded = didPreviousUpdateSucceed(storage);
+        mPreviousUpdateSucceeded = storage.didPreviousUpdateSucceed();

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

crrev.com/2725813004 makes the WebappDataStorage a member of this class. I'll
let you decide which should land first, but when that change comes through, you
won't need to store mPreviousUpdateSucceeded any more (just call
mStorage.didPreviousUpdatedSucceed() in onGotManifestData()).

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

Thanks for letting me know. I will talk to Felix. For my CL, I will remove the
|mPreviousUpdateSucceeded| too.

 
-        if (!shouldCheckIfWebManifestUpdated(storage, mInfo, mPreviousUpdateSucceeded)) return;
+        if (!shouldCheckIfWebManifestUpdated(storage, mInfo)) return;
 
         mFetcher = buildFetcher();
         mFetcher.start(tab, mInfo, this);
@@ -165,14 +155,14 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
 
         if (!needsUpgrade) {
             if (!mPreviousUpdateSucceeded) {
-                recordUpdate(storage, true);
+                recordUpdate(storage, true, false);

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

Nit: can you annotate call to recordUpdate with an explanation of what the
variable are?

recordUpdate(storage, true, /* success */ false /* relaxUpdates */);

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

Done.

             }
             return;
         }
 
         // Set WebAPK update as having failed in case that Chrome is killed prior to
         // {@link onBuiltWebApk} being called.
-        recordUpdate(storage, false);
+        recordUpdate(storage, false, false);
 
         if (fetchedInfo != null) {
             scheduleUpdate(fetchedInfo, bestIconUrl, false /* isManifestStale */);
@@ -265,11 +255,6 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
         mFetcher = null;
     }
 
-    /** Returns the current time. In a separate function for the sake of testing. */
-    protected long currentTimeMillis() {
-        return System.currentTimeMillis();
-    }
-
     /**
      * Reads the WebAPK's version code. Returns 0 on failure.
      */
@@ -286,19 +271,6 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
     }
 
     /**
-     * Returns whether the previous WebAPK update attempt succeeded. Returns true if there has not
-     * been any update attempts.
-     */
-    private static boolean didPreviousUpdateSucceed(WebappDataStorage storage) {
-        long lastUpdateCompletionTime = storage.getLastWebApkUpdateRequestCompletionTime();
-        if (lastUpdateCompletionTime == WebappDataStorage.LAST_USED_INVALID
-                || lastUpdateCompletionTime == WebappDataStorage.LAST_USED_UNSET) {
-            return true;
-        }
-        return storage.getDidLastWebApkUpdateRequestSucceed();
-    }
-
-    /**
      * Whether there is a new version of the //chrome/android/webapk/shell_apk code.
      */
     private static boolean isShellApkVersionOutOfDate(WebApkInfo info) {
@@ -310,11 +282,9 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
      * TODO: Make this method static once there is a static global clock class.
      * @param storage WebappDataStorage with the WebAPK's cached data.
      * @param info Meta data from WebAPK's Android Manifest.
-     * @param previousUpdateSucceeded Whether the previous update attempt succeeded.
      * True if there has not been any update attempts.
      */
-    private boolean shouldCheckIfWebManifestUpdated(
-            WebappDataStorage storage, WebApkInfo info, boolean previousUpdateSucceeded) {
+    private boolean shouldCheckIfWebManifestUpdated(WebappDataStorage storage, WebApkInfo info) {
         if (!sUpdatesEnabled) {
             return false;
         }
@@ -328,25 +298,20 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
 
         if (isShellApkVersionOutOfDate(info)) return true;
 
-        long now = currentTimeMillis();
-        long sinceLastCheckDurationMs = now - storage.getLastCheckForWebManifestUpdateTime();
-        if (sinceLastCheckDurationMs >= FULL_CHECK_UPDATE_INTERVAL) return true;
-
-        long sinceLastUpdateRequestDurationMs =
-                now - storage.getLastWebApkUpdateRequestCompletionTime();
-        return sinceLastUpdateRequestDurationMs >= RETRY_UPDATE_DURATION
-                && !previousUpdateSucceeded;
+        return storage.shouldUpdate();
     }
 
     /**
      * Updates {@link WebappDataStorage} with the time of the latest WebAPK update and whether the
      * WebAPK update succeeded.
      */
-    private static void recordUpdate(WebappDataStorage storage, boolean success) {
+    private static void recordUpdate(
+            WebappDataStorage storage, boolean success, boolean relaxUpdates) {
         // Update the request time and result together. It prevents getting a correct request time
         // but a result from the previous request.
         storage.updateTimeOfLastWebApkUpdateRequestCompletion();
         storage.updateDidLastWebApkUpdateRequestSucceed(success);
+        storage.setRelaxedUpdates(relaxUpdates);
     }
 
     /**
@@ -403,9 +368,9 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer {
      * fails.
      */
     @CalledByNative
-    private static void onBuiltWebApk(String id, boolean success) {
+    private static void onBuiltWebApk(String id, boolean success, boolean relaxUpdates) {
         WebappDataStorage storage = WebappRegistry.getInstance().getWebappDataStorage(id);
-        recordUpdate(storage, success);
+        recordUpdate(storage, success, relaxUpdates);
     }
 
     private static native void nativeUpdateAsync(String id, String startUrl, String scope,