Fix live tab showing wrong bitmap and add tests for View/ViewHolder

This changes the logic for the thumbnail to properly reset while reusing
old ViewHolders in the RecyclerView. It also adds tests using
DummyUIActivity for the View/VIewBinder logic in the TabList component.

BUG=935714

Change-Id: Iabf4defc977bfbc38c98f02ee9a9fcca999bf190
Reviewed-on: https://chromium-review.googlesource.com/c/1484735Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636586}

chrome/android/java/res/drawable/selected_tab_background.xml

@@ -5,7 +5,7 @@
 
 <shape xmlns:android="http://schemas.android.com/apk/res/android">
     <stroke
-        android:width="3dp"
-        android:color="@color/modern_blue_600" />
+        android:width="2dp"
+        android:color="@color/modern_grey_700" />
     <corners android:radius="@dimen/default_card_corner_radius" />
 </shape>
\ No newline at end of file

chrome/android/java/res/layout/tab_grid_card_item.xml

@@ -47,7 +47,7 @@
                 style="@style/HorizontalDivider"
                 android:layout_below="@id/tab_title"/>
     </RelativeLayout>
-    <ImageView
+    <org.chromium.ui.widget.ChromeImageView
         android:id="@+id/close_button"
         android:layout_width="48dp"
         android:layout_height="48dp"

chrome/android/java/res/values/dimens.xml

@@ -594,7 +594,7 @@
 
     <!-- Tab List dimensions -->
     <dimen name="tab_grid_favicon_size">32dp</dimen>
-    <dimen name="tab_list_selected_inset">5dp</dimen>
+    <dimen name="tab_list_selected_inset">6dp</dimen>
     <dimen name="bottom_tab_grid_toolbar_icon_size">48dp</dimen>
 
 </resources>

chrome/android/java/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabGridViewBinder.java

@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser.tasks.tab_list_ui;
 
 import android.content.res.Resources;
+import android.graphics.Bitmap;
 import android.graphics.drawable.Drawable;
 import android.graphics.drawable.InsetDrawable;
 import android.support.annotation.Nullable;
@@ -12,6 +13,7 @@ import android.support.v4.content.res.ResourcesCompat;
 import android.support.v7.widget.RecyclerView.ViewHolder;
 import android.widget.FrameLayout;
 
+import org.chromium.base.Callback;
 import org.chromium.chrome.R;
 import org.chromium.ui.modelutil.PropertyKey;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -58,7 +60,16 @@ class TabGridViewBinder {
         } else if (TabProperties.FAVICON == propertyKey) {
             holder.favicon.setImageBitmap(item.get(TabProperties.FAVICON));
         } else if (TabProperties.THUMBNAIL_FETCHER == propertyKey) {
-            item.get(TabProperties.THUMBNAIL_FETCHER).fetch(holder.thumbnail::setImageBitmap);
+            TabListMediator.ThumbnailFetcher fetcher = item.get(TabProperties.THUMBNAIL_FETCHER);
+            if (fetcher == null) return;
+            Callback<Bitmap> callback = result -> {
+                if (result == null) {
+                    holder.thumbnail.setImageResource(0);
+                } else {
+                    holder.thumbnail.setImageBitmap(result);
+                }
+            };
+            fetcher.fetch(callback);
         } else if (TabProperties.TAB_ID == propertyKey) {
             holder.setTabId(item.get(TabProperties.TAB_ID));
         }

chrome/android/java/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabGridViewHolder.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.tasks.tab_list_ui;
 
+import android.support.v4.graphics.drawable.DrawableCompat;
 import android.support.v7.widget.RecyclerView;
 import android.view.LayoutInflater;
 import android.view.View;
@@ -11,6 +12,7 @@ import android.view.ViewGroup;
 import android.widget.ImageView;
 import android.widget.TextView;
 
+import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.chrome.R;
 
 /**
@@ -30,6 +32,8 @@ class TabGridViewHolder extends RecyclerView.ViewHolder {
         this.title = itemView.findViewById(R.id.tab_title);
         this.favicon = itemView.findViewById(R.id.tab_favicon);
         this.closeButton = itemView.findViewById(R.id.close_button);
+        DrawableCompat.setTint(this.closeButton.getDrawable(),
+                ApiCompatibilityUtils.getColor(itemView.getResources(), R.color.light_icon_color));
     }
 
     public static TabGridViewHolder create(ViewGroup parent, int itemViewType) {

chrome/android/java/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabListMediator.java

@@ -44,7 +44,7 @@ class TabListMediator {
      * The object to set to TabProperties.THUMBNAIL_FETCHER for the TabGridViewBinder to obtain
      * the thumbnail asynchronously.
      */
-    class ThumbnailFetcher {
+    static class ThumbnailFetcher {
         private ThumbnailProvider mThumbnailProvider;
         private Tab mTab;
 

chrome/android/java/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabStripViewHolder.java

@@ -4,11 +4,11 @@
 
 package org.chromium.chrome.browser.tasks.tab_list_ui;
 
-import android.support.v7.widget.AppCompatImageButton;
 import android.support.v7.widget.RecyclerView;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
+import android.widget.ImageButton;
 
 import org.chromium.chrome.R;
 
@@ -17,7 +17,7 @@ import org.chromium.chrome.R;
  */
 class TabStripViewHolder extends RecyclerView.ViewHolder {
     public int mTabId;
-    public final AppCompatImageButton button;
+    public final ImageButton button;
 
     public TabStripViewHolder(View itemView) {
         super(itemView);

chrome/android/java_sources.gni

@@ -2326,6 +2326,8 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/tabmodel/TestTabModelDirectory.java",
   "javatests/src/org/chromium/chrome/browser/tabmodel/UndoTabModelTest.java",
   "javatests/src/org/chromium/chrome/browser/tabmodel/document/MockDocumentTabModel.java",
+  "javatests/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabListViewHolderTest.java",
+  "javatests/src/org/chromium/chrome/browser/tasks/tab_list_ui/TestRecyclerViewSimpleViewBinder.java",
   "javatests/src/org/chromium/chrome/browser/test/ChromeBrowserTestRule.java",
   "javatests/src/org/chromium/chrome/browser/test/ClearAppDataTestRule.java",
   "javatests/src/org/chromium/chrome/browser/test/CommandLineInitRule.java",

chrome/android/javatests/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabListViewHolderTest.java

+// Copyright 2019 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.tasks.tab_list_ui;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import android.graphics.Bitmap;
+import android.graphics.drawable.BitmapDrawable;
+import android.graphics.drawable.ColorDrawable;
+import android.os.Build;
+import android.support.test.annotation.UiThreadTest;
+import android.support.test.filters.MediumTest;
+import android.view.ViewGroup;
+import android.widget.FrameLayout;
+import android.widget.LinearLayout;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.Callback;
+import org.chromium.base.ThreadUtils;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.ui.DummyUiActivity;
+import org.chromium.chrome.test.ui.DummyUiActivityTestCase;
+import org.chromium.ui.modelutil.PropertyModel;
+import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * Tests for the {@link android.support.v7.widget.RecyclerView.ViewHolder} classes for {@link
+ * TabListCoordinator}.
+ */
+@RunWith(ChromeJUnit4ClassRunner.class)
+public class TabListViewHolderTest extends DummyUiActivityTestCase {
+    private TabGridViewHolder mTabGridViewHolder;
+    private PropertyModel mGridModel;
+    private PropertyModelChangeProcessor mGridMCP;
+
+    private TabStripViewHolder mTabStripViewHolder;
+    private PropertyModel mStripModel;
+    private PropertyModelChangeProcessor mStripMCP;
+
+    private TabListMediator.ThumbnailFetcher mMockThumbnailProvider =
+            new TabListMediator.ThumbnailFetcher(new TabListMediator.ThumbnailProvider() {
+                @Override
+                public void getTabThumbnailWithCallback(Tab tab, Callback<Bitmap> callback) {
+                    Bitmap bitmap = mShouldReturnBitmap
+                            ? Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888)
+                            : null;
+                    callback.onResult(bitmap);
+                }
+            }, null);
+
+    private TabListMediator.TabActionListener mMockCloseListener =
+            new TabListMediator.TabActionListener() {
+                @Override
+                public void run(int tabId) {
+                    mCloseClicked.set(true);
+                }
+            };
+    private AtomicBoolean mCloseClicked = new AtomicBoolean();
+
+    private TabListMediator.TabActionListener mMockSelectedListener =
+            new TabListMediator.TabActionListener() {
+                @Override
+                public void run(int tabId) {
+                    mSelectClicked.set(true);
+                }
+            };
+    private AtomicBoolean mSelectClicked = new AtomicBoolean();
+    private boolean mShouldReturnBitmap;
+
+    @BeforeClass
+    public static void setUpBeforeActivityLaunched() {
+        DummyUiActivity.setTestTheme(R.style.Theme_Chromium_Activity_Fullscreen);
+    }
+
+    @Override
+    public void setUpTest() throws Exception {
+        super.setUpTest();
+        ViewGroup view = new LinearLayout(getActivity());
+        FrameLayout.LayoutParams params = new FrameLayout.LayoutParams(
+                ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT);
+
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            getActivity().setContentView(view, params);
+
+            mTabGridViewHolder = TabGridViewHolder.create(view, 0);
+            mTabStripViewHolder = TabStripViewHolder.create(view, 0);
+
+            view.addView(mTabGridViewHolder.itemView);
+            view.addView(mTabStripViewHolder.itemView);
+        });
+
+        mGridModel = new PropertyModel.Builder(TabProperties.ALL_KEYS_TAB_GRID)
+                             .with(TabProperties.TAB_SELECTED_LISTENER, mMockSelectedListener)
+                             .with(TabProperties.TAB_CLOSED_LISTENER, mMockCloseListener)
+                             .build();
+        mStripModel = new PropertyModel.Builder(TabProperties.ALL_KEYS_TAB_STRIP)
+                              .with(TabProperties.TAB_SELECTED_LISTENER, mMockSelectedListener)
+                              .with(TabProperties.TAB_CLOSED_LISTENER, mMockCloseListener)
+                              .build();
+
+        mGridMCP = PropertyModelChangeProcessor.create(mGridModel, mTabGridViewHolder,
+                new TestRecyclerViewSimpleViewBinder<>(TabGridViewBinder::onBindViewHolder));
+        mStripMCP = PropertyModelChangeProcessor.create(mStripModel, mTabStripViewHolder,
+                new TestRecyclerViewSimpleViewBinder<>(TabStripViewBinder::onBindViewHolder));
+    }
+
+    @Test
+    @MediumTest
+    @UiThreadTest
+    public void testSelected() throws Exception {
+        mGridModel.set(TabProperties.IS_SELECTED, true);
+        Assert.assertTrue(((FrameLayout) (mTabGridViewHolder.itemView)).getForeground() != null);
+        mGridModel.set(TabProperties.IS_SELECTED, false);
+        Assert.assertFalse(((FrameLayout) (mTabGridViewHolder.itemView)).getForeground() != null);
+
+        mStripModel.set(TabProperties.IS_SELECTED, true);
+        Assert.assertTrue(((FrameLayout) (mTabStripViewHolder.itemView)).getForeground() != null);
+        mStripModel.set(TabProperties.IS_SELECTED, false);
+        Assert.assertFalse(((FrameLayout) (mTabStripViewHolder.itemView)).getForeground() != null);
+    }
+
+    @Test
+    @MediumTest
+    @UiThreadTest
+    public void testTitle() throws Exception {
+        final String title = "Surf the cool webz";
+        mGridModel.set(TabProperties.TITLE, title);
+        Assert.assertEquals(mTabGridViewHolder.title.getText(), title);
+    }
+
+    @Test
+    @MediumTest
+    @UiThreadTest
+    public void testThumbnail() throws Exception {
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        // This should have set the image resource id to 0 and reset it.
+        if (Build.VERSION.SDK_INT > Build.VERSION_CODES.KITKAT) {
+            Assert.assertNull(mTabGridViewHolder.thumbnail.getDrawable());
+        } else {
+            assertThat(mTabGridViewHolder.thumbnail.getDrawable(), instanceOf(ColorDrawable.class));
+        }
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, null);
+
+        mShouldReturnBitmap = true;
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        assertThat(mTabGridViewHolder.thumbnail.getDrawable(), instanceOf(BitmapDrawable.class));
+    }
+
+    @Test
+    @MediumTest
+    @UiThreadTest
+    public void testClickToSelect() throws Exception {
+        mTabGridViewHolder.itemView.performClick();
+        Assert.assertTrue(mSelectClicked.get());
+        mSelectClicked.set(false);
+
+        mStripModel.set(TabProperties.IS_SELECTED, false);
+        mTabStripViewHolder.button.performClick();
+        Assert.assertTrue(mSelectClicked.get());
+        mSelectClicked.set(false);
+
+        mStripModel.set(TabProperties.IS_SELECTED, true);
+        mTabStripViewHolder.button.performClick();
+        Assert.assertFalse(mSelectClicked.get());
+    }
+
+    @Test
+    @MediumTest
+    @UiThreadTest
+    public void testClickToClose() throws Exception {
+        mTabGridViewHolder.closeButton.performClick();
+        Assert.assertTrue(mCloseClicked.get());
+        mCloseClicked.set(false);
+
+        mStripModel.set(TabProperties.IS_SELECTED, true);
+        mTabStripViewHolder.button.performClick();
+        Assert.assertTrue(mCloseClicked.get());
+        mCloseClicked.set(false);
+
+        mStripModel.set(TabProperties.IS_SELECTED, false);
+        mTabStripViewHolder.button.performClick();
+        Assert.assertFalse(mCloseClicked.get());
+    }
+
+    @Override
+    public void tearDownTest() throws Exception {
+        mStripMCP.destroy();
+        mGridMCP.destroy();
+        super.tearDownTest();
+    }
+}

chrome/android/javatests/src/org/chromium/chrome/browser/tasks/tab_list_ui/TestRecyclerViewSimpleViewBinder.java

+// Copyright 2019 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.tasks.tab_list_ui;
+
+import android.support.v7.widget.RecyclerView;
+
+import org.chromium.ui.modelutil.PropertyKey;
+import org.chromium.ui.modelutil.PropertyModel;
+import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
+import org.chromium.ui.modelutil.SimpleRecyclerViewMcpBase;
+
+/**
+ * Test utility class to allow using {@link
+ * org.chromium.ui.modelutil.SimpleRecyclerViewMcpBase.ViewBinder} classes to be used in conjunction
+ * with a {@link PropertyModelChangeProcessor} so that individual items in the RecyclerView can be
+ * tested independently.
+ * @param <VH> The ViewHolder class to be used.
+ */
+public class TestRecyclerViewSimpleViewBinder<VH extends RecyclerView.ViewHolder>
+        implements PropertyModelChangeProcessor.ViewBinder<PropertyModel, VH, PropertyKey> {
+    SimpleRecyclerViewMcpBase.ViewBinder<PropertyModel, VH, PropertyKey> mInternalViewBinder;
+
+    /**
+     * Main constructor
+     * @param viewBinder The {@link org.chromium.ui.modelutil.SimpleRecyclerViewMcpBase.ViewBinder}
+     *         to wrap around.
+     */
+    TestRecyclerViewSimpleViewBinder(
+            SimpleRecyclerViewMcpBase.ViewBinder<PropertyModel, VH, PropertyKey> viewBinder) {
+        mInternalViewBinder = viewBinder;
+    }
+
+    @Override
+    public void bind(PropertyModel model, VH viewHolder, PropertyKey propertyKey) {
+        mInternalViewBinder.onBindViewHolder(viewHolder, model, propertyKey);
+    }
+}
\ No newline at end of file