[Android]: Hide add-to-homescreen app menu item when WebAPK is installed

chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
index a186f8746a97f51605c1c50b95f4a9a92d1cab05..7d6a73a2f306900a3dadd91f71a792ced68ec149 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
@@ -9,6 +9,7 @@ import android.annotation.TargetApi;
 import android.app.Activity;
 import android.app.SearchManager;
 import android.app.assist.AssistContent;
+import android.content.ActivityNotFoundException;
 import android.content.Context;
 import android.content.Intent;
 import android.content.res.Configuration;
@@ -44,6 +45,7 @@ import org.chromium.base.ApplicationStatus;
 import org.chromium.base.BaseSwitches;
 import org.chromium.base.Callback;
 import org.chromium.base.CommandLine;
+import org.chromium.base.ContextUtils;
 import org.chromium.base.SysUtils;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.VisibleForTesting;
@@ -87,6 +89,7 @@ import org.chromium.chrome.browser.metrics.LaunchMetrics;
 import org.chromium.chrome.browser.metrics.StartupMetrics;
 import org.chromium.chrome.browser.metrics.UmaSessionStats;
 import org.chromium.chrome.browser.metrics.UmaUtils;
+import org.chromium.chrome.browser.metrics.WebApkUma;
 import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
 import org.chromium.chrome.browser.net.spdyproxy.DataReductionProxySettings;
 import org.chromium.chrome.browser.nfc.BeamController;
@@ -149,6 +152,8 @@ import org.chromium.ui.base.ActivityWindowAndroid;
 import org.chromium.ui.base.DeviceFormFactor;
 import org.chromium.ui.base.PageTransition;
 import org.chromium.ui.base.WindowAndroid;
+import org.chromium.ui.widget.Toast;
+import org.chromium.webapk.lib.client.WebApkValidator;
 
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -1776,6 +1781,28 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
                     new AddToHomescreenManager(this, currentTab);
             addToHomescreenManager.start();
             RecordUserAction.record("MobileMenuAddToHomescreen");
+        } else if (id == R.id.open_webapk_id) {
+            Context context = ContextUtils.getApplicationContext();

[nyquist, 2017/02/28 07:04:19]

Is there a reason why you don't use |this| as the context? If something you call
needs the application context instead of an activity context, shouldn't they be
the ones to get the application context from the activity?

[gonzalon, 2017/02/28 16:12:23]

Not sure why I used the application context, we can definitely use |this|.

+            String packageName = WebApkValidator.queryWebApkPackage(context, currentTab.getUrl());

[nyquist, 2017/02/28 07:04:19]

This ended up being a very long section of the menu. Could you either extract
this into its own method?
It seems like the only thing it really needs is the URL from the current tab,
which you could pass in, i.e. it could even be a private static method if you
use the app context, or just an instance method if you end up using |this|.

I guess you could even keep the Toast here if you want, doing something like:

boolean launchFailed = openWebApkFromUrl(currentTab.getUrl());
if (launchFailed) {
  Toast.makeText(...);
}

That would at least make this part of the method so much easier to read.

WDYT?

[gonzalon, 2017/02/28 16:12:23]

Sounds great, it did get quite big. Only thing I would slightly change is for
the openWebApkFromUrl method to return true if it was opened successfully rather
than if it failed, since it seems to make more semantic sense. Are you all right
with that?

[nyquist, 2017/02/28 17:57:17]

Yeah, that's totally fine.

+            Intent launchIntent =
+                    context.getPackageManager().getLaunchIntentForPackage(packageName);
+            boolean launchFailed = false;
+            if (launchIntent != null) {
+                try {
+                    context.startActivity(launchIntent);
+                    RecordUserAction.record("MobileMenuOpenWebApk");
+                    WebApkUma.recordWebApkOpenAttempt(WebApkUma.WEBAPK_OPEN_LAUNCH_SUCCESS);
+                } catch (ActivityNotFoundException e) {
+                    WebApkUma.recordWebApkOpenAttempt(WebApkUma.WEBAPK_OPEN_ACTIVITY_NOT_FOUND);
+                    launchFailed = true;
+                }
+            } else {
+                WebApkUma.recordWebApkOpenAttempt(WebApkUma.WEBAPK_OPEN_NO_LAUNCH_INTENT);
+                launchFailed = true;
+            }
+            if (launchFailed) {
+                Toast.makeText(context, R.string.open_webapk_failed, Toast.LENGTH_SHORT).show();
+            }
         } else if (id == R.id.request_desktop_site_id) {
             final boolean reloadOnChange = !currentTab.isNativePage();
             final boolean usingDesktopUserAgent = currentTab.getUseDesktopUserAgent();