Split context menu display and population/handling

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

Index: chrome/android/java/src/org/chromium/chrome/browser/contextmenu/SimpleContextMenuUi.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/SimpleContextMenuUi.java b/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/SimpleContextMenuUi.java
new file mode 100644
index 0000000000000000000000000000000000000000..efe65c28708b34bd062974d61cf76194e06dd5c8
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/SimpleContextMenuUi.java
@@ -0,0 +1,62 @@
+// Copyright 2017 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.app.Activity;
+import android.content.Context;
+import android.util.Pair;
+import android.view.ContextMenu;
+import android.view.MenuItem;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * A context menu that displays the Android Standard Context Menu. Comes the Item Groups into one
+ * list displayed in the order received.
+ */
+public class SimpleContextMenuUi implements ContextMenuUi {

[Ted C, 2017/03/11 00:31:14]

as mentioned offline, I think PlatformContextMenuUi or FrameworkContextMenuUi is
better.

[JJ, 2017/03/13 20:24:29]

Done.

+    private ContextMenu mMenu;
+    private OnMenuClickedListener mMenuClickedListener;
+
+    SimpleContextMenuUi(ContextMenu menu, OnMenuClickedListener listener) {
+        mMenu = menu;
+        mMenuClickedListener = listener;
+    }
+
+    @Override
+    public void displayMenu(Activity activity, ContextMenuParams params,
+            List<Pair<Integer, List<ContextMenuItems>>> itemGroups) {
+        setHeaderText(activity, mMenu, ChromeContextMenuPopulator.createHeaderText(params));
+
+        if (params.isFile()) return;

[Ted C, 2017/03/11 00:31:14]

yeah, since this is in two places, moving it to the shared point definitely
seems right to me.

[JJ, 2017/03/13 23:39:32]

Done.

+
+        List<ContextMenuItems> items = new ArrayList<>();

[Ted C, 2017/03/11 00:31:14]

I wouldn't create this intermediate list.  Just have a nested for loop that adds
things to the menu

[JJ, 2017/03/13 20:24:29]

nested loops it is!

+        for (Pair<Integer, List<ContextMenuItems>> group : itemGroups) {

[Ted C, 2017/03/11 00:31:14]

same thing about using the for (int i = 0; i <...) for Lists instead of the
foreach iterators

[JJ, 2017/03/13 20:24:29]

Done.

+            items.addAll(group.second);
+        }
+        for (ContextMenuItems item : items) {
+            mMenu.add(0, item.menuId, 0, item.getString(activity));
+        }
+
+        for (int i = 0; i < mMenu.size(); i++) {

[Ted C, 2017/03/11 00:31:14]

can this just be part of the loop above?

[JJ, 2017/03/13 20:24:29]

Done.

+            MenuItem menuItem = mMenu.getItem(i);
+            if (menuItem.isVisible()) {

[Ted C, 2017/03/11 00:31:14]

they're all going to be visible at this point right?

[JJ, 2017/03/13 20:24:29]

Yup

+                menuItem.setOnMenuItemClickListener(new MenuItem.OnMenuItemClickListener() {
+                    @Override
+                    public boolean onMenuItemClick(MenuItem menuItem) {
+                        mMenuClickedListener.onItemClicked(menuItem.getItemId());
+                        return true;
+                    }
+                });
+            }
+        }
+    }
+
+    private void setHeaderText(Context context, ContextMenu menu, String text) {
+        ContextMenuTitleView title = new ContextMenuTitleView(context, text);
+        menu.setHeaderView(title);
+    }
+}