[Android] Update CCT/Herb menu item

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabAppMenuPropertiesDelegate.java

Index: chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabAppMenuPropertiesDelegate.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabAppMenuPropertiesDelegate.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabAppMenuPropertiesDelegate.java
index a894354e732826c9dc32d136ef5ef693e4db5dee..3cb6683312cfcf93ef97833ba07d09dcdab1ccc8 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabAppMenuPropertiesDelegate.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabAppMenuPropertiesDelegate.java
@@ -16,6 +16,7 @@ import org.chromium.base.BuildInfo;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.UrlConstants;
 import org.chromium.chrome.browser.appmenu.AppMenuPropertiesDelegate;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
 import org.chromium.chrome.browser.share.ShareHelper;
@@ -34,13 +35,44 @@ public class CustomTabAppMenuPropertiesDelegate extends AppMenuPropertiesDelegat
 
     private final boolean mShowShare;
     private final boolean mIsMediaViewer;
+    private final boolean mIsOpenedByChrome;
 
     private final List<String> mMenuEntries;
     private final Map<MenuItem, Integer> mItemToIndexMap = new HashMap<MenuItem, Integer>();
-    private final AsyncTask<Void, Void, String> mDefaultBrowserFetcher;
 
     private boolean mIsCustomEntryAdded;
 
+    private class DefaultBrowserFetcher extends AsyncTask<String, Void, String> {

[Ted C, 2016/12/28 00:53:47]

Hmm...I'm pretty sure you don't want this string logic for the bookmark
snackbar.

For bookmarks, we don't care if the default browser is not set (or if it is
different from the current Chrome), we always want to show
BuildInfo.getPackageLabel in that case.  We are educating the users where there
bookmark went, so we need to say the current version of Chrome always.

[ltian, 2016/12/29 16:31:56]

Done.

+        @Override
+        protected String doInBackground(String... params) {
+            String packageLabel = null;
+            if (mIsOpenedByChrome) {
+                // If the Custom Tab was created by Chrome, Chrome should open it.
+                packageLabel = BuildInfo.getPackageLabel(mActivity);
+            } else {
+                // Check if there is a default handler for the Intent.  If so, grab its label.
+                Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(SAMPLE_URL));
+                PackageManager pm = mActivity.getPackageManager();
+                ResolveInfo info = pm.resolveActivity(intent, 0);
+                if (info != null && info.match != 0) {
+                    packageLabel = info.loadLabel(pm).toString();
+                }
+            }
+
+            if (params != null && params.length > 0 && params[0].equals("menu")) {
+                return packageLabel == null
+                        ? mActivity.getString(R.string.menu_open_in_product_default)
+                        : mActivity.getString(R.string.menu_open_in_product, packageLabel);
+            } else if (params != null && params.length > 0 && params[0].equals("bookmark")) {
+                return packageLabel == null
+                        ? null
+                        : mActivity.getString(R.string.bookmark_page_saved, packageLabel);
+            } else {
+                return null;
+            }
+        }
+    }
+
     /**
      * Creates an {@link CustomTabAppMenuPropertiesDelegate} instance.
      */
@@ -51,29 +83,7 @@ public class CustomTabAppMenuPropertiesDelegate extends AppMenuPropertiesDelegat
         mMenuEntries = menuEntries;
         mShowShare = showShare;
         mIsMediaViewer = isMediaViewer;
-
-        mDefaultBrowserFetcher = new AsyncTask<Void, Void, String>() {
-            @Override
-            protected String doInBackground(Void... params) {
-                String packageLabel = null;
-                if (isOpenedByChrome) {
-                    // If the Custom Tab was created by Chrome, Chrome should open it.
-                    packageLabel = BuildInfo.getPackageLabel(activity);
-                } else {
-                    // Check if there is a default handler for the Intent.  If so, grab its label.
-                    Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(SAMPLE_URL));
-                    PackageManager pm = activity.getPackageManager();
-                    ResolveInfo info = pm.resolveActivity(intent, 0);
-                    if (info != null && info.match != 0) {
-                        packageLabel = info.loadLabel(pm).toString();
-                    }
-                }
-
-                return packageLabel == null
-                        ? activity.getString(R.string.menu_open_in_product_default)
-                        : activity.getString(R.string.menu_open_in_product, packageLabel);
-            }
-        }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);

[Ted C, 2016/12/28 00:53:47]

FWIW, by removing this, you are essentially blocking on the UI for all of these
calls.  This was done this way before to hopefully have it ready by the time the
menu code needed it.  Regardless, I don't think you'll need any of this change
based on my other comment in this file.

[ltian, 2016/12/29 16:31:56]

Done.

+        mIsOpenedByChrome = isOpenedByChrome;
     }
 
     @Override
@@ -103,11 +113,16 @@ public class CustomTabAppMenuPropertiesDelegate extends AppMenuPropertiesDelegat
                 openInChromeItem.setVisible(false);
             } else {
                 try {
-                    openInChromeItem.setTitle(mDefaultBrowserFetcher.get());
+                    openInChromeItem.setTitle(
+                            new DefaultBrowserFetcher()
+                                    .executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, "menu")
+                                    .get());
                 } catch (InterruptedException | ExecutionException e) {
                     openInChromeItem.setTitle(
                             mActivity.getString(R.string.menu_open_in_product_default));
                 }
+                MenuItem bookmarkMenuItem = menu.findItem(R.id.bookmark_this_page_id);
+                updateBookmarkMenuItem(bookmarkMenuItem, currentTab);
             }
 
             if (!FirstRunStatus.getFirstRunFlowComplete()) {

[Ted C, 2016/12/28 00:53:47]

if first run hasn't been completed, can you make it so that bookmarks and
downloads are turned off in the menu.

If you clear your settings and start first run again, the privacy and TOS links
will now open in a CCT.

[ltian, 2016/12/29 16:31:56]

Done.

@@ -122,6 +137,18 @@ public class CustomTabAppMenuPropertiesDelegate extends AppMenuPropertiesDelegat
                     mItemToIndexMap.put(item, i);
                 }
             }
+
+            // 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);
+            String url = currentTab.getUrl();
+            boolean isChromeScheme = url.startsWith(UrlConstants.CHROME_SCHEME)
+                    || url.startsWith(UrlConstants.CHROME_NATIVE_SCHEME);
+            requestItem.setVisible(!isChromeScheme || currentTab.isNativePage());
+            requestItem.setChecked(currentTab.getUseDesktopUserAgent());
+            requestItem.setTitleCondensed(requestItem.isChecked()
+                            ? mActivity.getString(R.string.menu_request_desktop_site_on)

[Ted C, 2016/12/28 00:53:47]

indented too much.  should be 8 from the start of the previous line

[ltian, 2016/12/29 16:31:56]

Done.

+                            : mActivity.getString(R.string.menu_request_desktop_site_off));
         }
     }
 
@@ -142,6 +169,19 @@ public class CustomTabAppMenuPropertiesDelegate extends AppMenuPropertiesDelegat
     }
 
     /**
+     * @return The app-based text message displayed in the snackbar when bookmark a page.
+     */
+    public String getSnackbarTextForBookmark() {
+        try {
+            return new DefaultBrowserFetcher()
+                    .executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, "bookmark")
+                    .get();
+        } catch (InterruptedException | ExecutionException e) {
+            return null;
+        }
+    }
+
+    /**
      * Get the {@link MenuItem} object associated with the given title. If multiple menu items have
      * the same title, a random one will be returned. This method is for testing purpose _only_.
      */