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

chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuPropertiesDelegate.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.appmenu;
 
+import android.content.Context;
+import android.content.pm.ResolveInfo;
 import android.graphics.PorterDuff;
 import android.graphics.drawable.Drawable;
 import android.view.Menu;
 import android.view.MenuItem;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.CommandLine;
+import org.chromium.base.ContextUtils;
+import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.UrlConstants;
 import org.chromium.chrome.browser.banners.AppBannerManager;
 import org.chromium.chrome.browser.bookmarks.BookmarkBridge;
 import org.chromium.chrome.browser.download.DownloadUtils;
 import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
 import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
 import org.chromium.chrome.browser.preferences.ManagedPreferencesUtils;
 import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.share.ShareHelper;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.components.dom_distiller.core.DomDistillerUrlUtils;
 import org.chromium.ui.base.DeviceFormFactor;
+import org.chromium.webapk.lib.client.WebApkValidator;
+
+import java.util.concurrent.TimeUnit;
 
 /**
  * App Menu helper that handles hiding and showing menu items based on activity state.
  */
 public class AppMenuPropertiesDelegate {

[dominickn, 2017/02/21 23:04:42]

You also need to change CustomTabAppMenuPropertiesDelegate.java, which is used
for CCT (and also has add to homescreen in the menu).

[gonzalon, 2017/02/22 15:50:49]

Done.

     // Indices for different levels in drawable.btn_reload_stop.
     // Used only when preparing menu and refresh reload button in menu when tab
     // page load status changes.
     private static final int RELOAD_BUTTON_LEVEL_RELOAD = 0;
     private static final int RELOAD_BUTTON_LEVEL_STOP_LOADING = 1;
 
     protected MenuItem mReloadMenuItem;
 
     protected final ChromeActivity mActivity;
 
@@ skipping 109 matching lines @@
             // Hide 'Add to homescreen' for the following:
             // * chrome:// pages - Android doesn't know how to direct those URLs.
             // * incognito pages - To avoid problems where users create shortcuts in incognito
             //                      mode and then open the webapp in regular mode.
             // * file:// - After API 24, file: URIs are not supported in VIEW intents and thus
             //             can not be added to the homescreen.
             // * content:// - Accessing external content URIs requires the calling app to grant
             //                access to the resource via FLAG_GRANT_READ_URI_PERMISSION, and that
             //                is not persisted when adding to the homescreen.
             // * If creating shortcuts it not supported by the current home screen.
+            long addToHomeScreenStart = System.currentTimeMillis();

[dominickn, 2017/02/21 23:04:42]

This timing is currently mixing in the work needed to compute the add to
homescreen menu item as well as the open WebAPK item. Can this be untangled?

Ideally you should have:

1. get current time
2. query webapk
3. record time histogram
4. work out whether to show add to homescreen or open WebAPK

Because what we really care about is how long step 2 takes.

[gonzalon, 2017/02/22 15:50:49]

Done.

             MenuItem homescreenItem = menu.findItem(R.id.add_to_homescreen_id);
-            boolean homescreenItemVisible = ShortcutHelper.isAddToHomeIntentSupported(mActivity)
+            MenuItem openWebApkItem = menu.findItem(R.id.open_webapk_id);

[pkotwicz, 2017/02/21 22:52:32]

Based on reading the discussion on the bug (and comment #5), I think that
agreement was reached w.r.t graying out the menu item if the WebAPK is already
installed

[dominickn, 2017/02/21 23:04:42]

I thought that we were going to display "Open appname" like this CL implements?

[pkotwicz, 2017/02/22 15:04:51]

Yes Dominick you are right. I somehow missed Comment #8

+            boolean canBeAddedToHomeScreen = ShortcutHelper.isAddToHomeIntentSupported(mActivity)
                     && !isChromeScheme && !isFileScheme && !isContentScheme && !isIncognito;
-            if (homescreenItemVisible) {
+            Context context = ContextUtils.getApplicationContext();
+            ResolveInfo resolveInfo = WebApkValidator.queryWebApk(context, currentTab.getUrl());
+            boolean openWebApkItemVisible = canBeAddedToHomeScreen && resolveInfo != null
+                    && resolveInfo.activityInfo.packageName != null;
+            if (canBeAddedToHomeScreen && !openWebApkItemVisible) {

[dominickn, 2017/02/21 23:04:42]

I think the logic here is more clearly expressed like this:

if (homescreenItemVisible) {
  if (openWebApkItemVisible) {
    openWebApkItem.setTitle()
    openWebApkItem.setVisible(true);
    homescreenItem.setVisible(false);
  } else {
    homescreenItem.setTitle()
    homescreenItem.setVisible(true);
    openWebApkItem.setVisible(false);
  }
}

That way we're not continually (and redundantly) computing booleans so much, and
it's exceedingly clear that only one or the other of homescreenItem and
openWebApkItem can be visible at once.

[gonzalon, 2017/02/22 15:50:49]

Done.

                 homescreenItem.setTitle(AppBannerManager.getHomescreenLanguageOption());
             }
-            homescreenItem.setVisible(homescreenItemVisible);
+            if (openWebApkItemVisible) {
+                String appName = resolveInfo.loadLabel(context.getPackageManager()).toString();
+                openWebApkItem.setTitle(context.getString(R.string.menu_open_webapk, appName));
+            }
+            homescreenItem.setVisible(canBeAddedToHomeScreen && !openWebApkItemVisible);
+            openWebApkItem.setVisible(openWebApkItemVisible);
+            RecordHistogram.recordTimesHistogram("AndroidPrepareMenu.OpenWebAPK",

[pkotwicz, 2017/02/21 22:52:32]

How about "Android.PrepareMenu.DisableAddToHomescreenCheck"

- I think that the OWNERS of tools/metrics/histograms/histograms.xml don't like
introducing new namespaces

You also need to add an entry to tools/metrics/histograms/histograms.xml 

[dominickn, 2017/02/21 23:04:42]

+1 to all Peter's comments

[gonzalon, 2017/02/22 15:50:49]

Done.

+                    System.currentTimeMillis() - addToHomeScreenStart, TimeUnit.MILLISECONDS);

[pkotwicz, 2017/02/21 22:52:32]

From looking at the codebase, it looks like we prefer
SystemClock.elapsedRealtime() 

[gonzalon, 2017/02/22 15:50:49]

Done.

 
             // Hide request desktop site on all chrome:// pages except for the NTP. Check request
             // desktop site if it's activated on this page.
             MenuItem requestItem = menu.findItem(R.id.request_desktop_site_id);
             updateRequestDesktopSiteMenuItem(requestItem, currentTab);
 
             // Only display reader mode settings menu option if the current page is in reader mode.
             menu.findItem(R.id.reader_mode_prefs_id)
                     .setVisible(DomDistillerUrlUtils.isDistilledPage(currentTab.getUrl()));
 
@@ skipping 119 matching lines @@
         String url = currentTab.getUrl();
         boolean isChromeScheme = url.startsWith(UrlConstants.CHROME_URL_PREFIX)
                 || url.startsWith(UrlConstants.CHROME_NATIVE_URL_PREFIX);
         requstMenuItem.setVisible(!isChromeScheme || currentTab.isNativePage());
         requstMenuItem.setChecked(currentTab.getUseDesktopUserAgent());
         requstMenuItem.setTitleCondensed(requstMenuItem.isChecked()
                 ? mActivity.getString(R.string.menu_request_desktop_site_on)
                 : mActivity.getString(R.string.menu_request_desktop_site_off));
     }
 }