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

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

 // Copyright 2015 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;
 
 import android.annotation.SuppressLint;
 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;
 import android.graphics.Bitmap;
 import android.graphics.Color;
 import android.graphics.Rect;
 import android.graphics.drawable.ColorDrawable;
 import android.graphics.drawable.Drawable;
 import android.net.Uri;
 import android.os.Build;
@@ skipping 15 matching lines @@
 import android.view.accessibility.AccessibilityManager;
 import android.view.accessibility.AccessibilityManager.AccessibilityStateChangeListener;
 import android.view.accessibility.AccessibilityManager.TouchExplorationStateChangeListener;
 
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApiCompatibilityUtils;
 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;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.IntentHandler.IntentHandlerDelegate;
 import org.chromium.chrome.browser.IntentHandler.TabOpenType;
 import org.chromium.chrome.browser.appmenu.AppMenu;
 import org.chromium.chrome.browser.appmenu.AppMenuHandler;
@@ skipping 23 matching lines @@
 import org.chromium.chrome.browser.gsa.GSAAccountChangeListener;
 import org.chromium.chrome.browser.gsa.GSAState;
 import org.chromium.chrome.browser.help.HelpAndFeedback;
 import org.chromium.chrome.browser.history.HistoryManagerUtils;
 import org.chromium.chrome.browser.infobar.InfoBarContainer;
 import org.chromium.chrome.browser.init.AsyncInitializationActivity;
 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;
 import org.chromium.chrome.browser.nfc.BeamProvider;
 import org.chromium.chrome.browser.ntp.NewTabPage;
 import org.chromium.chrome.browser.ntp.NewTabPageUma;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
 import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
 import org.chromium.chrome.browser.pageinfo.WebsiteSettingsPopup;
@@ skipping 42 matching lines @@
 import org.chromium.content_public.browser.readback_types.ReadbackResponse;
 import org.chromium.policy.CombinedPolicyProvider;
 import org.chromium.policy.CombinedPolicyProvider.PolicyChangeListener;
 import org.chromium.printing.PrintManagerDelegateImpl;
 import org.chromium.printing.PrintingController;
 import org.chromium.printing.PrintingControllerImpl;
 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;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import javax.annotation.Nullable;
 
 /**
@@ skipping 1607 matching lines @@
                     && PrefServiceBridge.getInstance().isPrintingEnabled()) {
                 printingController.startPrint(new TabPrinter(currentTab),
                         new PrintManagerDelegateImpl(this));
                 RecordUserAction.record("MobileMenuPrint");
             }
         } else if (id == R.id.add_to_homescreen_id) {
             AddToHomescreenManager addToHomescreenManager =
                     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();
             currentTab.setUseDesktopUserAgent(!usingDesktopUserAgent, reloadOnChange);
             RecordUserAction.record("MobileMenuRequestDesktopSite");
         } else if (id == R.id.reader_mode_prefs_id) {
             if (currentTab.getWebContents() != null) {
                 RecordUserAction.record("DomDistiller_DistilledPagePrefsOpened");
                 AlertDialog.Builder builder =
                         new AlertDialog.Builder(this, R.style.AlertDialogTheme);
@@ skipping 231 matching lines @@
                 "Android.MultiWindowMode.IsTabletScreenWidthBelow600",
                 mScreenWidthDp < DeviceFormFactor.MINIMUM_TABLET_WIDTH_DP);
 
         if (mScreenWidthDp < DeviceFormFactor.MINIMUM_TABLET_WIDTH_DP) {
             RecordHistogram.recordLinearCountHistogram(
                     "Android.MultiWindowMode.TabletScreenWidth", mScreenWidthDp, 1,
                     DeviceFormFactor.MINIMUM_TABLET_WIDTH_DP, 50);
         }
     }
 }