📰 Add a context menu to remove status items

chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java

 // Copyright 2016 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.ntp;
 
 import android.support.annotation.IntDef;
 import android.support.v4.view.ViewCompat;
 import android.view.ContextMenu;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.MenuItem.OnMenuItemClickListener;
 import android.view.View;
 
 import org.chromium.base.Callback;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager;
 import org.chromium.ui.mojom.WindowOpenDisposition;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 
 /**
  * Takes care of creating a context menu and triaging the time clicks.
  */
 public class ContextMenuHandler implements OnMenuItemClickListener {
     @IntDef({ID_OPEN_IN_NEW_WINDOW, ID_OPEN_IN_NEW_TAB, ID_OPEN_IN_INCOGNITO_TAB, ID_REMOVE,
             ID_SAVE_FOR_OFFLINE})
     @Retention(RetentionPolicy.SOURCE)
-    @interface ContextMenuItemId {}
-    static final int ID_OPEN_IN_NEW_WINDOW = 0;
-    static final int ID_OPEN_IN_NEW_TAB = 1;
-    static final int ID_OPEN_IN_INCOGNITO_TAB = 2;
-    static final int ID_REMOVE = 3;
-    static final int ID_SAVE_FOR_OFFLINE = 4;
+    public @interface ContextMenuItemId {}
+    public static final int ID_OPEN_IN_NEW_WINDOW = 0;
+    public static final int ID_OPEN_IN_NEW_TAB = 1;
+    public static final int ID_OPEN_IN_INCOGNITO_TAB = 2;
+    public static final int ID_REMOVE = 3;
+    public static final int ID_SAVE_FOR_OFFLINE = 4;
 
     private final NewTabPageManager mManager;
     private final Delegate mDelegate;
     private final TouchDisableableView mOuterView;
 
     /** Defines callback to configure the context menu and respond to user interaction. */
     public interface Delegate {
         /** Opens the current item the way specified by {@code windowDisposition}. */
         void openItem(int windowDisposition);
 
         /** Removed the current item. */
         void removeItem();
 
         /** @return whether the current item can be saved offline. */
         boolean canBeSavedOffline();
     }
 
     /** Interface for a view that can be set to stop responding to touches. */
     public interface TouchDisableableView {
         void setTouchEnabled(boolean enabled);
         View asView();
     }
 
     public ContextMenuHandler(NewTabPageManager newTabPageManager, TouchDisableableView outerView,
             Delegate delegate) {
-        assert outerView instanceof View;
         mManager = newTabPageManager;
         mOuterView = outerView;
         mDelegate = delegate;
     }
 
     public boolean showItem(@ContextMenuItemId int itemId) {
         switch (itemId) {
             case ID_OPEN_IN_NEW_WINDOW:
                 return mManager.isOpenInNewWindowEnabled();
             case ID_OPEN_IN_NEW_TAB:
                 return true;
             case ID_OPEN_IN_INCOGNITO_TAB:
                 return mManager.isOpenInIncognitoEnabled();
             case ID_SAVE_FOR_OFFLINE:
                 return mDelegate.canBeSavedOffline();
             case ID_REMOVE:
                 return true;
 
             default:
                 return false;
         }
     }
 
-    public void onCreateContextMenu(ContextMenu menu) {
+    public void onCreateContextMenu(ContextMenu menu, final View associatedView) {
         if (showItem(ID_OPEN_IN_NEW_WINDOW)) {
             addContextMenuItem(
                     menu, ID_OPEN_IN_NEW_WINDOW, R.string.contextmenu_open_in_other_window);
         }
 
         if (showItem(ID_OPEN_IN_NEW_TAB)) {
             addContextMenuItem(menu, ID_OPEN_IN_NEW_TAB, R.string.contextmenu_open_in_new_tab);
         }
 
         if (showItem(ID_OPEN_IN_INCOGNITO_TAB)) {
             addContextMenuItem(
                     menu, ID_OPEN_IN_INCOGNITO_TAB, R.string.contextmenu_open_in_incognito_tab);
         }
 
         if (showItem(ID_SAVE_FOR_OFFLINE)) {
             addContextMenuItem(menu, ID_SAVE_FOR_OFFLINE, R.string.contextmenu_save_link);
         }
 
         if (showItem(ID_REMOVE)) {
             addContextMenuItem(menu, ID_REMOVE, R.string.remove);
         }
 
         // Disable touch events on the RecyclerView while the context menu is open. This is to
         // prevent the user long pressing to get the context menu then on the same press scrolling
         // or swiping to dismiss an item (eg. https://crbug.com/638854, https://crbug.com/638555,
         // https://crbug.com/636296)
         mOuterView.setTouchEnabled(false);
 
+        mManager.getContextMenuLifetimeManager().onContextMenuCreated(menu, associatedView);

[Bernhard Bauer, 2016/11/11 13:30:08]

Wait, so the manager owns the LifetimeManager, which is defined by this class
and has a private method that only we are able to call? That seems like very
tight coupling. Could we move this into its own class? And maybe we could move
the contextMenuCloseCallback methods to that class as well, making it a
ContextMenuManager or some'n?

[dgn, 2016/11/14 18:34:08]

Done.

+
         mManager.addContextMenuCloseCallback(new Callback<Menu>() {
             @Override
             public void onResult(Menu result) {
                 mOuterView.setTouchEnabled(true);
                 mManager.removeContextMenuCloseCallback(this);
+                mManager.getContextMenuLifetimeManager().onMenuClosed();
             }
         });
     }
 
     @Override
     public boolean onMenuItemClick(MenuItem item) {
         // If the user clicks a snippet then immediately long presses they will create a context
         // menu while the snippet's URL loads in the background. This means that when they press
         // an item on context menu the NTP will not actually be open. We add this check here to
         // prevent taking any action if the user has already left the NTP.
@@ skipping 21 matching lines @@
                 return false;
         }
     }
 
     /**
      * Convenience method to reduce multi-line function call to single line.
      */
     private void addContextMenuItem(ContextMenu menu, @ContextMenuItemId int id, int resourceId) {
         menu.add(Menu.NONE, id, Menu.NONE, resourceId).setOnMenuItemClickListener(this);
     }
+
+    /** Helps closing open context menus when the view the associated view is modified. */
+    public static class LifetimeManager {
+        private ContextMenu mContextMenu;
+        private View mAssociatedView;
+
+        public void onContextMenuCreated(ContextMenu c, View v) {
+            mContextMenu = c;
+            mAssociatedView = v;
+        }
+
+        public void onViewModified(View v) {
+            if (mAssociatedView == null || mAssociatedView != v) return;

[Bernhard Bauer, 2016/11/11 13:30:08]

|mAssociatedView| would be null if either that is passed at construction time,
or the menu is closed, right? In the former case, that will make the whole class
a no-op, so the client could just skip creating it, and in the latter case the
next line will catch it anyway.

[dgn, 2016/11/14 18:34:08]

Simplified this and removed the redundant checks.

+            if (mContextMenu == null) return;
+            mContextMenu.close();
+            mContextMenu = null;
+        }
+
+        private void onMenuClosed() {
+            mContextMenu = null;
+            mAssociatedView = null;
+        }
+    }
 }