Refactor ChromeContextMenuPopulator population

chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java

 // Copyright 2013 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.contextmenu;
 
 import android.content.Context;
 import android.net.MailTo;
 import android.support.annotation.IntDef;
 import android.text.TextUtils;
 import android.view.ContextMenu;
 import android.view.MenuInflater;
 import android.view.MenuItem;
 import android.webkit.MimeTypeMap;
 
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.UrlConstants;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
 import org.chromium.chrome.browser.net.spdyproxy.DataReductionProxySettings;
 import org.chromium.chrome.browser.preferences.datareduction.DataReductionProxyUma;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService;
 import org.chromium.chrome.browser.util.UrlUtilities;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
 
 /**
  * A {@link ContextMenuPopulator} used for showing the default Chrome context menu.
  */
 public class ChromeContextMenuPopulator implements ContextMenuPopulator {
 
     private static final String TAG = "CCMenuPopulator";
 
     /**
      * Defines the context menu modes
@@ skipping 157 matching lines @@
         }
 
         if (mMenuInflater == null) mMenuInflater = new MenuInflater(context);
 
         mMenuInflater.inflate(R.menu.chrome_context_menu, menu);
 
         menu.setGroupVisible(R.id.contextmenu_group_anchor, params.isAnchor());
         menu.setGroupVisible(R.id.contextmenu_group_image, params.isImage());
         menu.setGroupVisible(R.id.contextmenu_group_video, params.isVideo());
 
+        menu.setGroupVisible(R.id.contextmenu_group_message, MailTo.isMailTo(params.getLinkUrl())
+                        || UrlUtilities.isTelScheme(params.getLinkUrl()));

[Ted C, 2017/02/28 06:10:13]

this looks to be indented too much...is this what git cl format/clang-format
tried to do?

Should be 8 from the start of the line

[JJ, 2017/03/01 01:41:04]

Yup! Sorry about that, it was yelling at me each time I tried to submit and this
must've yelled at me.

+
+        Set<Integer> menuIdSet = new HashSet<>(menu.size());
+        for (int i = 0; i < menu.size(); i++) {
+            MenuItem item = menu.getItem(i);
+            if (item.isVisible()) {
+                menuIdSet.add(item.getItemId());
+            }
+        }
+        trimMenuOptions(params, menuIdSet);
+
+        // Iterate through the entire menu list, if if doesn't exist in the map, don't show it.
+        for (int i = 0; i < menu.size(); i++) {
+            MenuItem item = menu.getItem(i);
+            item.setVisible(menuIdSet.contains(item.getItemId()));
+        }
+
+        // Special case for searching by image element.
+        if (menuIdSet.contains(R.id.contextmenu_search_by_image)) {
+            menu.findItem(R.id.contextmenu_search_by_image)
+                    .setTitle(context.getString(R.string.contextmenu_search_web_for_image,
+                            TemplateUrlService.getInstance()
+                                    .getDefaultSearchEngineTemplateUrl()
+                                    .getShortName()));
+        }
+    }
+
+    /**
+     * Given a set of params and context, and a list of all the items. This method never adds items,
+     * only removes or edits them from the
+     * list.
+     * @param params The parameters used for filtering out the the list of items.
+     * @param allItems A set, with all the possible options for a link represented as ids Items will
+     *                 be removed based off the parameters.
+     */
+    private void trimMenuOptions(ContextMenuParams params, Set<Integer> allItems) {

[Ted C, 2017/02/28 06:10:13]

Instead of this getting the full list of items, what does this look like if it
returned a set of the disabled items (i.e. adding to the set instead of
removing).

Set<Integer> supportedOptions = new HashSet<>();
Collections.addAll(supportedOptions, BASE_WHITELIST);
Collections.addAll(supportedOptions, <mode specific whitelist>);

Set<Integer> disabledOptions = <this method call>;

for (int i = 0; i < menu.size(); i++) {
    item.setVisible(supportedOptions.contains(itemId) &&
!disabledOptions.contains(itemId));
}

It's not cutting down on a ton of iterations or anything, but I think having
methods have less access to the larger state is better.  In this case, there is
no enforcement that it is actually trimming as you could add as well, so making
it easier to enforce contracts is preferable to me.

[JJ, 2017/03/01 01:41:04]

Alright, take a look at what I did. While it does make one item have access to a
method, a second had to be made since adding all to the baselist doesn't
properly work.

Also the only way to access the groups is to access the entire menu and if we do
that then the point of this rather moot. So there's a second method to address
this that trims the basic options. I realize this is a little bit of a problem
and I do address this in a future cl (where we partially get rid of
chrome_context_menu.xml) so bare with me partially.

If you feel this adds more confusion feel free to tell me and we can revert.

         if (params.isAnchor() && !mDelegate.isOpenInOtherWindowSupported()) {
-            menu.findItem(R.id.contextmenu_open_in_other_window).setVisible(false);
+            allItems.remove(R.id.contextmenu_open_in_other_window);
         }
 
         if (mDelegate.isIncognito() || !mDelegate.isIncognitoSupported()) {
-            menu.findItem(R.id.contextmenu_open_in_incognito_tab).setVisible(false);
+            allItems.remove(R.id.contextmenu_open_in_incognito_tab);
         }
 
         if (params.getLinkText().trim().isEmpty() || params.isImage()) {
-            menu.findItem(R.id.contextmenu_copy_link_text).setVisible(false);
+            allItems.remove(R.id.contextmenu_copy_link_text);
         }
 
         if (params.isAnchor() && !UrlUtilities.isAcceptedScheme(params.getLinkUrl())) {
-            menu.findItem(R.id.contextmenu_open_in_other_window).setVisible(false);
-            menu.findItem(R.id.contextmenu_open_in_new_tab).setVisible(false);
-            menu.findItem(R.id.contextmenu_open_in_incognito_tab).setVisible(false);
+            allItems.remove(R.id.contextmenu_open_in_other_window);
+            allItems.remove(R.id.contextmenu_open_in_new_tab);
+            allItems.remove(R.id.contextmenu_open_in_incognito_tab);
         }
 
         if (MailTo.isMailTo(params.getLinkUrl())) {
-            menu.findItem(R.id.contextmenu_copy_link_text).setVisible(false);
-            menu.findItem(R.id.contextmenu_copy_link_address).setVisible(false);
-            menu.setGroupVisible(R.id.contextmenu_group_message, true);
+            allItems.remove(R.id.contextmenu_copy_link_text);
+            allItems.remove(R.id.contextmenu_copy_link_address);
             if (!mDelegate.supportsSendEmailMessage()) {
-                menu.findItem(R.id.contextmenu_send_message).setVisible(false);
+                allItems.remove(R.id.contextmenu_send_message);
             }
             if (TextUtils.isEmpty(MailTo.parse(params.getLinkUrl()).getTo())
                     || !mDelegate.supportsAddToContacts()) {
-                menu.findItem(R.id.contextmenu_add_to_contacts).setVisible(false);
+                allItems.remove(R.id.contextmenu_add_to_contacts);
             }
-            menu.findItem(R.id.contextmenu_call).setVisible(false);
+            allItems.remove(R.id.contextmenu_call);
         } else if (UrlUtilities.isTelScheme(params.getLinkUrl())) {
-            menu.findItem(R.id.contextmenu_copy_link_text).setVisible(false);
-            menu.findItem(R.id.contextmenu_copy_link_address).setVisible(false);
-            menu.setGroupVisible(R.id.contextmenu_group_message, true);
+            allItems.remove(R.id.contextmenu_copy_link_text);
+            allItems.remove(R.id.contextmenu_copy_link_address);
             if (!mDelegate.supportsCall()) {
-                menu.findItem(R.id.contextmenu_call).setVisible(false);
+                allItems.remove(R.id.contextmenu_call);
             }
             if (!mDelegate.supportsSendTextMessage()) {
-                menu.findItem(R.id.contextmenu_send_message).setVisible(false);
+                allItems.remove(R.id.contextmenu_send_message);
             }
             if (!mDelegate.supportsAddToContacts()) {
-                menu.findItem(R.id.contextmenu_add_to_contacts).setVisible(false);
+                allItems.remove(R.id.contextmenu_add_to_contacts);
             }
-        } else {
-            menu.setGroupVisible(R.id.contextmenu_group_message, false);
         }
 
-        menu.findItem(R.id.contextmenu_save_link_as).setVisible(
-                UrlUtilities.isDownloadableScheme(params.getLinkUrl()));
+        if (!UrlUtilities.isDownloadableScheme(params.getLinkUrl())) {
+            allItems.remove(R.id.contextmenu_save_link_as);
+        }
 
         if (params.imageWasFetchedLoFi()
                 || !DataReductionProxySettings.getInstance().wasLoFiModeActiveOnMainFrame()
                 || !DataReductionProxySettings.getInstance().canUseDataReductionProxy(
                         params.getPageUrl())) {
-            menu.findItem(R.id.contextmenu_load_images).setVisible(false);
+            allItems.remove(R.id.contextmenu_load_images);
         } else {
             // Links can have images as backgrounds that aren't recognized here as images. CSS
             // properties can also prevent an image underlying a link from being clickable.
             // When Lo-Fi is active, provide the user with a "Load images" option on links
             // to get the images in these cases.
             DataReductionProxyUma.previewsLoFiContextMenuAction(
                     DataReductionProxyUma.ACTION_LOFI_LOAD_IMAGES_CONTEXT_MENU_SHOWN);
         }
 
+        boolean isSrcDownloadableScheme = UrlUtilities.isDownloadableScheme(params.getSrcUrl());
         if (params.isVideo()) {
-            menu.findItem(R.id.contextmenu_save_video).setVisible(
-                    params.canSaveMedia() && UrlUtilities.isDownloadableScheme(params.getSrcUrl()));
+            boolean saveableAndDownloadable = params.canSaveMedia() && isSrcDownloadableScheme;
+            if (!saveableAndDownloadable) {
+                allItems.remove(R.id.contextmenu_save_video);
+            }
         } else if (params.isImage() && params.imageWasFetchedLoFi()) {
             DataReductionProxyUma.previewsLoFiContextMenuAction(
                     DataReductionProxyUma.ACTION_LOFI_LOAD_IMAGE_CONTEXT_MENU_SHOWN);
             // All image context menu items other than "Load image," "Open original image in
             // new tab," and "Copy image URL" should be disabled on Lo-Fi images.
-            menu.findItem(R.id.contextmenu_save_image).setVisible(false);
-            menu.findItem(R.id.contextmenu_open_image).setVisible(false);
-            menu.findItem(R.id.contextmenu_search_by_image).setVisible(false);
-            menu.findItem(R.id.contextmenu_share_image).setVisible(false);
+            allItems.remove(R.id.contextmenu_save_image);
+            allItems.remove(R.id.contextmenu_open_image);
+            allItems.remove(R.id.contextmenu_search_by_image);
+            allItems.remove(R.id.contextmenu_share_image);
         } else if (params.isImage() && !params.imageWasFetchedLoFi()) {
-            menu.findItem(R.id.contextmenu_load_original_image).setVisible(false);
+            allItems.remove(R.id.contextmenu_load_original_image);
 
-            menu.findItem(R.id.contextmenu_save_image).setVisible(
-                    UrlUtilities.isDownloadableScheme(params.getSrcUrl()));
+            if (!isSrcDownloadableScheme) {
+                allItems.remove(R.id.contextmenu_save_image);
+            }
 
             // Avoid showing open image option for same image which is already opened.
             if (mDelegate.getPageUrl().equals(params.getSrcUrl())) {
-                menu.findItem(R.id.contextmenu_open_image).setVisible(false);
+                allItems.remove(R.id.contextmenu_open_image);
             }
             final TemplateUrlService templateUrlServiceInstance = TemplateUrlService.getInstance();
-            final boolean isSearchByImageAvailable =
-                    UrlUtilities.isDownloadableScheme(params.getSrcUrl())
-                            && templateUrlServiceInstance.isLoaded()
-                            && templateUrlServiceInstance.isSearchByImageAvailable()
-                            && templateUrlServiceInstance.getDefaultSearchEngineTemplateUrl()
-                                    != null;
+            final boolean isSearchByImageAvailable = isSrcDownloadableScheme
+                    && templateUrlServiceInstance.isLoaded()
+                    && templateUrlServiceInstance.isSearchByImageAvailable()
+                    && templateUrlServiceInstance.getDefaultSearchEngineTemplateUrl() != null;
 
-            menu.findItem(R.id.contextmenu_search_by_image).setVisible(isSearchByImageAvailable);
-            if (isSearchByImageAvailable) {
-                menu.findItem(R.id.contextmenu_search_by_image).setTitle(
-                        context.getString(R.string.contextmenu_search_web_for_image,
-                                TemplateUrlService.getInstance()
-                                        .getDefaultSearchEngineTemplateUrl().getShortName()));
+            if (!isSearchByImageAvailable) {
+                allItems.remove(R.id.contextmenu_search_by_image);
             }
         }
 
         // Hide all items that could spawn additional tabs until FRE has been completed.
         if (!FirstRunStatus.getFirstRunFlowComplete()) {
-            menu.findItem(R.id.contextmenu_open_image_in_new_tab).setVisible(false);
-            menu.findItem(R.id.contextmenu_open_in_other_window).setVisible(false);
-            menu.findItem(R.id.contextmenu_open_in_new_tab).setVisible(false);
-            menu.findItem(R.id.contextmenu_open_in_incognito_tab).setVisible(false);
-            menu.findItem(R.id.contextmenu_search_by_image).setVisible(false);
-            menu.findItem(R.id.menu_id_open_in_chrome).setVisible(false);
+            allItems.remove(R.id.contextmenu_open_image_in_new_tab);
+            allItems.remove(R.id.contextmenu_open_in_other_window);
+            allItems.remove(R.id.contextmenu_open_in_new_tab);
+            allItems.remove(R.id.contextmenu_open_in_incognito_tab);
+            allItems.remove(R.id.contextmenu_search_by_image);
+            allItems.remove(R.id.menu_id_open_in_chrome);
         }
 
         if (mMode == FULLSCREEN_TAB_MODE) {
-            removeUnsupportedItems(menu, FULLSCREEN_TAB_MODE_WHITELIST);
+            removeUnsupportedItems(allItems, FULLSCREEN_TAB_MODE_WHITELIST);
         } else if (mMode == CUSTOM_TAB_MODE) {
-            removeUnsupportedItems(menu, CUSTOM_TAB_MODE_WHITELIST);
+            removeUnsupportedItems(allItems, CUSTOM_TAB_MODE_WHITELIST);
         } else {
-            removeUnsupportedItems(menu, NORMAL_MODE_WHITELIST);
+            removeUnsupportedItems(allItems, NORMAL_MODE_WHITELIST);
         }
     }
 
-    private void removeUnsupportedItems(ContextMenu menu, int[] whitelist) {
+    private void removeUnsupportedItems(Set<Integer> itemList, int[] whitelist) {
         Arrays.sort(BASE_WHITELIST);
         Arrays.sort(whitelist);
-        for (int i = 0; i < menu.size(); i++) {
-            MenuItem item = menu.getItem(i);
-            if (Arrays.binarySearch(whitelist, item.getItemId()) < 0
-                    && Arrays.binarySearch(BASE_WHITELIST, item.getItemId()) < 0) {
-                menu.removeItem(item.getItemId());
-                i--;
+        for (Iterator<Integer> idIterator = itemList.iterator(); idIterator.hasNext();) {
+            int itemId = idIterator.next();
+            if (Arrays.binarySearch(whitelist, itemId) < 0
+                    && Arrays.binarySearch(BASE_WHITELIST, itemId) < 0) {
+                idIterator.remove();
             }
         }
     }
 
     @Override
     public boolean onItemSelected(ContextMenuHelper helper, ContextMenuParams params, int itemId) {
         if (itemId == R.id.contextmenu_open_in_other_window) {
             ContextMenuUma.record(params, ContextMenuUma.ACTION_OPEN_IN_OTHER_WINDOW);
             mDelegate.onOpenInOtherWindow(params.getLinkUrl(), params.getReferrer());
         } else if (itemId == R.id.contextmenu_open_in_new_tab) {
@@ skipping 86 matching lines @@
         }
 
         return true;
     }
 
     private void setHeaderText(Context context, ContextMenu menu, String text) {
         ContextMenuTitleView title = new ContextMenuTitleView(context, text);
         menu.setHeaderView(title);
     }
 }