Chrome talks to Play to install WebAPKs.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
index 2b3a2c9fb66f47a8b1437241f5ee51515d29292a..33b02d6acc7945dd0a33e7032e587aef6591e0f2 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
@@ -10,15 +10,20 @@ import android.content.pm.PackageManager;
 import android.net.Uri;
 import android.os.Build;
 import android.os.Looper;
+import android.text.TextUtils;
 
 import org.chromium.base.ApplicationState;
 import org.chromium.base.ApplicationStatus;
+import org.chromium.base.Callback;
 import org.chromium.base.ContentUriUtils;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.annotations.CalledByNative;
+import org.chromium.chrome.browser.ChromeApplication;
+import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.banners.InstallerDelegate;
 import org.chromium.chrome.browser.util.IntentUtils;
+import org.chromium.components.variations.VariationsAssociatedData;
 
 import java.io.File;
 
@@ -30,6 +35,9 @@ import java.io.File;
 public class WebApkInstaller {
     private static final String TAG = "WebApkInstaller";
 
+    /** Flag to enable installing WebAPKs from Play Store. */

[pkotwicz, 2016/12/02 02:24:00]

"Play Store" -> "Google Play"

[Xi Han, 2016/12/02 20:45:36]

Done.

+    private static final String PLAY_INSTALL_WEBAPKS = "play_install_webapks";

[pkotwicz, 2016/12/02 02:24:00]

Nit: You can name the parameter "play_install". WebAPKs is redundant because
this is a parameter for the WebAPK feature

[Xi Han, 2016/12/02 20:45:36]

Done.

+
     /** The WebAPK's package name. */
     private String mWebApkPackageName;
 
@@ -42,11 +50,38 @@ public class WebApkInstaller {
     /** Indicates whether to install or update a WebAPK. */
     private boolean mIsInstall;
 
+    /** Talks to Play Store to install WebAPKs. */

[pkotwicz, 2016/12/02 02:24:00]

"Play Store" -> "Google Play"

Can you please put the non-final-static-members either before or after the
non-final-non-static-members?

[Xi Han, 2016/12/02 20:45:37]

Done.

+    private static WebApkInstallClientDelegate sWebApkInstallClientDelegate;
+
+    /**
+     * Indicates whether the static {@link sWebApkInstallClientDelegate} has been initialized. It
+     * prevents from initializing the client delegate multiple time when the delegate is null.
+     */
+    private static boolean sIsWebApkInstallClientDelegateInitialized;

[pkotwicz, 2016/12/02 02:24:00]

I think that you can remove this variable. I am not worried about running
canUseGooglePlayToInstallWebApk() each time that
ChromeApplication#createWebApkInstallClientDelegate() is called

[Xi Han, 2016/12/02 20:45:36]

Hmmm, it is still wired that we initialize a static variable multiple times.
Move the initialization to DeferredStartupHandler.

+
     /** Weak pointer to the native WebApkInstaller. */
     private long mNativePointer;
 
     private WebApkInstaller(long nativePtr) {
         mNativePointer = nativePtr;
+        if (!sIsWebApkInstallClientDelegateInitialized) {
+            sIsWebApkInstallClientDelegateInitialized = true;
+            ChromeApplication application =
+                    (ChromeApplication) ContextUtils.getApplicationContext();
+            sWebApkInstallClientDelegate = application.createWebApkInstallClientDelegate();
+        }
+    }
+
+    /** Gets the WebApkInstallClientDelegate. */
+    public static WebApkInstallClientDelegate getWebApkInstallClientDelegate() {
+        return sWebApkInstallClientDelegate;
+    }
+
+    /** Return whether installing WebAPKs from Play Store is enabled. */

[pkotwicz, 2016/12/02 02:24:00]

"from Play Store" -> "using Google Play"

WebAPKs are not listed in the Play Store :)

[Xi Han, 2016/12/02 20:45:36]

Done.

+    public static boolean enablePlayInstallWebApk() {

[pkotwicz, 2016/12/02 02:24:00]

How about: canUseGooglePlayToInstallWebApk()

Your function name makes it sound like this function "enables installing WebAPKs
from Google Play"

Should this function be part of ChromeWebApkHost?

[Xi Han, 2016/12/02 20:45:36]

Moved.

+        if (!ChromeWebApkHost.isEnabled()) return false;
+        return TextUtils.equals(VariationsAssociatedData.getVariationParamValue(
+                ChromeSwitches.ENABLE_WEBAPK, PLAY_INSTALL_WEBAPKS), "true");

[pkotwicz, 2016/12/02 02:24:00]

ChromeSwitches.ENABLE_WEBAPK -> ChromeFeatureList.WEBAPKS

[Xi Han, 2016/12/02 20:45:36]

Done.

     }
 
     @CalledByNative
@@ -55,8 +90,16 @@ public class WebApkInstaller {
     }
 
     @CalledByNative
+    private boolean hasWebApkInstallClientDelegate() {
+        return sWebApkInstallClientDelegate != null;
+    }
+
+    @CalledByNative
     private void destroy() {
-        ApplicationStatus.unregisterApplicationStateListener(mListener);
+        if (mListener != null) {
+            ApplicationStatus.unregisterApplicationStateListener(mListener);
+        }
+        mListener = null;
         mNativePointer = 0;
     }
 
@@ -86,6 +129,29 @@ public class WebApkInstaller {
     }

[pkotwicz, 2016/12/02 02:23:59]

Nit: Group all of the @CalledByNative functions together

[Xi Han, 2016/12/02 20:45:37]

Done.

 
     /**
+     * Install a WebAPK from Play Store and monitors the installation.

[pkotwicz, 2016/12/02 02:24:00]

Nit: monitors -> monitor

[Xi Han, 2016/12/02 20:45:37]

Should be Install->Installs.

+     * @param packageName The package name of the WebAPK to install.
+     * @param version The version of WebAPK to install.
+     * @param title The title of the WebAPK to display during installation.
+     * @param wamToken The token from WebAPK Minter Server.

[pkotwicz, 2016/12/02 02:24:00]

Nit: We use "WebAPK Server" elsewhere. Might as well keep on doing this. (We
only have one WebAPK server so we don't need to specify "which WebAPK server")

[Xi Han, 2016/12/02 20:45:36]

Done.

+     */
+    @CalledByNative
+    private void installWebApkFromPlayStoreAsyncAndMonitorInstallation(String packageName,
+            int version, String title, String wamToken) {
+        if (sWebApkInstallClientDelegate == null) return;
+
+        Callback<Boolean> callback = new Callback<Boolean>() {
+            @Override
+            public void onResult(Boolean success) {
+                if (mNativePointer != 0) {
+                    nativeOnInstallFinished(mNativePointer, success);
+                }
+            }
+        };
+        sWebApkInstallClientDelegate.installAsync(packageName, version, title, wamToken, callback);
+    }
+
+    /**
      * Send intent to Android to show prompt and install downloaded WebAPK.
      * @param filePath File to install.
      */
@@ -144,6 +210,16 @@ public class WebApkInstaller {
         return installDownloadedWebApk(filePath);
     }
 

[pkotwicz, 2016/12/02 02:24:00]

Please add a comment

[Xi Han, 2016/12/02 20:45:36]

Done.

+    @CalledByNative
+    private void updateAsyncFromPlayStore(String packageName, int version, String title,
+            String wamToken) {
+        if (sWebApkInstallClientDelegate == null) return;
+
+        // TODO(hanxi):crbug.com/634499. Adds a callback to show an infobar after the update
+        // succeeded.
+        sWebApkInstallClientDelegate.installAsync(packageName, version, title, wamToken, null);
+    }
+
     private ApplicationStatus.ApplicationStateListener createApplicationStateListener() {
         return new ApplicationStatus.ApplicationStateListener() {
             @Override