Bug 1184211 - Replace TwoWayView in SearchBar with RecyclerView. r=sebastian
authorJonathan Almeida [:jonalmeida] <jalmeida@mozilla.com>
Wed, 22 Jul 2015 21:06:33 -0700
changeset 256136 f52bfe71d212
parent 256135 c67439c13ec0
child 256137 cf4a7df98beb
push id29170
push usercbook@mozilla.com
push date2015-08-05 10:55 +0000
treeherdermozilla-central@5e8e09201617 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1184211, 1158291
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1184211 - Replace TwoWayView in SearchBar with RecyclerView. r=sebastian - Also includes changes from bug 1158291 applied on top of the re-written view. CLOSED TREE
mobile/android/base/home/BrowserSearch.java
mobile/android/base/home/SearchEngineAdapter.java
mobile/android/base/home/SearchEngineBar.java
mobile/android/base/moz.build
mobile/android/base/resources/layout/search_engine_bar_item.xml
mobile/android/base/resources/layout/search_engine_bar_label.xml
mobile/android/tests/browser/robocop/AboutHomeTest.java
mobile/android/tests/browser/robocop/testAddSearchEngine.java
--- a/mobile/android/base/home/BrowserSearch.java
+++ b/mobile/android/base/home/BrowserSearch.java
@@ -46,17 +46,16 @@ import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.MotionEvent;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.view.ViewStub;
-import android.view.WindowManager;
 import android.view.WindowManager.LayoutParams;
 import android.view.animation.AccelerateInterpolator;
 import android.view.animation.Animation;
 import android.view.animation.TranslateAnimation;
 import android.widget.AdapterView;
 import android.widget.LinearLayout;
 import android.widget.ListView;
 import android.widget.TextView;
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/home/SearchEngineAdapter.java
@@ -0,0 +1,122 @@
+package org.mozilla.gecko.home;
+
+import android.content.Context;
+import android.graphics.drawable.Drawable;
+import android.support.v4.content.ContextCompat;
+import android.support.v4.graphics.drawable.DrawableCompat;
+import android.support.v7.widget.RecyclerView;
+import android.view.LayoutInflater;
+import android.view.View;
+import android.view.ViewGroup;
+import android.widget.ImageView;
+
+import org.mozilla.gecko.R;
+
+import java.util.Collections;
+import java.util.List;
+
+public class SearchEngineAdapter
+        extends RecyclerView.Adapter<SearchEngineAdapter.SearchEngineViewHolder> {
+
+    private static final String LOGTAG = SearchEngineAdapter.class.getSimpleName();
+
+    private static final int VIEW_TYPE_SEARCH_ENGINE = 0;
+    private static final int VIEW_TYPE_LABEL = 1;
+    private final Context mContext;
+
+    private int mContainerWidth;
+    private List<SearchEngine> mSearchEngines = Collections.emptyList();
+
+    public void setSearchEngines(List<SearchEngine> searchEngines) {
+        mSearchEngines = searchEngines;
+        notifyDataSetChanged();
+    }
+
+    /**
+     * The container width is used for setting the appropriate calculated amount of width that
+     * a search engine icon can have. This varies depending on the space available in the
+     * {@link SearchEngineBar}. The setter exists for this attribute, in creating the view in the
+     * adapter after said calculation is done when the search bar is created.
+     * @param iconContainerWidth Width of each search icon.
+     */
+    void setIconContainerWidth(int iconContainerWidth) {
+        mContainerWidth = iconContainerWidth;
+    }
+
+    public static class SearchEngineViewHolder extends RecyclerView.ViewHolder {
+        final private ImageView faviconView;
+
+        public void bindItem(SearchEngine searchEngine) {
+            faviconView.setImageBitmap(searchEngine.getIcon());
+            final String desc = itemView.getResources().getString(R.string.search_bar_item_desc,
+                searchEngine.getEngineIdentifier());
+            itemView.setContentDescription(desc);
+        }
+
+        public SearchEngineViewHolder(View itemView) {
+            super(itemView);
+            faviconView = (ImageView) itemView.findViewById(R.id.search_engine_icon);
+        }
+    }
+
+    public SearchEngineAdapter(Context context) {
+        mContext = context;
+    }
+
+    @Override
+    public int getItemViewType(int position) {
+        return position == 0 ? VIEW_TYPE_LABEL : VIEW_TYPE_SEARCH_ENGINE;
+    }
+
+    public SearchEngine getItem(int position) {
+        // We omit the first position which is where the label currently is.
+        return position == 0 ? null : mSearchEngines.get(position - 1);
+    }
+
+    @Override
+    public SearchEngineViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
+        switch (viewType) {
+            case VIEW_TYPE_LABEL:
+                return new SearchEngineViewHolder(createLabelView(parent));
+            case VIEW_TYPE_SEARCH_ENGINE:
+                return new SearchEngineViewHolder(createSearchEngineView(parent));
+            default:
+                throw new IllegalArgumentException("Unknown view type: "  + viewType);
+        }
+    }
+
+    @Override
+    public void onBindViewHolder(SearchEngineViewHolder holder, int position) {
+        if (position != 0) {
+            holder.bindItem(getItem(position));
+        }
+    }
+
+    @Override
+    public int getItemCount() {
+        return mSearchEngines.size() + 1;
+    }
+
+    private View createLabelView(ViewGroup parent) {
+        View view = LayoutInflater.from(mContext)
+                        .inflate(R.layout.search_engine_bar_label, parent, false);
+        final Drawable icon = DrawableCompat.wrap(
+                ContextCompat.getDrawable(mContext, R.drawable.search_icon_active).mutate());
+        DrawableCompat.setTint(icon, mContext.getResources().getColor(R.color.disabled_grey));
+
+        final ImageView iconView = (ImageView) view.findViewById(R.id.search_engine_label);
+        iconView.setImageDrawable(icon);
+        return view;
+    }
+
+    private View createSearchEngineView(ViewGroup parent) {
+        View view = LayoutInflater.from(mContext)
+                    .inflate(R.layout.search_engine_bar_item, parent, false);
+
+        ViewGroup.LayoutParams params = view.getLayoutParams();
+        params.width = mContainerWidth;
+        view.setLayoutParams(params);
+
+        return view;
+    }
+}
--- a/mobile/android/base/home/SearchEngineBar.java
+++ b/mobile/android/base/home/SearchEngineBar.java
@@ -1,215 +1,146 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
- package org.mozilla.gecko.home;
+package org.mozilla.gecko.home;
 
 import android.content.Context;
 import android.graphics.Canvas;
-import android.graphics.Color;
 import android.graphics.Paint;
-import android.graphics.drawable.Drawable;
+import android.support.v7.widget.LinearLayoutManager;
+import android.support.v7.widget.RecyclerView;
 import android.util.AttributeSet;
 import android.util.DisplayMetrics;
 import android.util.TypedValue;
-import android.view.Gravity;
-import android.view.LayoutInflater;
 import android.view.View;
-import android.view.ViewGroup;
-import android.widget.AdapterView;
-import android.widget.BaseAdapter;
-import android.widget.FrameLayout;
-import android.widget.ImageView;
 
 import org.mozilla.gecko.R;
-import org.mozilla.gecko.util.DrawableUtil;
-import org.mozilla.gecko.widget.TwoWayView;
+import org.mozilla.gecko.mozglue.RobocopTarget;
 
-import java.util.ArrayList;
 import java.util.List;
 
-public class SearchEngineBar extends TwoWayView
-                             implements AdapterView.OnItemClickListener {
-    private static final String LOGTAG = "Gecko" + SearchEngineBar.class.getSimpleName();
+public class SearchEngineBar extends RecyclerView
+        implements RecyclerViewItemClickListener.OnClickListener {
+    private static final String LOGTAG = SearchEngineBar.class.getSimpleName();
 
     private static final float ICON_CONTAINER_MIN_WIDTH_DP = 72;
     private static final float LABEL_CONTAINER_WIDTH_DP = 48;
     private static final float DIVIDER_HEIGHT_DP = 1;
 
     public interface OnSearchBarClickListener {
-        public void onSearchBarClickListener(SearchEngine searchEngine);
+        void onSearchBarClickListener(SearchEngine searchEngine);
     }
 
-    private final SearchEngineAdapter adapter;
-    private final Paint dividerPaint;
-    private final float minIconContainerWidth;
-    private final float dividerHeight;
-    private final int labelContainerWidth;
+    private final SearchEngineAdapter mAdapter;
+    private final LinearLayoutManager mLayoutManager;
+    private final Paint mDividerPaint;
+    private final float mMinIconContainerWidth;
+    private final float mDividerHeight;
+    private final int mLabelContainerWidth;
 
-    private int iconContainerWidth;
-    private OnSearchBarClickListener onSearchBarClickListener;
+    private int mIconContainerWidth;
+    private OnSearchBarClickListener mOnSearchBarClickListener;
 
     public SearchEngineBar(final Context context, final AttributeSet attrs) {
         super(context, attrs);
 
-        dividerPaint = new Paint();
-        dividerPaint.setColor(getResources().getColor(R.color.divider_light));
-        dividerPaint.setStyle(Paint.Style.FILL_AND_STROKE);
+        mDividerPaint = new Paint();
+        mDividerPaint.setColor(getResources().getColor(R.color.divider_light));
+        mDividerPaint.setStyle(Paint.Style.FILL_AND_STROKE);
 
         final DisplayMetrics displayMetrics = getResources().getDisplayMetrics();
-        minIconContainerWidth = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, ICON_CONTAINER_MIN_WIDTH_DP, displayMetrics);
-        dividerHeight = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, DIVIDER_HEIGHT_DP, displayMetrics);
-        labelContainerWidth = (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, LABEL_CONTAINER_WIDTH_DP, displayMetrics);
+        mMinIconContainerWidth = TypedValue.applyDimension(
+                TypedValue.COMPLEX_UNIT_DIP, ICON_CONTAINER_MIN_WIDTH_DP, displayMetrics);
+        mDividerHeight = TypedValue.applyDimension(
+                TypedValue.COMPLEX_UNIT_DIP, DIVIDER_HEIGHT_DP, displayMetrics);
+        mLabelContainerWidth = Math.round(TypedValue.applyDimension(
+                TypedValue.COMPLEX_UNIT_DIP, LABEL_CONTAINER_WIDTH_DP, displayMetrics));
+
+        mIconContainerWidth = Math.round(mMinIconContainerWidth);
 
-        iconContainerWidth =  (int) minIconContainerWidth;
+        mAdapter = new SearchEngineAdapter(context);
+        mAdapter.setIconContainerWidth(mIconContainerWidth);
+        mLayoutManager = new LinearLayoutManager(context);
+        mLayoutManager.setOrientation(LinearLayoutManager.HORIZONTAL);
 
-        adapter = new SearchEngineAdapter();
-        setAdapter(adapter);
-        setOnItemClickListener(this);
+        setAdapter(mAdapter);
+        setLayoutManager(mLayoutManager);
+        addOnItemTouchListener(new RecyclerViewItemClickListener(context, this, this));
     }
 
-    @Override
-    public void onItemClick(final AdapterView<?> parent, final View view, final int position,
-            final long id) {
-        if (onSearchBarClickListener == null) {
-            throw new IllegalStateException(
-                    OnSearchBarClickListener.class.getSimpleName() + " is not initialized");
-        }
-
-        if (position == 0) {
-            // Ignore click on label
-            return;
-        }
-
-        final SearchEngine searchEngine = adapter.getItem(position);
-        onSearchBarClickListener.onSearchBarClickListener(searchEngine);
+    public void setSearchEngines(List<SearchEngine> searchEngines) {
+        mAdapter.setSearchEngines(searchEngines);
     }
 
-    protected void setOnSearchBarClickListener(final OnSearchBarClickListener listener) {
-        onSearchBarClickListener = listener;
-    }
-
-    protected void setSearchEngines(final List<SearchEngine> searchEngines) {
-        adapter.setSearchEngines(searchEngines);
+    public void setOnSearchBarClickListener(OnSearchBarClickListener listener) {
+        mOnSearchBarClickListener = listener;
     }
 
     @Override
     protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
         super.onMeasure(widthMeasureSpec, heightMeasureSpec);
 
-        final int searchEngineCount = adapter.getCount() - 1;
+        final int searchEngineCount = mAdapter.getItemCount() - 1;
 
         if (searchEngineCount > 0) {
-            final int availableWidth = getMeasuredWidth() - labelContainerWidth;
+            final int availableWidth = getMeasuredWidth() - mLabelContainerWidth;
             final double searchEnginesToDisplay;
 
-            if (searchEngineCount * minIconContainerWidth <= availableWidth) {
+            if (searchEngineCount * mMinIconContainerWidth <= availableWidth) {
                 // All search engines fit int: So let's just display all.
                 searchEnginesToDisplay = searchEngineCount;
             } else {
                 // If only (n) search engines fit into the available space then display (n - 0.5): The last search
                 // engine will be cut-off to show ability to scroll this view
-
-                searchEnginesToDisplay = Math.floor(availableWidth / minIconContainerWidth) - 0.5;
+                searchEnginesToDisplay = Math.floor(availableWidth / mMinIconContainerWidth) - 0.5;
             }
 
             // Use all available width and spread search engine icons
             final int availableWidthPerContainer = (int) (availableWidth / searchEnginesToDisplay);
 
-            if (availableWidthPerContainer != iconContainerWidth) {
-                iconContainerWidth = availableWidthPerContainer;
-                adapter.notifyDataSetChanged();
+            if (availableWidthPerContainer != mIconContainerWidth) {
+                mIconContainerWidth = availableWidthPerContainer;
             }
+            mAdapter.setIconContainerWidth(mIconContainerWidth);
         }
     }
 
     @Override
-    protected void onDraw(Canvas canvas) {
+    public void onDraw(Canvas canvas) {
         super.onDraw(canvas);
 
-        canvas.drawRect(0, 0, getWidth(), dividerHeight, dividerPaint);
+        canvas.drawRect(0, 0, getWidth(), mDividerHeight, mDividerPaint);
     }
 
-    public class SearchEngineAdapter extends BaseAdapter {
-        private static final int VIEW_TYPE_SEARCH_ENGINE = 0;
-        private static final int VIEW_TYPE_LABEL = 1;
-
-        List<SearchEngine> searchEngines = new ArrayList<>();
-
-        public void setSearchEngines(final List<SearchEngine> searchEngines) {
-            this.searchEngines = searchEngines;
-            notifyDataSetChanged();
+    @Override
+    public void onClick(View view, int position) {
+        if (mOnSearchBarClickListener == null) {
+            throw new IllegalStateException(
+                    OnSearchBarClickListener.class.getSimpleName() + " is not initializer."
+            );
         }
 
-        @Override
-        public int getCount() {
-            // Adding offset for label at position 0 (Bug 1172071)
-            return searchEngines.size() + 1;
-        }
-
-        @Override
-        public SearchEngine getItem(final int position) {
-            // Returning null for the label at position 0 (Bug 1172071)
-            return position == 0 ? null : searchEngines.get(position - 1);
-        }
-
-        @Override
-        public long getItemId(final int position) {
-            return position;
-        }
-
-        @Override
-        public int getItemViewType(int position) {
-            return position == 0 ? VIEW_TYPE_LABEL : VIEW_TYPE_SEARCH_ENGINE;
-        }
-
-        @Override
-        public int getViewTypeCount() {
-            return 2;
+        if (position == 0) {
+            return;
         }
 
-        @Override
-        public View getView(final int position, final View convertView, final ViewGroup parent) {
-            if (position == 0) {
-                return getLabelView(convertView, parent);
-            } else {
-                return getSearchEngineView(position, convertView, parent);
-            }
-        }
+        final SearchEngine searchEngine = mAdapter.getItem(position);
+        mOnSearchBarClickListener.onSearchBarClickListener(searchEngine);
+    }
 
-        private View getLabelView(View view, final ViewGroup parent) {
-            if (view == null) {
-                view = LayoutInflater.from(getContext()).inflate(R.layout.search_engine_bar_label, parent, false);
-            }
-
-            final Drawable icon =
-                    DrawableUtil.tintDrawable(parent.getContext(), R.drawable.search_icon_active, R.color.disabled_grey);
-
-            final ImageView iconView = (ImageView) view.findViewById(R.id.search_engine_label);
-            iconView.setImageDrawable(icon);
-            iconView.setScaleType(ImageView.ScaleType.FIT_XY);
+    @Override
+    public void onLongClick(View view, int position) {
+        // do nothing
+    }
 
-            return view;
-        }
-
-        private View getSearchEngineView(final int position, View view, final ViewGroup parent) {
-            if (view == null) {
-                view = LayoutInflater.from(getContext()).inflate(R.layout.search_engine_bar_item, parent, false);
-            }
-
-            LayoutParams params = (LayoutParams) view.getLayoutParams();
-            params.width = iconContainerWidth;
-            view.setLayoutParams(params);
-
-            final ImageView faviconView = (ImageView) view.findViewById(R.id.search_engine_icon);
-            final SearchEngine searchEngine = getItem(position);
-            faviconView.setImageBitmap(searchEngine.getIcon());
-
-            final String desc = getResources().getString(R.string.search_bar_item_desc, searchEngine.getEngineIdentifier());
-            view.setContentDescription(desc);
-
-            return view;
-        }
+    /**
+     * We manually add the override for getAdapter because we see this method getting stripped
+     * out during compile time by aggressive proguard rules.
+     */
+    @RobocopTarget
+    @Override
+    public SearchEngineAdapter getAdapter() {
+        return mAdapter;
     }
 }
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -347,16 +347,17 @@ gbjar.sources += [
     'home/RecyclerViewItemClickListener.java',
     'home/RemoteTabsBaseFragment.java',
     'home/RemoteTabsExpandableListFragment.java',
     'home/RemoteTabsExpandableListState.java',
     'home/RemoteTabsPanel.java',
     'home/RemoteTabsSplitPlaneFragment.java',
     'home/RemoteTabsStaticFragment.java',
     'home/SearchEngine.java',
+    'home/SearchEngineAdapter.java',
     'home/SearchEngineBar.java',
     'home/SearchEngineRow.java',
     'home/SearchLoader.java',
     'home/SimpleCursorLoader.java',
     'home/SpacingDecoration.java',
     'home/TabMenuStrip.java',
     'home/TabMenuStripLayout.java',
     'home/TopSitesGridItemView.java',
--- a/mobile/android/base/resources/layout/search_engine_bar_item.xml
+++ b/mobile/android/base/resources/layout/search_engine_bar_item.xml
@@ -12,16 +12,17 @@
 
      The actual width of the FrameLayout is calculated at runtime by the
      SearchEngineBar class to spread the icons across the device's width. -->
 <FrameLayout
     xmlns:android="http://schemas.android.com/apk/res/android"
     android:id="@+id/search_engine_icon_container"
     android:layout_width="72dp"
     android:layout_height="match_parent"
+    android:clickable="true"
     android:background="@color/pressed_about_page_header_grey">
 
     <!-- Width & height are set to make the Favicons as sharp as possible
          based on asset size. -->
     <ImageView
         android:id="@+id/search_engine_icon"
         android:layout_width="24dp"
         android:layout_height="24dp"
--- a/mobile/android/base/resources/layout/search_engine_bar_label.xml
+++ b/mobile/android/base/resources/layout/search_engine_bar_label.xml
@@ -11,11 +11,11 @@
     android:layout_width="48dp"
     android:layout_height="match_parent">
 
     <ImageView
         android:id="@+id/search_engine_label"
         android:layout_width="16dp"
         android:layout_height="16dp"
         android:layout_gravity="center"
-        android:scaleType="fitCenter"/>
+        android:scaleType="fitXY"/>
 
 </FrameLayout>
--- a/mobile/android/tests/browser/robocop/AboutHomeTest.java
+++ b/mobile/android/tests/browser/robocop/AboutHomeTest.java
@@ -5,16 +5,17 @@
 package org.mozilla.gecko.tests;
 
 import java.util.ArrayList;
 
 import org.mozilla.gecko.Actions;
 import org.mozilla.gecko.home.HomePager;
 
 import android.support.v4.view.ViewPager;
+import android.support.v7.widget.RecyclerView;
 import android.text.TextUtils;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.ListAdapter;
 import android.widget.ListView;
 import android.widget.TabWidget;
 import android.widget.TextView;
 
@@ -203,17 +204,17 @@ abstract class AboutHomeTest extends Pix
     }
 
     private void clickAboutHomeTab(AboutHomeTabs tab) {
         mSolo.clickOnText(tab.toString().replace("_", " "));
     }
 
     /**
      * Swipes to an about:home tab.
-     * @param int swipeVector Value and direction to swipe (go left for negative, right for positive).
+     * @param swipeVector swipeVector Value and direction to swipe (go left for negative, right for positive).
      */
     private void swipeAboutHome(int swipeVector) {
         // Increase swipe width, which will especially impact tablets.
         int swipeWidth = mDriver.getGeckoWidth() - 1;
         int swipeHeight = mDriver.getGeckoHeight() / 2;
 
         if (swipeVector >= 0) {
             // Emulate swipe motion from right to left.
@@ -227,18 +228,16 @@ abstract class AboutHomeTest extends Pix
                 mActions.drag(0, swipeWidth, swipeHeight, swipeHeight);
                 mSolo.sleep(100);
             }
         }
     }
 
     /**
      * This method can be used to open the different tabs of about:home.
-     *
-     * @param AboutHomeTabs enum item
      */
     protected void openAboutHomeTab(AboutHomeTabs tab) {
         focusUrlBar();
         ViewPager pager = mSolo.getView(ViewPager.class, 0);
         final int currentTabIndex = pager.getCurrentItem();
         int tabOffset;
 
         // Handle tablets by just clicking the visible tab title.
--- a/mobile/android/tests/browser/robocop/testAddSearchEngine.java
+++ b/mobile/android/tests/browser/robocop/testAddSearchEngine.java
@@ -152,17 +152,17 @@ public class testAddSearchEngine extends
                 }
 
                 SearchEngineBar searchEngineBar = (SearchEngineBar) mSolo.getView(R.id.search_engine_bar);
                 if (searchEngineBar == null || searchEngineBar.getAdapter() == null) {
                     return false;
                 }
 
                 final int actualCount = searchResultList.getAdapter().getCount()
-                        + searchEngineBar.getAdapter().getCount()
+                        + searchEngineBar.getAdapter().getItemCount()
                         - 1; // Subtract one for the search engine bar label (Bug 1172071)
 
                 return (actualCount == expectedCount);
             }
         }, MAX_WAIT_TEST_MS);
 
         // Exit about:home
         mSolo.goBack();