Show favicons in Site Settings lists.

chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreference.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreference.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreference.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreference.java
index e9a4364be758bc2b76dc54369975c38b6da119ec..397e42d99feb191d3b841ac40580e0bc23e63099 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreference.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreference.java
@@ -5,14 +5,25 @@
 package org.chromium.chrome.browser.preferences.website;
 
 import android.content.Context;
+import android.content.res.Resources;
+import android.graphics.Bitmap;
+import android.graphics.Color;
+import android.graphics.drawable.BitmapDrawable;
 import android.preference.Preference;
 import android.view.View;
+import android.view.ViewGroup.MarginLayoutParams;
 import android.widget.ImageView;
 import android.widget.TextView;
 
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.favicon.FaviconHelper;
+import org.chromium.chrome.browser.favicon.FaviconHelper.FaviconImageCallback;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.widget.RoundedIconGenerator;
 
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.text.DecimalFormat;
 import java.text.NumberFormat;
 
@@ -22,13 +33,25 @@ import java.text.NumberFormat;
  * access the site has.)
  */
 @SuppressFBWarnings("EQ_COMPARETO_USE_OBJECT_EQUALS")
-class WebsitePreference extends Preference {
+class WebsitePreference extends Preference implements FaviconImageCallback {
     private final Website mSite;
     private final String mCategoryFilter;
     private final WebsiteSettingsCategoryFilter mFilter;
 
     private static final int TEXT_SIZE_SP = 13;
 
+    // Loads the favicons asynchronously.
+    private FaviconHelper mFaviconHelper;
+
+    // The desired size of the favicon.
+    private int mDesiredSize = 0;

[newt (away), 2015/01/22 17:17:23]

how about: mDesiredFaviconSize or just mFaviconSize

[Finnur, 2015/01/23 14:59:47]

Obsolete. Was able to do away with this var.

+
+    // Metrics for the default favicon (when no other favicon is available).
+    private static final int FAVICON_CORNER_RADIUS_DP = 2;
+    private static final int FAVICON_TEXT_SIZE_SP = 10;
+    private static final int FAVICON_BACKGROUND_COLOR =

[newt (away), 2015/01/22 17:17:24]

It's fine to write colors in hex (0xff969696), and I personally find that easier
to read, since it matches what's in our colors.xml files.

[Finnur, 2015/01/23 14:59:47]

Done.

+            Color.rgb(150, 150, 150);
+
     WebsitePreference(Context context, Website site, String categoryFilter) {
         super(context);
         mSite = site;
@@ -36,12 +59,40 @@ class WebsitePreference extends Preference {
         mFilter = new WebsiteSettingsCategoryFilter();
         setWidgetLayoutResource(R.layout.website_features);
         refresh();
+
+        // Start the favicon fetching. Will respond in onFaviconAvailable.

[newt (away), 2015/01/22 17:17:23]

It would be nice to be smarter about favicon fetching here. If the user has 1000
websites with cookies (which is quite possible), this will incur a huge overhead
(at least 10MB of RAM) when opening the cookies or all sites pages.

One option: fetch favicons only once the preference is actually shown (e.g. when
onBindView() is called).

[Finnur, 2015/01/23 14:59:47]

Good suggestion. Done.

+        Resources resources = context.getResources();
+        int dimension =

[newt (away), 2015/01/22 17:17:23]

100 character limit. No need to wrap here.

[Finnur, 2015/01/23 14:59:47]

Done.

+                resources.getDimensionPixelSize(R.dimen.pref_list_favicon);
+        mDesiredSize =
+                (int) (dimension / resources.getDisplayMetrics().density);

[newt (away), 2015/01/22 17:17:24]

rather than dividing by the density, I'd store the desired size in DP as a
constant in this file, then multiple by the density to get dimension:

  private static final int FAVICON_SIZE_DP = 16;
  ...
  mFaviconSize = Math.round(FAVICON_SIZE_DP *
resources.getDisplayMetrics().density);

[Finnur, 2015/01/23 14:59:47]

Done.

+        mFaviconHelper = new FaviconHelper();
+        mFaviconHelper.getLocalFaviconImageForURL(
+                Profile.getLastUsedProfile(), faviconUrl(),
+                FaviconHelper.FAVICON | FaviconHelper.TOUCH_ICON
+                        | FaviconHelper.TOUCH_PRECOMPOSED_ICON,
+                dimension,
+                this);
     }
 
     public void putSiteIntoExtras(String key) {
         getExtras().putSerializable(key, mSite);
     }
 
+    @Override
+    public void onFaviconAvailable(Bitmap image, String iconUrl) {
+        if (image == null || image.getHeight() == 0) {

[newt (away), 2015/01/22 17:17:24]

does image.getHeight() == 0 happen in practice?

[Finnur, 2015/01/23 14:59:47]

No, I had some problems with icons not appearing at all at first, thinking I was
getting invalid icons. This code was testing, but forgot to delete. Thanks for
catching.

+            // Invalid favicon, produce a generic one.
+            RoundedIconGenerator faviconGenerator = new RoundedIconGenerator(
+                    getContext(), mDesiredSize, mDesiredSize,
+                    FAVICON_CORNER_RADIUS_DP, FAVICON_BACKGROUND_COLOR,
+                    FAVICON_TEXT_SIZE_SP);
+            image = faviconGenerator.generateIconForUrl(faviconUrl());

[newt (away), 2015/01/22 17:17:24]

Can you just use iconUrl here? Calling faviconUrl() again seems wasteful.

[Finnur, 2015/01/23 14:59:47]

No, we need to create a generic image because the icon couldn't be retrieved and
iconUrl is therefore blank.

+        }
+
+        setIcon(new BitmapDrawable(getContext().getResources(), image));
+    }
+
     /**
      * Return the Website this object is representing.
      */
@@ -49,6 +100,39 @@ class WebsitePreference extends Preference {
         return mSite;
     }
 
+    /**
+     * Returns the url of the site to fetch a favicon for.
+     */
+    private String faviconUrl() {
+        String url = mSite.getAddress().getOrigin();
+        if (url == null) {
+            url = mSite.getTitle();
+        }
+
+        // Both icon functions (the one that fetches the favicon and the one

[newt (away), 2015/01/22 17:17:23]

How about using android.net.Uri.Builder here? That seems at least better than
constructing URLs, parsing them, and catching parse exceptions. E.g.:

  Uri uri = Uri.parse(mSite.getAddress().getOrigin());
  if (uri.getPort() != null) {
    // Remove port.
    uri = uri.buildUpon().authority(uri.host()).build();
  }
  return uri.toString();

[Finnur, 2015/01/23 14:59:47]

Ooh, I like that.

+        // that creates a generic one) expect a fully qualified URL, but the
+        // website object skips http:// for most URLs.
+        String urlProtocol = "";
+        try {
+            urlProtocol = (new URL(url)).getProtocol();
+        } catch (MalformedURLException e) {
+            url = "http://" + url;
+        }
+
+        if (urlProtocol.isEmpty()) {
+            try {
+                urlProtocol = (new URL(url)).getProtocol();
+            } catch (MalformedURLException e) {
+                return "";
+            }
+        }
+
+        if (!urlProtocol.matches("http|https")) {
+            url = "http://" + url;
+        }
+        return url;
+    }
+
     private void refresh() {
         setTitle(mSite.getTitle());
         String subtitleText = mSite.getSummary();
@@ -96,6 +180,26 @@ class WebsitePreference extends Preference {
                 mediaCaptureIcon.setVisibility(View.VISIBLE);
             }
         }
+
+        /*
+          The code below doesn't work (does not affect margins/paddings at all).

[newt (away), 2015/01/22 17:17:24]

The icon and title view already have padding and margins of 0, so I'm not sure
what you're trying to achieve here. See Android's preference_holo.xml and
preference_material.xml.

If you're trying to get rid of the spacing between the icon and the title,
that's going to be tricky with the default preference layout since the icon is
wrapped in a parent view that has a min width. You *could* remove the parent's
min width, but then we're relying on implementation details in Android (maybe OK
here, since preference_holo and preference_material won't change, but
preference_fancy_new_future_android_ui might have a very different structure).
Alternatively, you could use a custom preference layout.

+          Any ideas on how to fix it?
+        */
+        ImageView icon = (ImageView) view.findViewById(android.R.id.icon);
+        icon.setPadding(0, 0, 0, 0);
+        icon.setMinimumWidth(0);
+        MarginLayoutParams params = (MarginLayoutParams) icon.getLayoutParams();
+        params.setMarginStart(0);
+        params.setMarginEnd(0);
+        icon.setLayoutParams(params);
+
+        TextView title = (TextView) view.findViewById(android.R.id.title);
+        title.setPadding(0, 0, 0, 0);
+        title.setMinimumWidth(0);
+        params = (MarginLayoutParams) title.getLayoutParams();
+        params.setMarginStart(0);
+        params.setMarginEnd(0);
+        title.setLayoutParams(params);
     }
 
     /**