Added a UI for the Interests Prototype.

chrome/android/java/src/org/chromium/chrome/browser/ntp/InterestsItemView.java

+// Copyright 2015 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.ntp;
+
+import android.content.Context;
+import android.content.res.Resources;
+import android.content.res.TypedArray;
+import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
+import android.graphics.Color;
+import android.graphics.Rect;
+import android.graphics.drawable.BitmapDrawable;
+import android.graphics.drawable.Drawable;
+import android.media.ThumbnailUtils;
+import android.os.AsyncTask;
+import android.support.v4.graphics.drawable.RoundedBitmapDrawable;
+import android.support.v4.graphics.drawable.RoundedBitmapDrawableFactory;
+import android.support.v7.widget.AppCompatTextView;
+import android.text.TextUtils;
+import android.util.LruCache;
+import android.util.TypedValue;
+import android.view.Gravity;
+import android.view.View;
+import android.view.View.OnClickListener;
+
+import org.chromium.base.Log;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ntp.InterestsPage.InterestsClickListener;
+import org.chromium.chrome.browser.ntp.InterestsService.Interest;
+import org.chromium.chrome.browser.widget.RoundedIconGenerator;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * Displays the interest name along with an image of it. This item can be clicked.
+ */
+class InterestsItemView extends AppCompatTextView implements OnClickListener {
+
+    private static final String TAG = "InterestsItemView";
+
+    /**
+     * Drawing-related values that can be shared between instances of InterestsItemView.
+     */
+    static final class DrawingData {

[newt (away), 2015/12/12 00:30:30]

make this private

[PEConn, 2015/12/14 17:05:15]

The DrawingData instance is created and held by the InterestsView.

+
+        private final int mPadding;
+        private final int mMinHeight;
+        private final int mImageSize;
+        private final int mTextSize;
+        private final int mImageTextSize;
+
+        /**
+         * Initialize shared values used for drawing the image.
+         *
+         * @param context The view context in which the InterestsItemView will be drawn.
+         */
+        DrawingData(Context context) {
+            Resources res = context.getResources();
+            mPadding = res.getDimensionPixelOffset(R.dimen.ntp_list_item_padding);
+            mMinHeight = res.getDimensionPixelSize(R.dimen.ntp_interest_item_min_height);
+            mTextSize = res.getDimensionPixelSize(R.dimen.ntp_interest_item_text_size);
+            mImageSize = res.getDimensionPixelSize(R.dimen.ntp_interest_item_image_size);
+            mImageTextSize = res.getDimensionPixelSize(R.dimen.ntp_interest_item_image_text_size);
+        }
+    }
+
+    private Interest mInterest;
+
+    private final Context mContext;
+    private final DrawingData mDrawingData;
+    private final LruCache<String, ImagePlaceholder> mImageCache;
+    private final InterestsClickListener mListener;
+    private final RoundedIconGenerator mIconGenerator;
+
+    /**
+     * @param context The view context in which this item will be shown.
+     * @param interest The interest to display.
+     * @param listener Callback object for when a view is pressed.
+     * @param imageCache A cache to store downloaded images.
+     * @param drawingData Information about the view size.
+     */
+    InterestsItemView(Context context, Interest interest, InterestsClickListener listener,
+            LruCache<String, ImagePlaceholder> imageCache, DrawingData drawingData) {
+        super(context);
+
+        mContext = context;
+        mListener = listener;
+        mImageCache = imageCache;
+        mDrawingData = drawingData;
+
+        setTextSize(TypedValue.COMPLEX_UNIT_PX, mDrawingData.mTextSize);
+        setMinimumHeight(mDrawingData.mMinHeight);
+        setGravity(Gravity.CENTER);
+        setTextAlignment(View.TEXT_ALIGNMENT_CENTER);
+
+        mIconGenerator = new RoundedIconGenerator(

[newt (away), 2015/12/12 00:30:30]

It'd be worth reusing the RoundedIconGenerator (e.g. by moving it into
DrawingData) since it creates a Paint object, which is fairly expensive.

[PEConn, 2015/12/14 17:05:15]

Done.

+                mDrawingData.mImageSize,
+                mDrawingData.mImageSize,
+                mDrawingData.mImageSize / 2,
+                Color.GRAY,
+                mDrawingData.mImageTextSize);
+
+        setOnClickListener(this);
+
+        reset(interest);
+    }
+
+    /**
+     * Resets the view contents so that it can be reused in the grid view.
+     *
+     * @param interest The interest to display.
+     */
+    public void reset(Interest interest) {
+        // Reset Drawable state so ripples don't continue when the View is reused.
+        jumpDrawablesToCurrentState();
+
+        // Exit early if this View is already displaying the Interest given.
+        if (mInterest != null
+                && TextUtils.equals(interest.getName(), mInterest.getName())

[newt (away), 2015/12/12 00:30:31]

Seems like you should add an equals() method interest and use it here, or just
use == if you only care about identity.

[PEConn, 2015/12/14 17:05:15]

An interest contains a name, an image url and a relevance score. In this case
only the name and the image url matter to the ItemView - there's no need to
update if two different Interests share an image url and a name and I don't feel
an equals() method would be appropriate here since it ignores the relevance.

[newt (away), 2015/12/14 21:18:39]

Gotcha. Thanks for explaining.

+                && TextUtils.equals(interest.getImageUrl(), mInterest.getImageUrl())) {
+            mInterest = interest;
+            return;
+        }
+
+        mInterest = interest;
+
+        setText(mInterest.getName());
+
+        ImagePlaceholder placeholder = mImageCache.get(mInterest.getImageUrl());
+        if (placeholder == null) {
+            // Create a new placeholder, add it to the cache and set it downloading.
+            placeholder = new ImagePlaceholder();
+            mImageCache.put(mInterest.getImageUrl(), placeholder);
+            new ImageDownloadTask(mInterest.getImageUrl(), placeholder, getResources()).execute();
+        }
+
+        if (placeholder.isFilled()) {
+            setImage(placeholder.get());
+        } else {
+            // Add a callback to a subclass that will call setImage once the placeholder is filled.
+            placeholder.addListener(new ImageDownloadedCallback());
+
+            // Display a letter tile in the meantime.

[newt (away), 2015/12/12 00:30:31]

It might look a bit jumpy to generate default icons and then replace them with
real images.

Alternatively, we could just fade in the images when they're downloaded and not
show a placeholder (unless, perhaps, the download failed).

There's no need to change this behavior in this CL; just a thought for the
future.

[PEConn, 2015/12/14 17:05:15]

Acknowledged.

+            mIconGenerator.setBackgroundColor(getTileColor(mInterest.getName()));
+            setImage(new BitmapDrawable(mContext.getResources(),
+                    mIconGenerator.generateIconForText(mInterest.getName())));
+        }
+    }
+
+    /**
+     * @return The image URL for the interest.
+     */
+    public String getImageUrl() {
+        return mInterest.getImageUrl();
+    }
+
+    /**
+     * @return The name of the interest.
+     */
+    public String getName() {
+        return mInterest.getName();
+    }
+
+    private void setImage(Drawable image) {
+        image.setBounds(new Rect(0, 0, mDrawingData.mImageSize, mDrawingData.mImageSize));
+        setCompoundDrawables(null, image, null, null);
+    }
+
+    private int getTileColor(String str) {
+        // Rough copy of LetterTileDrawable.pickColor.
+        // TODO(peconn): Move this to a more general class.
+        TypedArray colors = mContext.getResources().obtainTypedArray(R.array.letter_tile_colors);
+        return colors.getColor(Math.abs(str.hashCode() % colors.length()), Color.DKGRAY);
+    }
+
+    @Override
+    public void onClick(View v) {
+        mListener.onInterestClicked(getName());
+    }
+
+    /*
+     * An AsyncTask that downloads an image, formats it then puts it in the given placeholder.
+     */
+    private static class ImageDownloadTask extends AsyncTask<Void, Void, Drawable> {

[newt (away), 2015/12/12 00:30:31]

This class doesn't logically belong inside InterestItemView. I'm OK with keeping
it here for now, though, since there's already a TODO to "Replace this with
something from the C++ Chrome stack".

[PEConn, 2015/12/14 17:05:15]

Acknowledged.

+
+        private final String mUrl;
+        private final ImagePlaceholder mImagePlaceholder;
+        private final Resources mResources;
+
+        public ImageDownloadTask(String url, ImagePlaceholder placeholder, Resources resources) {
+            mUrl = url;
+            mImagePlaceholder = placeholder;
+            mResources = resources;
+        }
+
+        @Override
+        protected Drawable doInBackground(Void... voids) {
+            // This is run on a background thread.
+            try {
+                // TODO(peconn): Replace this with something from the C++ Chrome stack.
+                URL imageUrl = new URL(mUrl);
+                InputStream in = imageUrl.openStream();
+
+                Bitmap raw = BitmapFactory.decodeStream(in);
+                int dimension = Math.min(raw.getHeight(), raw.getWidth());
+                RoundedBitmapDrawable img = RoundedBitmapDrawableFactory.create(mResources,
+                        ThumbnailUtils.extractThumbnail(raw, dimension, dimension));
+                img.setCircular(true);
+
+                return img;
+            } catch (IOException e) {
+                Log.e(TAG, "Error downloading image: " + e.toString());
+            }
+            return null;
+        }
+
+        @Override
+        protected void onPostExecute(Drawable image) {
+            // This is run on the main thread.
+            mImagePlaceholder.set(image, mUrl);
+        }
+    }
+
+    /*
+     * A callback class that will set it's parent's image.
+     */
+    private class ImageDownloadedCallback {
+        public void onCallback(Drawable image, String url) {

[newt (away), 2015/12/12 00:30:31]

I'd call this "onImageDownloaded()"

[PEConn, 2015/12/14 17:05:15]

Done.

+            if (image == null) {
+                return;
+            }
+            // If the Interest this View is displaying has changed while downloading, do not update
+            // the image.
+            if (url == mInterest.getImageUrl()) {
+                setImage(image);
+            }
+        }
+    }
+
+    /*
+     * A placeholder for an Image that allows listeners to subscribe to when it is set. It is
+     * like a listenable future that doesn't calculate the value itself. It can only be
+     * accessed on one thread. It can only be set once.
+     */
+    static class ImagePlaceholder {

[newt (away), 2015/12/12 00:30:30]

How about calling this "ImageHolder"?  "ImagePlaceholder" sounds it's a default
image (i.e. an image generated by RoundedIconGenerator).

[PEConn, 2015/12/14 17:05:15]

Done.

+        private final List<ImageDownloadedCallback> mListeners = new LinkedList<>();

[newt (away), 2015/12/12 00:30:31]

"Callback" or "Listener"? Pick one and use it consistently :)

Also, you can use ObserverList (in base/), though its fancy features (such as
removing a listener during iteration) may not be needed here.

[PEConn, 2015/12/14 17:05:15]

Done.

+        private Drawable mImage = null;

[newt (away), 2015/12/12 00:30:31]

no need for "= null" in a member variable declaration; that's the default in
java

[PEConn, 2015/12/14 17:05:15]

Done.

+        private String mUrl;
+
+        public void set(Drawable image, String url) {
+            assert mImage == null;
+            mImage = image;
+            mUrl = url;
+
+            for (ImageDownloadedCallback listener : mListeners) {

[newt (away), 2015/12/12 00:30:31]

Shouldn't you clear mListeners after calling them? Otherwise, you have the
potential for memory leaks: the ImageCache holds references to all the
ImagePlaceholders, which in turn hold references to every IterestsItemView
that's ever been created with that ImageCache.

[PEConn, 2015/12/14 17:05:15]

Done.

+                listener.onCallback(image, mUrl);
+            }
+        }
+
+        public void addListener(ImageDownloadedCallback listener) {
+            if (mImage == null) {
+                mListeners.add(listener);
+            } else {
+                listener.onCallback(mImage, mUrl);
+            }
+        }
+
+        public Boolean isFilled() {

[newt (away), 2015/12/12 00:30:31]

s/Boolean/boolean/   (Never use Boolean if you can avoid it.)

Though I'm not convinced you need this method. The caller could just use get().

[PEConn, 2015/12/14 17:05:15]

Done.

+            return mImage != null;
+        }
+
+        public Drawable get() {

[newt (away), 2015/12/12 00:30:30]

s/get/getImageDrawable/ ?

[PEConn, 2015/12/14 17:05:15]

Done.

+            return mImage;
+        }
+    }
+}