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..b77fa29bb47997a76cb1b1f61f4cc878e83208bf 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.
@@ -799,7 +804,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 +945,16 @@ 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
+                if (!mItem.getLargeIconPath().isEmpty()) {

[newt (away), 2016/03/08 22:09:04]

Where are these icons stored and how did they get there? Are they inside the
Chrome app directory? Can we trust that this path isn't malicious?

[atanasova, 2016/03/09 11:40:55]

These icons come from the whitelist extension installation. They are stored in
the app data directory and we get and save the path during the extension
installation.
The extension manifest has a category called icons and there is has the path to
the icon.
I am not sure if we can know for certain whether the content specified is not
malicious, so do you have any suggestions on the Android side for us to check?

@treib do you think we can have a problem here?

[Marc Treib, 2016/03/09 12:01:46]

I think we can be reasonably sure that the path itself is okay - we (more
specifically, the component updater) put the file there.
The file contents themselves are of course not trusted, so we need to take care
when decoding the image. But that applies to all downloaded images.

[newt (away), 2016/03/09 21:09:58]

Ok, is there a design doc somewhere describing how the images are downloaded?
This should be fine, but you should definitely loop in security so they can take
a look.

+                    Bitmap bitmap = BitmapFactory.decodeFile(mItem.getLargeIconPath());
+                    if (bitmap != null) {
+                        icon = bitmap;
+                    } else {
+                        Log.w(TAG, "Icon path did not result in an image: "

[newt (away), 2016/03/08 22:09:03]

Is this really a condition worth logging? Avoid logging (especially at the
warning level) unless it's a significant situation that might be helpful when
reviewing crash reports or other bug reports. Log spam is a big problem.

Also, don't concatenate strings in logs. Use format strings, e.g.

    Log.d(TAG, "Bad path: %s", mItem.getLargeIconPath());

which results in less wasted work if the log is going to be discarded. Also, if
you use Log.d or Log.i, those logs will be stripped in release builds.

[atanasova, 2016/03/09 11:40:55]

This situation is not supposed to happen, because if we have saved a path and it
does not result in an image something went wrong - either the path is saved
badly or the image has a problem. I think we need to log that.
I changed it to Log.d.

+                                        + mItem.getLargeIconPath());
+                    }
+                }
                 if (icon == null) {
                     mIconGenerator.setBackgroundColor(fallbackColor);
                     icon = mIconGenerator.generateIconForUrl(mItem.getUrl());