Refactor NavigationPopup to not expose the View type to clients.

This is a step towards removing ListPopupWindow and using a vanilla
Popup to deal with styling issues.

BUG=800033

Change-Id: I79506cdd7c3e840e13e9ebbabce39948a9fa5936
Reviewed-on: https://chromium-review.googlesource.com/c/1259480
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Becky Zhou <huayinz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596734}

chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

@@ -2092,29 +2092,8 @@ public class ChromeTabbedActivity
         mNavigationPopup = new NavigationPopup(tab.getProfile(), this,
                 tab.getWebContents().getNavigationController(),
                 NavigationPopup.Type.ANDROID_SYSTEM_BACK);
-        mNavigationPopup.setWidth(
-                getResources().getDimensionPixelSize(R.dimen.navigation_popup_width));
-        mNavigationPopup.setAnchorView(findViewById(R.id.navigation_popup_anchor_stub));
-        mNavigationPopup.setOnDismissListener(() -> mNavigationPopup = null);
-
-        positionAndShowNavigationPopup();
-    }
-
-    @Override
-    public void onOrientationChange(int orientation) {
-        super.onOrientationChange(orientation);
-        positionAndShowNavigationPopup();
-    }
-
-    private void positionAndShowNavigationPopup() {
-        if (mNavigationPopup == null) return;
-
-        // Center popup window.
-        ViewGroup coordinator = findViewById(R.id.coordinator);
-        int horizontalOffset = coordinator.getWidth() / 2 - mNavigationPopup.getWidth() / 2;
-        if (horizontalOffset > 0) mNavigationPopup.setHorizontalOffset(horizontalOffset);
-
-        mNavigationPopup.show();
+        mNavigationPopup.setOnDismissCallback(() -> mNavigationPopup = null);
+        mNavigationPopup.show(findViewById(R.id.navigation_popup_anchor_stub));
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/NavigationPopup.java

@@ -5,12 +5,16 @@
 package org.chromium.chrome.browser;
 
 import android.content.Context;
+import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.support.annotation.IntDef;
+import android.support.annotation.Nullable;
+import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.View.OnAttachStateChangeListener;
+import android.view.View.OnLayoutChangeListener;
 import android.view.ViewGroup;
 import android.widget.AdapterView;
 import android.widget.BaseAdapter;
@@ -41,8 +45,7 @@ import java.util.Set;
 /**
  * A popup that handles displaying the navigation history for a given tab.
  */
-public class NavigationPopup extends ListPopupWindow implements AdapterView.OnItemClickListener {
-
+public class NavigationPopup implements AdapterView.OnItemClickListener {
     private static final int MAXIMUM_HISTORY_ITEMS = 8;
     private static final int FULL_HISTORY_ENTRY_INDEX = -1;
 
@@ -57,12 +60,14 @@ public class NavigationPopup extends ListPopupWindow implements AdapterView.OnIt
 
     private final Profile mProfile;
     private final Context mContext;
+    private final ListPopupWindow mPopup;
     private final NavigationController mNavigationController;
     private NavigationHistory mHistory;
     private final NavigationAdapter mAdapter;
     private final @Type int mType;
-
     private final int mFaviconSize;
+    @Nullable
+    private final OnLayoutChangeListener mAnchorViewLayoutChangeListener;
 
     private DefaultFaviconHelper mDefaultFaviconHelper;
 
@@ -70,6 +75,7 @@ public class NavigationPopup extends ListPopupWindow implements AdapterView.OnIt
      * Loads the favicons asynchronously.
      */
     private FaviconHelper mFaviconHelper;
+    private Runnable mOnDismissCallback;
 
     private boolean mInitialized;
 
@@ -83,9 +89,9 @@ public class NavigationPopup extends ListPopupWindow implements AdapterView.OnIt
      */
     public NavigationPopup(Profile profile, Context context,
             NavigationController navigationController, @Type int type) {
-        super(context, null, android.R.attr.popupMenuStyle);
         mProfile = profile;
         mContext = context;
+        Resources resources = mContext.getResources();
         mNavigationController = navigationController;
         mType = type;
 
@@ -95,51 +101,105 @@ public class NavigationPopup extends ListPopupWindow implements AdapterView.OnIt
         mHistory = mNavigationController.getDirectedNavigationHistory(
                 isForward, MAXIMUM_HISTORY_ITEMS);
         mHistory.addEntry(new NavigationEntry(FULL_HISTORY_ENTRY_INDEX, UrlConstants.HISTORY_URL,
-                null, null, null, mContext.getResources().getString(R.string.show_full_history),
-                null, 0));
-
-        setBackgroundDrawable(ApiCompatibilityUtils.getDrawable(mContext.getResources(),
-                anchorToBottom ? R.drawable.popup_bg_bottom : R.drawable.popup_bg));
-        // By default ListPopupWindow uses the top & bottom padding of the background to determine
-        // the vertical offset applied to the window.  This causes the popup to be shifted up
-        // by the top padding, and thus we forcibly need to specify a vertical offset of 0 to
-        // prevent that.
-        if (anchorToBottom) setVerticalOffset(0);
+                null, null, null, resources.getString(R.string.show_full_history), null, 0));
 
         mAdapter = new NavigationAdapter();
-        if (anchorToBottom) mAdapter.reverseOrder();
 
-        setModal(true);
-        setInputMethodMode(PopupWindow.INPUT_METHOD_NOT_NEEDED);
-        setHeight(ViewGroup.LayoutParams.WRAP_CONTENT);
-        setOnItemClickListener(this);
-        setAdapter(mAdapter);
+        mPopup = new ListPopupWindow(context);
+        mPopup.setOnDismissListener(this::onDismiss);
+        mPopup.setBackgroundDrawable(ApiCompatibilityUtils.getDrawable(
+                resources, anchorToBottom ? R.drawable.popup_bg_bottom : R.drawable.popup_bg));
+        mPopup.setModal(true);
+        mPopup.setInputMethodMode(PopupWindow.INPUT_METHOD_NOT_NEEDED);
+        mPopup.setHeight(ViewGroup.LayoutParams.WRAP_CONTENT);
+        mPopup.setOnItemClickListener(this);
+        mPopup.setAdapter(mAdapter);
+        mPopup.setWidth(resources.getDimensionPixelSize(
+                anchorToBottom ? R.dimen.navigation_popup_width : R.dimen.menu_width));
+
+        if (anchorToBottom) {
+            // By default ListPopupWindow uses the top & bottom padding of the background to
+            // determine the vertical offset applied to the window.  This causes the popup to be
+            // shifted up by the top padding, and thus we forcibly need to specify a vertical offset
+            // of 0 to prevent that.
+            mPopup.setVerticalOffset(0);
+            mAnchorViewLayoutChangeListener = new OnLayoutChangeListener() {
+                @Override
+                public void onLayoutChange(View v, int left, int top, int right, int bottom,
+                        int oldLeft, int oldTop, int oldRight, int oldBottom) {
+                    centerPopupOverAnchorViewAndShow();
+                }
+            };
+            mAdapter.reverseOrder();
+        } else {
+            mAnchorViewLayoutChangeListener = null;
+        }
 
-        mFaviconSize = mContext.getResources().getDimensionPixelSize(R.dimen.default_favicon_size);
+        mFaviconSize = resources.getDimensionPixelSize(R.dimen.default_favicon_size);
+    }
+
+    @VisibleForTesting
+    ListPopupWindow getPopupForTesting() {
+        return mPopup;
     }
 
     private String buildComputedAction(String action) {
         return (mType == Type.TABLET_FORWARD ? "ForwardMenu_" : "BackMenu_") + action;
     }
 
-    @Override
-    public void show() {
+    /**
+     * Shows the popup attached to the specified anchor view.
+     */
+    public void show(View anchorView) {
         if (!mInitialized) initialize();
-        if (!isShowing()) RecordUserAction.record(buildComputedAction("Popup"));
-        super.show();
+        if (!mPopup.isShowing()) RecordUserAction.record(buildComputedAction("Popup"));
+        if (mPopup.getAnchorView() != null && mAnchorViewLayoutChangeListener != null) {
+            mPopup.getAnchorView().removeOnLayoutChangeListener(mAnchorViewLayoutChangeListener);
+        }
+        mPopup.setAnchorView(anchorView);
+        if (mType == Type.ANDROID_SYSTEM_BACK) {
+            anchorView.addOnLayoutChangeListener(mAnchorViewLayoutChangeListener);
+            centerPopupOverAnchorViewAndShow();
+        } else {
+            mPopup.show();
+        }
         if (mAdapter.mInReverseOrder) scrollToBottom();
     }
 
-    @Override
+    /**
+     * Dismisses the popup.
+     */
     public void dismiss() {
+        mPopup.dismiss();
+    }
+
+    /**
+     * Sets the callback to be notified when the popup has been dismissed.
+     * @param onDismiss The callback to be notified.
+     */
+    public void setOnDismissCallback(Runnable onDismiss) {
+        mOnDismissCallback = onDismiss;
+    }
+
+    private void centerPopupOverAnchorViewAndShow() {
+        assert mInitialized;
+        int horizontalOffset = (mPopup.getAnchorView().getWidth() - mPopup.getWidth()) / 2;
+        if (horizontalOffset > 0) mPopup.setHorizontalOffset(horizontalOffset);
+        mPopup.show();
+    }
+
+    private void onDismiss() {
         if (mInitialized) mFaviconHelper.destroy();
         mInitialized = false;
         if (mDefaultFaviconHelper != null) mDefaultFaviconHelper.clearCache();
-        super.dismiss();
+        if (mAnchorViewLayoutChangeListener != null) {
+            mPopup.getAnchorView().removeOnLayoutChangeListener(mAnchorViewLayoutChangeListener);
+        }
+        if (mOnDismissCallback != null) mOnDismissCallback.run();
     }
 
     private void scrollToBottom() {
-        getListView().addOnAttachStateChangeListener(new OnAttachStateChangeListener() {
+        mPopup.getListView().addOnAttachStateChangeListener(new OnAttachStateChangeListener() {
             @Override
             public void onViewDetachedFromWindow(View v) {
                 if (v != null) v.removeOnAttachStateChangeListener(this);
@@ -206,7 +266,7 @@ public class NavigationPopup extends ListPopupWindow implements AdapterView.OnIt
             mNavigationController.goToNavigationIndex(entry.getIndex());
         }
 
-        dismiss();
+        mPopup.dismiss();
     }
 
     private class NavigationAdapter extends BaseAdapter {

chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarTablet.java

@@ -301,13 +301,7 @@ public class ToolbarTablet
         mNavigationPopup = new NavigationPopup(tab.getProfile(), getContext(),
                 tab.getWebContents().getNavigationController(),
                 isForward ? NavigationPopup.Type.TABLET_FORWARD : NavigationPopup.Type.TABLET_BACK);
-
-        mNavigationPopup.setAnchorView(anchorView);
-
-        int menuWidth = getResources().getDimensionPixelSize(R.dimen.menu_width);
-        mNavigationPopup.setWidth(menuWidth);
-
-        mNavigationPopup.show();
+        mNavigationPopup.show(anchorView);
     }
 
     @Override

chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java

@@ -9,6 +9,7 @@ import android.support.test.filters.MediumTest;
 import android.support.test.filters.SmallTest;
 import android.view.KeyEvent;
 import android.view.View;
+import android.widget.ListPopupWindow;
 import android.widget.ListView;
 import android.widget.TextView;
 
@@ -241,7 +242,7 @@ public class NavigationPopupTest {
     @Feature({"Navigation"})
     public void testFaviconFetching() throws ExecutionException {
         final TestNavigationController controller = new TestNavigationController();
-        final NavigationPopup popup = showPopup(controller);
+        final ListPopupWindow popup = showPopup(controller);
 
         CriteriaHelper.pollUiThread(new Criteria("All favicons did not get updated.") {
             @Override
@@ -264,7 +265,7 @@ public class NavigationPopupTest {
     @Feature({"Navigation"})
     public void testItemSelection() throws ExecutionException {
         final TestNavigationController controller = new TestNavigationController();
-        final NavigationPopup popup = showPopup(controller);
+        final ListPopupWindow popup = showPopup(controller);
 
         ThreadUtils.runOnUiThreadBlocking((Runnable) () -> popup.performItemClick(1));
 
@@ -278,7 +279,7 @@ public class NavigationPopupTest {
     @Feature({"Navigation"})
     public void testShowAllHistory() throws ExecutionException {
         final TestNavigationController controller = new TestNavigationController();
-        final NavigationPopup popup = showPopup(controller);
+        final ListPopupWindow popup = showPopup(controller);
 
         ThreadUtils.runOnUiThreadBlocking(() -> {
             ListView list = popup.getListView();
@@ -331,16 +332,12 @@ public class NavigationPopupTest {
                 () -> mActivityTestRule.getActivity().getNavigationPopupForTesting()));
     }
 
-    private NavigationPopup showPopup(NavigationController controller) throws ExecutionException {
+    private ListPopupWindow showPopup(NavigationController controller) throws ExecutionException {
         return ThreadUtils.runOnUiThreadBlocking(() -> {
             NavigationPopup popup = new NavigationPopup(mProfile, mActivityTestRule.getActivity(),
                     controller, NavigationPopup.Type.TABLET_FORWARD);
-            popup.setWidth(300);
-            popup.setHeight(300);
-            popup.setAnchorView(mActivityTestRule.getActivity().getActivityTab().getContentView());
-
-            popup.show();
-            return popup;
+            popup.show(mActivityTestRule.getActivity().getActivityTab().getContentView());
+            return popup.getPopupForTesting();
         });
     }