Allow installing apps via the AppBannerInfoBar

chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java b/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
index b8e42298a42a78db4305cf4273ede864350777cb..a792358b8a79a1676c475afd8dea0bf8b0b76062 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
@@ -4,6 +4,12 @@
 
 package org.chromium.chrome.browser.banners;
 
+import android.app.Activity;
+import android.content.ContentResolver;
+import android.content.Context;
+import android.content.Intent;
+import android.content.pm.PackageManager;
+import android.os.Looper;
 import android.text.TextUtils;
 
 import org.chromium.base.ApplicationStatus;
@@ -12,7 +18,10 @@ import org.chromium.base.JNINamespace;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.EmptyTabObserver;
 import org.chromium.chrome.browser.Tab;
+import org.chromium.chrome.browser.infobar.AppBannerInfoBar;
 import org.chromium.content_public.browser.WebContents;
+import org.chromium.ui.base.WindowAndroid;
+import org.chromium.ui.base.WindowAndroid.IntentCallback;
 
 /**
  * Manages an AppBannerInfoBar for a Tab.
@@ -26,7 +35,8 @@ import org.chromium.content_public.browser.WebContents;
  * from the network.
  */
 @JNINamespace("banners")
-public class AppBannerManager extends EmptyTabObserver implements AppDetailsDelegate.Observer {
+public class AppBannerManager extends EmptyTabObserver implements AppDetailsDelegate.Observer,

[newt (away), 2015/02/06 22:46:27]

This class implements so many different interfaces, it would be helpful to have
each interface implementation commented, e.g

    // IntentCallback implementation

Or you could use anonymous classes to reduce the number of interfaces that
AppBannerManager implements.

[gone, 2015/02/06 23:32:08]

Done.

+        IntentCallback, InstallerDelegate.Observer, ApplicationStatus.ApplicationStateListener {
     private static final String TAG = "AppBannerManager";
 
     /** Retrieves information about a given package. */
@@ -38,6 +48,9 @@ public class AppBannerManager extends EmptyTabObserver implements AppDetailsDele
     /** Tab that the AppBannerView/AppBannerManager is owned by. */
     private final Tab mTab;
 
+    /** Monitors an installation in progress. */
+    private InstallerDelegate mInstallTask;
+
     /**
      * Checks if app banners are enabled.
      * @return True if banners are enabled, false otherwise.
@@ -63,6 +76,7 @@ public class AppBannerManager extends EmptyTabObserver implements AppDetailsDele
         mNativePointer = nativeInit();
         mTab = tab;
         updatePointers();
+        ApplicationStatus.registerApplicationStateListener(this);

[newt (away), 2015/02/06 22:46:27]

Why not listen for window visibility changed events? You don't really care if
other Activities are visible in Chrome; you only care if the infobar is visible.

[gone, 2015/02/06 22:55:00]

InfoBarView isn't a legit View; I'd need to add it to InfoBarLayout.  Alright
with that?

[newt (away), 2015/02/06 23:15:47]

We could add it to one of the custom views in the AppBannerInfobar. Let's wait
until the UI is in place.

     }
 
     @Override
@@ -80,6 +94,11 @@ public class AppBannerManager extends EmptyTabObserver implements AppDetailsDele
      * Destroys the native AppBannerManager.
      */
     public void destroy() {
+        if (mInstallTask != null) {
+            mInstallTask.cancel();

[newt (away), 2015/02/06 22:46:27]

Should we cancel the installation if the user exits chrome while it's still in
progress? That would surprise me as a user. (Or am I misunderstanding what
mInstallTask.cancel() does?)

[gone, 2015/02/06 22:55:00]

This just watches the install progress; the Play Store is the one that's
actually installing things.

[newt (away), 2015/02/06 23:15:47]

Gotcha. Sounds good then.

+            mInstallTask = null;
+        }
+        ApplicationStatus.unregisterApplicationStateListener(this);
         nativeDestroy(mNativePointer);
     }
 
@@ -124,6 +143,69 @@ public class AppBannerManager extends EmptyTabObserver implements AppDetailsDele
                 mNativePointer, data, data.title(), data.packageName(), data.imageUrl());
     }
 
+    @CalledByNative
+    private boolean installOrOpenNativeApp(AppData appData) {
+        Context context = ApplicationStatus.getApplicationContext();
+        String packageName = appData.packageName();
+        PackageManager packageManager = context.getPackageManager();
+
+        if (InstallerDelegate.isInstalled(packageManager, packageName)) {
+            // Open the app.
+            Intent launchIntent = packageManager.getLaunchIntentForPackage(packageName);
+            if (launchIntent == null) return true;
+            context.startActivity(launchIntent);

[newt (away), 2015/02/06 22:46:27]

I seem to recall an advantage to using an Activity context when launching new
activities. Should we be using an activity context here?

[gone, 2015/02/06 22:55:01]

Doesn't seem to matter here; apps launch just fine.

+            return true;
+        } else {
+            // Try installing the app.  If the installation was kicked off, return false to prevent
+            // the infobar from disappearing.
+            return !mTab.getWindowAndroid().showIntent(
+                    appData.installIntent(), this, R.string.low_memory_error);

[newt (away), 2015/02/06 22:46:27]

Why "low memory"? 

[gone, 2015/02/06 22:55:00]

Carry over from the AppBannerView.  Most of the other calling interfaces also
(strangely) use this string for lack of a better one.

[newt (away), 2015/02/06 23:15:47]

We might as well say "Too many camels" here. This is a meaningless and
misleading error message.

[gone, 2015/02/06 23:32:08]

Debatably... the error message indicates that Chrome was killed before the user
could make it back.  What message would you suggest?

[gone, 2015/02/07 00:20:42]

Working with Dave to get rid of the error message requirement.  We don't
actually care if Chrome dies since the app will already be installing (or not)
by the time it gets back.  If Chrome dies, the infobar's gone anyway.  Are you
alright with landing while I land that other patch?

+        }
+    }
+
+    @CalledByNative
+    private void showAppDetails(AppData appData) {
+        mTab.getWindowAndroid().showIntent(
+                appData.detailsIntent(), this, R.string.low_memory_error);
+    }
+
+    @Override
+    public void onIntentCompleted(WindowAndroid window, int resultCode,

[newt (away), 2015/02/06 22:46:27]

This will be called as a result of both showing app details and installing the
app. Why are both cases handled in the same way?

[gone, 2015/02/06 22:55:00]

This is only called as a result of the Play Store Intent returning after showing
app details.

[gone, 2015/02/06 23:05:45]

Ah, got it.  It's handled the same way because the Java class doesn't really
know anything about the package or Intent being fired.  It's just acting on
behalf of the native code, which houses the logic for what to do after an Intent
returns.

[gone, 2015/02/06 23:08:05]

Redacting that again.  Showing the app details should just hide the infobar
(which it does).  Shouldn't need to have a callback in that case...

[newt (away), 2015/02/06 23:15:47]

I'm not crazy after all :)

+            ContentResolver contentResolver, Intent data) {
+        nativeOnIntentCompleted(mNativePointer, resultCode == Activity.RESULT_OK);
+    }
+
+    @CalledByNative
+    private void monitorInstall(AppData appData) {
+        PackageManager pm = ApplicationStatus.getApplicationContext().getPackageManager();
+        mInstallTask =
+                new InstallerDelegate(Looper.getMainLooper(), pm, this, appData.packageName());
+        mInstallTask.start();
+    }
+
+    @Override
+    public void onInstallFinished(InstallerDelegate task, boolean success) {
+        if (mInstallTask != task) return;
+        mInstallTask = null;
+        nativeOnInstallFinished(mNativePointer, success);
+    }
+
+    @Override
+    public void onApplicationStateChange(int newState) {
+        if (!ApplicationStatus.hasVisibleActivities()) return;
+        nativeUpdateInstallState(mNativePointer);
+    }
+
+    @CalledByNative
+    private int determineInstallState(AppData appData) {
+        if (mInstallTask != null) return AppBannerInfoBar.INSTALL_STATE_INSTALLING;
+
+        PackageManager pm = ApplicationStatus.getApplicationContext().getPackageManager();
+        boolean isInstalled = InstallerDelegate.isInstalled(pm, appData.packageName());
+        return isInstalled ? AppBannerInfoBar.INSTALL_STATE_INSTALLED
+                : AppBannerInfoBar.INSTALL_STATE_NOT_INSTALLED;
+    }
+
     private static native boolean nativeIsEnabled();
     private native long nativeInit();
     private native void nativeDestroy(long nativeAppBannerManager);
@@ -131,6 +213,9 @@ public class AppBannerManager extends EmptyTabObserver implements AppDetailsDele
             WebContents webContents);
     private native boolean nativeOnAppDetailsRetrieved(long nativeAppBannerManager, AppData data,
             String title, String packageName, String imageUrl);
+    private native void nativeOnIntentCompleted(long nativeAppBannerManager, boolean isInstalling);
+    private native void nativeOnInstallFinished(long nativeAppBannerManager, boolean success);
+    private native void nativeUpdateInstallState(long nativeAppBannerManager);
 
     // UMA tracking.
     private static native void nativeRecordDismissEvent(int metric);