Revert "Refactor NavigationPopup to not expose the View type to clients."

This reverts commit 7cff6f2f.

Reason for revert: NavigationPopupTest.testFaviconFetching started crashing after this landed in https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20CFI/3164

Original change's description:
> 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}

TBR=tedchoc@chromium.org,huayinz@chromium.org

Change-Id: I6a7d3c914fcca88e64eea4158fbea9db224d762c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 800033
Reviewed-on: https://chromium-review.googlesource.com/c/1263344Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596905}

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

@@ -2092,8 +2092,29 @@ public class ChromeTabbedActivity
         mNavigationPopup = new NavigationPopup(tab.getProfile(), this,
                 tab.getWebContents().getNavigationController(),
                 NavigationPopup.Type.ANDROID_SYSTEM_BACK);
-        mNavigationPopup.setOnDismissCallback(() -> mNavigationPopup = null);
-        mNavigationPopup.show(findViewById(R.id.navigation_popup_anchor_stub));
+        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();
     }
 
     @Override

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

@@ -5,16 +5,12 @@
 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;
@@ -45,7 +41,8 @@ import java.util.Set;
 /**
  * A popup that handles displaying the navigation history for a given tab.
  */
-public class NavigationPopup implements AdapterView.OnItemClickListener {
+public class NavigationPopup extends ListPopupWindow implements AdapterView.OnItemClickListener {
+
     private static final int MAXIMUM_HISTORY_ITEMS = 8;
     private static final int FULL_HISTORY_ENTRY_INDEX = -1;
 
@@ -60,14 +57,12 @@ public class NavigationPopup implements AdapterView.OnItemClickListener {
 
     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;
 
@@ -75,7 +70,6 @@ public class NavigationPopup implements AdapterView.OnItemClickListener {
      * Loads the favicons asynchronously.
      */
     private FaviconHelper mFaviconHelper;
-    private Runnable mOnDismissCallback;
 
     private boolean mInitialized;
 
@@ -89,9 +83,9 @@ public class NavigationPopup implements AdapterView.OnItemClickListener {
      */
     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;
 
@@ -101,105 +95,51 @@ public class NavigationPopup implements AdapterView.OnItemClickListener {
         mHistory = mNavigationController.getDirectedNavigationHistory(
                 isForward, MAXIMUM_HISTORY_ITEMS);
         mHistory.addEntry(new NavigationEntry(FULL_HISTORY_ENTRY_INDEX, UrlConstants.HISTORY_URL,
-                null, null, null, resources.getString(R.string.show_full_history), null, 0));
+                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);
 
         mAdapter = new NavigationAdapter();
+        if (anchorToBottom) mAdapter.reverseOrder();
 
-        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;
-        }
+        setModal(true);
+        setInputMethodMode(PopupWindow.INPUT_METHOD_NOT_NEEDED);
+        setHeight(ViewGroup.LayoutParams.WRAP_CONTENT);
+        setOnItemClickListener(this);
+        setAdapter(mAdapter);
 
-        mFaviconSize = resources.getDimensionPixelSize(R.dimen.default_favicon_size);
-    }
-
-    @VisibleForTesting
-    ListPopupWindow getPopupForTesting() {
-        return mPopup;
+        mFaviconSize = mContext.getResources().getDimensionPixelSize(R.dimen.default_favicon_size);
     }
 
     private String buildComputedAction(String action) {
         return (mType == Type.TABLET_FORWARD ? "ForwardMenu_" : "BackMenu_") + action;
     }
 
-    /**
-     * Shows the popup attached to the specified anchor view.
-     */
-    public void show(View anchorView) {
+    @Override
+    public void show() {
         if (!mInitialized) initialize();
-        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 (!isShowing()) RecordUserAction.record(buildComputedAction("Popup"));
+        super.show();
         if (mAdapter.mInReverseOrder) scrollToBottom();
     }
 
-    /**
-     * Dismisses the popup.
-     */
+    @Override
     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();
-        if (mAnchorViewLayoutChangeListener != null) {
-            mPopup.getAnchorView().removeOnLayoutChangeListener(mAnchorViewLayoutChangeListener);
-        }
-        if (mOnDismissCallback != null) mOnDismissCallback.run();
+        super.dismiss();
     }
 
     private void scrollToBottom() {
-        mPopup.getListView().addOnAttachStateChangeListener(new OnAttachStateChangeListener() {
+        getListView().addOnAttachStateChangeListener(new OnAttachStateChangeListener() {
             @Override
             public void onViewDetachedFromWindow(View v) {
                 if (v != null) v.removeOnAttachStateChangeListener(this);
@@ -266,7 +206,7 @@ public class NavigationPopup implements AdapterView.OnItemClickListener {
             mNavigationController.goToNavigationIndex(entry.getIndex());
         }
 
-        mPopup.dismiss();
+        dismiss();
     }
 
     private class NavigationAdapter extends BaseAdapter {

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

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

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

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