[NTP::Downloads] Show thumbnails for Download suggestions.

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
index aeb54cc9520624a47da71ff387df5f4d48746d72..512c752cd92831e67719afd6fb63ae14529fdc85 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser.ntp.snippets;
 
 import android.annotation.SuppressLint;
+import android.content.res.ColorStateList;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.drawable.BitmapDrawable;
@@ -14,6 +15,7 @@ import android.media.ThumbnailUtils;
 import android.os.StrictMode;
 import android.os.SystemClock;
 import android.support.v4.text.BidiFormatter;
+import android.text.TextUtils;
 import android.text.format.DateUtils;
 import android.view.View;
 import android.view.View.MeasureSpec;
@@ -25,6 +27,10 @@ import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.Callback;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.download.ui.DownloadFilter;
+import org.chromium.chrome.browser.download.ui.DownloadItemView;
+import org.chromium.chrome.browser.download.ui.ThumbnailProvider;
+import org.chromium.chrome.browser.download.ui.ThumbnailProviderImpl;
 import org.chromium.chrome.browser.favicon.FaviconHelper.FaviconImageCallback;
 import org.chromium.chrome.browser.favicon.FaviconHelper.IconAvailabilityCallback;
 import org.chromium.chrome.browser.ntp.ContextMenuManager;
@@ -36,6 +42,7 @@ import org.chromium.chrome.browser.ntp.cards.NewTabPageRecyclerView;
 import org.chromium.chrome.browser.ntp.cards.NewTabPageViewHolder;
 import org.chromium.chrome.browser.ntp.cards.SuggestionsCategoryInfo;
 import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate;
+import org.chromium.chrome.browser.widget.TintedImageView;
 import org.chromium.chrome.browser.widget.displaystyle.DisplayStyleObserver;
 import org.chromium.chrome.browser.widget.displaystyle.DisplayStyleObserverAdapter;
 import org.chromium.chrome.browser.widget.displaystyle.HorizontalDisplayStyle;
@@ -50,7 +57,8 @@ import java.util.concurrent.TimeUnit;
 /**
  * A class that represents the view for a single card snippet.
  */
-public class SnippetArticleViewHolder extends CardViewHolder implements ImpressionTracker.Listener {
+public class SnippetArticleViewHolder extends CardViewHolder
+        implements ImpressionTracker.Listener, ThumbnailProvider.ThumbnailRequest {
     /**
      * A single instance of {@link RefreshOfflineBadgeVisibilityCallback} that can be reused as it
      * has no state.
@@ -68,7 +76,7 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
     private final TextView mHeadlineTextView;
     private final TextView mPublisherTextView;
     private final TextView mArticleSnippetTextView;
-    private final ImageView mThumbnailView;
+    private final TintedImageView mThumbnailView;
     private final ImageView mOfflineBadge;
     private final View mPublisherBar;
 
@@ -80,6 +88,10 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
     private final boolean mUseFaviconService;
     private final UiConfig mUiConfig;
 
+    private final ColorStateList mWhiteTint;
+    private final int mIconBackgroundColor;
+    private final ThumbnailProvider mThumbnailProvider;
+
     /**
      * Constructs a {@link SnippetArticleViewHolder} item used to display snippets.
      *  @param parent The NewTabPageRecyclerView that is going to contain the newly created view.
@@ -93,7 +105,7 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
         super(R.layout.new_tab_page_snippets_card, parent, uiConfig, contextMenuManager);
 
         mUiDelegate = uiDelegate;
-        mThumbnailView = (ImageView) itemView.findViewById(R.id.article_thumbnail);
+        mThumbnailView = (TintedImageView) itemView.findViewById(R.id.article_thumbnail);
         mHeadlineTextView = (TextView) itemView.findViewById(R.id.article_headline);
         mPublisherTextView = (TextView) itemView.findViewById(R.id.article_publisher);
         mArticleSnippetTextView = (TextView) itemView.findViewById(R.id.article_snippet);
@@ -111,6 +123,11 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
         });
 
         mUseFaviconService = CardsVariationParameters.isFaviconServiceEnabled();
+
+        mIconBackgroundColor = DownloadItemView.getIconBackgroundColor(parent.getContext());
+        mWhiteTint = DownloadItemView.getWhiteTint(parent.getContext());
+        mThumbnailProvider = new ThumbnailProviderImpl(
+                Math.min(mThumbnailView.getMaxWidth(), mThumbnailView.getMaxHeight()));
     }
 
     @Override
@@ -237,6 +254,46 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
                 BidiFormatter.getInstance().unicodeWrap(article.mPublisher), relativeTimeSpan);
     }
 
+    @Override
+    public String getFilePath() {
+        return mArticle == null ? null : mArticle.getAssetDownloadFile().getAbsolutePath();
+    }
+
+    @Override
+    public void onThumbnailRetrieved(String filePath, Bitmap thumbnail) {

[dgn, 2017/02/14 10:54:28]

nit: For the sake of encapsulation and smaller public APIs, I'd prefer the
ThumbnailRequest implementation to be an inner class that we instantiate only
when we need, but I might be overdoing it. Michael wdyt?

[vitaliii, 2017/02/14 16:09:30]

Done.

+        if (TextUtils.equals(getFilePath(), filePath) && thumbnail != null
+                && thumbnail.getWidth() != 0 && thumbnail.getHeight() != 0) {
+            mArticle.setThumbnailBitmap(thumbnail);
+            mThumbnailView.setBackground(null);
+            mThumbnailView.setImageBitmap(mArticle.getThumbnailBitmap());
+            mThumbnailView.setTint(null);
+        }
+    }
+
+    private void setDownloadThumbnail() {

[dgn, 2017/02/14 10:54:28]

 nit: private methods after public methods but before inner classes

[vitaliii, 2017/02/14 16:09:30]

Done.

+        if (mArticle.mIsAssetDownload) {
+            int fileType = DownloadFilter.fromMimeType(mArticle.getAssetDownloadMimeType());
+            mThumbnailView.setBackgroundColor(mIconBackgroundColor);
+            mThumbnailView.setImageResource(DownloadItemView.getIconResId(fileType));
+            mThumbnailView.setTint(mWhiteTint);
+
+            if (fileType == DownloadFilter.FILTER_IMAGE) {
+                mThumbnailProvider.cancelRetrieval(this);
+                mArticle.setThumbnailBitmap(mThumbnailProvider.getThumbnail(this));
+                if (mArticle.getThumbnailBitmap() != null) {
+                    mThumbnailView.setBackground(null);

[dgn, 2017/02/14 10:54:28]

why do this after setting the color/icon/tint a few lines above?

I think some utility methods like setThumbnailFromFileType(int) and
setThumbnailFromBitmap(Bitmap (or Drawable, whatever it is)) would bring some
clarity here.

Also, prefer early returns to nested ifs

[vitaliii, 2017/02/14 16:09:30]

Because color/icon/tint are not needed when we show bitmap (and it is done in
similar fashion in DownloadItemView).
Done.

+                    mThumbnailView.setImageBitmap(mArticle.getThumbnailBitmap());
+                    mThumbnailView.setTint(null);
+                }
+            }
+        } else {
+            mThumbnailView.setBackgroundColor(mIconBackgroundColor);
+            mThumbnailView.setImageResource(
+                    DownloadItemView.getIconResId(DownloadFilter.FILTER_PAGE));
+            mThumbnailView.setTint(mWhiteTint);
+        }
+    }
+
     public void onBindViewHolder(SnippetArticle article, SuggestionsCategoryInfo categoryInfo) {
         super.onBindViewHolder();
 
@@ -264,9 +321,16 @@ public class SnippetArticleViewHolder extends CardViewHolder implements Impressi
             if (mArticle.getThumbnailBitmap() != null) {

[dgn, 2017/02/14 10:54:28]

I guess the entire thumbnail handling from here can be moved to a neat method

[vitaliii, 2017/02/14 16:09:30]

Done.

                 mThumbnailView.setImageBitmap(mArticle.getThumbnailBitmap());
             } else {
-                mThumbnailView.setImageResource(R.drawable.ic_snippet_thumbnail_placeholder);
-                mImageCallback = new FetchImageCallback(this, mArticle);
-                mUiDelegate.getSuggestionsSource().fetchSuggestionImage(mArticle, mImageCallback);
+                if (mArticle.isDownload()) {
+                    setDownloadThumbnail();
+                } else {
+                    mThumbnailView.setBackground(null);
+                    mThumbnailView.setImageResource(R.drawable.ic_snippet_thumbnail_placeholder);
+                    mThumbnailView.setTint(null);
+                    mImageCallback = new FetchImageCallback(this, mArticle);
+                    mUiDelegate.getSuggestionsSource().fetchSuggestionImage(
+                            mArticle, mImageCallback);
+                }
             }
         }