Start using the whitelist icon on the NTP.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
index 2cca3fb1a2b7b646c8b8d4206c1151b78f16b7a5..46828f4a0d2c179f762bdb5530b11975708613af 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
@@ -8,6 +8,7 @@ import android.annotation.SuppressLint;
 import android.content.Context;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
 import android.graphics.Canvas;
 import android.graphics.Rect;
 import android.graphics.drawable.BitmapDrawable;
@@ -34,6 +35,7 @@ import android.widget.FrameLayout;
 import android.widget.ImageView;
 import android.widget.TextView;
 
+import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
@@ -68,6 +70,7 @@ public class NewTabPageView extends FrameLayout
 
     private static final int SHADOW_COLOR = 0x11000000;
     private static final long SNAP_SCROLL_DELAY_MS = 30;
+    private static final String TAG = "NewTabPageView";
 
     private ViewGroup mContentView;
     private NewTabScrollView mScrollView;
@@ -767,7 +770,8 @@ public class NewTabPageView extends FrameLayout
     // MostVisitedURLsObserver implementation
 
     @Override
-    public void onMostVisitedURLsAvailable(String[] titles, String[] urls) {
+    public void onMostVisitedURLsAvailable(
+            String[] titles, String[] urls, String[] largeIconPaths) {
         mMostVisitedLayout.removeAllViews();
 
         MostVisitedItem[] oldItems = mMostVisitedItems;
@@ -781,6 +785,7 @@ public class NewTabPageView extends FrameLayout
         for (int i = 0; i < titles.length; i++) {
             final String url = urls[i];
             final String title = titles[i];
+            final String largeIconPath = largeIconPaths[i];
             boolean offlineAvailable = mManager.isOfflineAvailable(url);
 
             // Look for an existing item to reuse.
@@ -789,7 +794,8 @@ public class NewTabPageView extends FrameLayout
                 MostVisitedItem oldItem = oldItems[j];
                 if (oldItem != null && TextUtils.equals(url, oldItem.getUrl())
                         && TextUtils.equals(title, oldItem.getTitle())
-                        && offlineAvailable == oldItem.isOfflineAvailable()) {
+                        && offlineAvailable == oldItem.isOfflineAvailable()
+                        && largeIconPath.equals(oldItem.getLargeIconPath())) {
                     item = oldItem;
                     item.setIndex(i);
                     oldItems[j] = null;
@@ -799,7 +805,8 @@ public class NewTabPageView extends FrameLayout
 
             // If nothing can be reused, create a new item.
             if (item == null) {
-                item = new MostVisitedItem(mManager, title, url, offlineAvailable, i);
+                item = new MostVisitedItem(
+                        mManager, title, url, largeIconPath, offlineAvailable, i);
                 View view =
                         mMostVisitedDesign.createMostVisitedItemView(inflater, item, isInitialLoad);
                 item.initView(view);
@@ -939,6 +946,15 @@ public class NewTabPageView extends FrameLayout
             @Override
             public void onLargeIconAvailable(Bitmap icon, int fallbackColor) {
                 MostVisitedItemView view = (MostVisitedItemView) mItem.getView();
+                // Try to use the locally stored icon

[newt (away), 2016/03/15 17:21:31]

Do we only want to load the whitelist icon as a fallback if "icon" is null? Or
do we always want to use the whitelist icon?

In the former case, you should also check "icon == null" in the if statement on
line 950. In the latter case, you should load the whitelist icon in
createMostVisitedItemView(), instead of calling mManager.getLargeIconForUrl().

Also, we should maybe load/decode these icons on a background thread using
AsyncTask. Could you measure how long it takes to decode each icon on your dev
device (using System.nanoTime() before and after the call to
BitmapFactory.decodeFile())?

[atanasova, 2016/03/16 10:55:24]

We have decided to have priority over the whitelist icon, so I have moved that
logic out.

I did the measurement and these are the results:
Average - 5276265.7 nanoseconds
Median - 4747370 nanoseconds
Low - 4004010 nanoseconds
High - 8692760 nanoseconds

Should we create an AsyncTask?

[newt (away), 2016/03/16 18:08:28]

An AsyncTask would help reduce jank a little bit, but it adds some complexity
and there's larger low-hanging fruit for reducing jank on the NTP. So, I think
it's fine not to use AsyncTask for now.

[atanasova, 2016/03/16 18:43:06]

Acknowledged.

+                if (!mItem.getLargeIconPath().isEmpty()) {
+                    Bitmap bitmap = BitmapFactory.decodeFile(mItem.getLargeIconPath());
+                    if (bitmap != null) {
+                        icon = bitmap;
+                    } else {
+                        Log.d(TAG, "Image decoding failed: %s", mItem.getLargeIconPath());
+                    }
+                }
                 if (icon == null) {
                     mIconGenerator.setBackgroundColor(fallbackColor);
                     icon = mIconGenerator.generateIconForUrl(mItem.getUrl());