Add calls to the server to request WebAPK updates.

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
new file mode 100644
index 0000000000000000000000000000000000000000..0014cd434d2b1fb52a0a86578a794f95429d5b1a
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
@@ -0,0 +1,113 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.webapps;
+
+import android.content.pm.PackageInfo;
+import android.content.pm.PackageManager;
+import android.graphics.Bitmap;
+
+import org.chromium.base.ContextUtils;
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.chrome.browser.webapps.WebappRegistry.FetchWebappDataStorageCallback;
+import org.chromium.webapk.lib.common.WebApkConstants;
+
+import java.util.concurrent.TimeUnit;
+
+/**
+ * WebApkUpdateManager manages when to check the Web Manifest updates of the WebAPK, and
+ * sends request to the WebAPK Minting Server when a WebAPK needs update.
+ */

[pkotwicz, 2016/08/09 14:35:42]

How about: "WebApkUpdateManager manages when to check for updates to the
WebAPK's Web Manifest, and sends update request to the WebAPK Server when an
update is needed."

In webapk_installer.cc I refer to the server as the "WebAPK server" not the
"WebAPK Minting Server". I suggest to keep on doing this for the sake of
consistency (and because I landed by code first 😉)

[Yaron, 2016/08/10 02:26:28]

+1

[Xi Han, 2016/08/11 19:01:13]

Acknowledged.

+public class WebApkUpdateManager {
+    /** Represents a period of 3 days in milliseconds */

[pkotwicz, 2016/08/09 14:35:42]

The comment does not provide any information which is not obvious from line 24.
How about: "Number of milliseconds between checks for whether the WebAPK's web
manifest has changed."

Maybe FULL_CHECK_UPDATE_INTERVAL is more accurate than
FULL_CHECK_UPDATE_DURATION (The "full update" does not take 3 days to finish)

[Xi Han, 2016/08/11 19:01:14]

Sounds good, updated.

+    static final long FULL_CHECK_UPDATE_DURATION = TimeUnit.DAYS.toMillis(3L);
+
+    /** Represents a period of 12 hours in milliseconds */

[pkotwicz, 2016/08/09 14:35:42]

The comment does not provide any information which is not obvious from line 27

How about: "Number of milliseconds to wait before re-requesting an updated
WebAPK from the WebAPK server if the previous update attempt failed."

[Xi Han, 2016/08/11 19:01:13]

Done.

+    static final long RETRY_UPDATE_DURATION = TimeUnit.HOURS.toMillis(12L);
+
+    /**
+     * Check Web Manifest updates if we haven't checked updates for at least
+     * {@link FULL_CHECK_UPDATE_DURATION}.
+     * If a request to update the WebAPK has been sent to the WebAPK Minting Server recently
+     * {@link RETRY_UPDATE_DURATION} but failed, resends the update request to the server.

[pkotwicz, 2016/08/09 14:35:42]

The specific times (FULL_CHECK_UPDATE_DURATION, RETRY_UPDATE_DURATION) are an
implementation detail

How about: "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."

[Xi Han, 2016/08/11 19:01:14]

Done.

+     * @param webApkId The ID of the WebAPK.
+     * @param detector The ManfiestUpgradeDetector owned by the WebAPK.

[pkotwicz, 2016/08/09 14:35:42]

Nit ManfiestUpgradeDetector -> ManifestUpgradeDetector

[Xi Han, 2016/08/11 19:01:14]

Done.

+     */
+    public static void checkUpdate(String webApkId, final ManifestUpgradeDetector detector) {

[pkotwicz, 2016/08/09 14:35:41]

Can you please add checks w.r.t to which thread things a run on. My
understanding is that this function is not run on the UI thread

[Yaron, 2016/08/10 02:26:28]

I find this method name overloading confusing. Also the method name should be
more of an action. Perhaps rename checkIfUpdateNeeded

[Yaron, 2016/08/10 02:26:28]

+1

[Xi Han, 2016/08/11 19:01:14]

Done.

+        WebappRegistry.getWebappDataStorage(ContextUtils.getApplicationContext(), webApkId,
+                new WebappRegistry.FetchWebappDataStorageCallback() {
+                    @Override
+                    public void onWebappDataStorageRetrieved(WebappDataStorage storage) {
+                        if (storage == null) {

[Yaron, 2016/08/10 02:26:28]

you dereference storage in the next function which will NPE. Is it really
possible to not find the metadata? Does that happen if you clear chrome's data
and then launch a webapk? Or would we have repopulated it somewhere?

[Xi Han, 2016/08/11 19:01:13]

We repopulate the storage during the launching process, but updating the
SharedPerference is always an Async task. I add the null check to prevent any
crash due to the storage is null.

+                            checkUpdate(storage, detector);
+                            return;
+                        }
+
+                        long sinceLastCheckDuration = System.currentTimeMillis()
+                                - storage.getLastCheckForWebManifestUpdateTime();
+                        long sinceLastRequestDuration = System.currentTimeMillis()
+                                - storage.getLastRequestForNewShellApkVersionTime();
+                        if (sinceLastCheckDuration > FULL_CHECK_UPDATE_DURATION
+                                || (sinceLastRequestDuration > RETRY_UPDATE_DURATION
+                                && !storage.getDoesLastRequestForNewShellApkVersionSucceed())) {
+                            checkUpdate(storage, detector);
+                        }
+                    }
+                });
+    }
+
+    private static void checkUpdate(WebappDataStorage storage, ManifestUpgradeDetector detector) {

[pkotwicz, 2016/08/09 14:35:42]

My understanding is that this function is run on the UI thread

[Yaron, 2016/08/10 02:26:28]

yes. also please rename updateCheckFinished

[Xi Han, 2016/08/11 19:01:14]

Add the assert. Rename it to startCheckUpdate, since we just start the detector
and the check hasn't finished yet.

+        storage.updateLastCheckForWebManifestUpdateTime();

[pkotwicz, 2016/08/09 14:35:42]

We call WebappDataStorage#updateLastCheckForWebManifestUpdateTime() when we
start the UpgradeManifestDetectorFetcher not when the
UpdateManifestDetectorFetcher finds the Web Manifest.

I am concerned that some WebAPK-able sites link to the Web Manifest only from
the start_url. If a user usually launches a WebAPK via a deep link, it is
unlikely that we will do the upgrade check when the user launches the WebAPK via
the app list shortcut. This might cause the WebAPK never to be updated.

I think that it is worth filing a bug with this concern. We should investigate
whether sites link to the Web Manifest from *all* of the pages within the Web
Manifest scope. The washingtonpost.com/pwa does do so. I do not know of any
other PWAs

[Xi Han, 2016/08/11 19:01:14]

Your concern is right. Filed a bug 636525.

+        detector.start();
+    }
+
+    /**
+     * Sends request to WebAPK Minting Server to update WebAPK.

[pkotwicz, 2016/08/09 14:35:42]

"WebAPK Minting Server" -> "WebAPK server"

[Xi Han, 2016/08/11 19:01:14]

Done.

+     * @param webappInfo The new fetched Web Manifest data of the WebAPK.
+     */
+    public static void updateAsync(WebappInfo webappInfo) {
+        int webApkVersion = getVersionFromMetaData(webappInfo.webApkPackageName());
+        nativeUpdateAsync(webappInfo.uri().toString(), webappInfo.scopeUri().toString(),
+                webappInfo.name(), webappInfo.shortName(), "", webappInfo.icon(),
+                webappInfo.displayMode(), webappInfo.orientation(), webappInfo.themeColor(),
+                webappInfo.backgroundColor(), webappInfo.webManifestUri().toString(),
+                webappInfo.webApkPackageName(), webApkVersion);
+    }
+
+    private static int getVersionFromMetaData(String webApkPackage) {
+        try {
+            PackageManager packageManager =
+                    ContextUtils.getApplicationContext().getPackageManager();
+            PackageInfo info = packageManager.getPackageInfo(webApkPackage, 0);
+            return info.versionCode;
+        } catch (PackageManager.NameNotFoundException e) {
+            e.printStackTrace();
+        }
+        return 1;
+    }
+
+    /**
+     * Called after the either a request to update the WebAPK has been sent or the creation
+     * process of a new WebAPK fails.
+     */

[pkotwicz, 2016/08/09 14:35:42]

Nit: "the creation process of a new WebAPK fails." -> "update process fails."

[Xi Han, 2016/08/11 19:01:14]

Done.

+    @CalledByNative
+    public static void onBuiltWebApk(final boolean success, String webapkPackage) {
+        WebappRegistry.getWebappDataStorage(ContextUtils.getApplicationContext(),
+                WebApkConstants.WEBAPK_ID_PREFIX + webapkPackage,
+                new FetchWebappDataStorageCallback() {
+                    @Override
+                    public void onWebappDataStorageRetrieved(WebappDataStorage storage) {
+                        // Update the request time and result together. It prevents getting a
+                        // correct request time but a result from the previous request.
+                        storage.updateLastRequestForNewShellApkVersionTime();
+                        storage.updateDoesLastRequestForNewShellApkVersionSucceed(success);
+                    }
+                });
+    }
+
+    private static native void nativeUpdateAsync(String url, String scope, String name,

[pkotwicz, 2016/08/09 14:35:41]

Nit: |url| -> |startUrl|

[Xi Han, 2016/08/11 19:01:14]

Done.

+            String shortName, String iconUrl, Bitmap icon, int displayMode, int orientation,
+            long themeColor, long backgroundColor, String manifestUrl, String webApkPackage,
+            int webApkVersion);
+}