[CIF] Allow ability to fetch gifs from java

This allows fetching gifs from java using new api methods. Involved
some additions/changes to the customization of CachedImageFetcher:
* Allow skipping transcoding option when only fetching image_data.
* Pipe up FetchImageData to Java.
* Wrap FetchImageData into FetchGif.
* Write code to potentially fetch/decode gif from disk in Java.

Bug: 919915
Change-Id: I8d825272f67b136e5c0d79bdff236d37e3f70f73
Reviewed-on: https://chromium-review.googlesource.com/c/1474799
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635850}

chrome/android/java/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcher.java

@@ -9,6 +9,8 @@ import android.graphics.Bitmap;
 import org.chromium.base.Callback;
 import org.chromium.base.ThreadUtils;
 
+import jp.tomorrowkey.android.gifplayer.BaseGifImage;
+
 /**
  * Provides cached image fetching capabilities. Uses getLastUsedProfile, which
  * will need to be changed when supporting multi-profile.
@@ -32,6 +34,17 @@ public interface CachedImageFetcher {
      */
     void reportEvent(String clientName, @CachedImageFetcherEvent int eventId);
 
+    /**
+     * Fetch the gif for the given url.
+     *
+     * @param url The url to fetch the image from.
+     * @param clientName The UMA client name to report the metrics to. If using CachedImageFetcher
+     *         to fetch images and gifs, use separate clientNames for them.
+     * @param callback The function which will be called when the image is ready; will be called
+     *         with null result if fetching fails.
+     */
+    void fetchGif(String url, String clientName, Callback<BaseGifImage> callback);
+
     /**
      * Fetches the image at url with the desired size. Image is null if not
      * found or fails decoding.
@@ -39,11 +52,11 @@ public interface CachedImageFetcher {
      * @param url The url to fetch the image from.
      * @param clientName Name of the cached image fetcher client to report UMA metrics for.
      * @param width The new bitmap's desired width (in pixels). If the given value is <= 0, the
-     * image won't be scaled.
+     *         image won't be scaled.
      * @param height The new bitmap's desired height (in pixels). If the given value is <= 0, the
-     * image won't be scaled.
+     *         image won't be scaled.
      * @param callback The function which will be called when the image is ready; will be called
-     * with null result if fetching fails;
+     *         with null result if fetching fails;
      */
     void fetchImage(
             String url, String clientName, int width, int height, Callback<Bitmap> callback);
@@ -54,7 +67,7 @@ public interface CachedImageFetcher {
      * @param url The url to fetch the image from.
      * @param clientName Name of the cached image fetcher client to report UMA metrics for.
      * @param callback The function which will be called when the image is ready; will be called
-     * with null result if fetching fails;
+     *         with null result if fetching fails;
      */
     void fetchImage(String url, String clientName, Callback<Bitmap> callback);
 

chrome/android/java/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherBridge.java

@@ -10,6 +10,8 @@ import org.chromium.base.Callback;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.chrome.browser.profiles.Profile;
 
+import jp.tomorrowkey.android.gifplayer.BaseGifImage;
+
 /**
  * Provides access to native implementations of CachedImageFetcher for the given profile.
  */
@@ -43,6 +45,24 @@ class CachedImageFetcherBridge {
         return nativeGetFilePath(mNativeCachedImageFetcherBridge, url);
     }
 
+    /**
+     * Fetch a gif from native or null if the gif can't be fetched or decoded.
+     *
+     * @param url The url to fetch.
+     * @param clientName The UMA client name to report the metrics to.
+     * @param callback The callback to call when the gif is ready.
+     */
+    public void fetchGif(String url, String clientName, Callback<BaseGifImage> callback) {
+        assert mNativeCachedImageFetcherBridge != 0;
+        nativeFetchImageData(mNativeCachedImageFetcherBridge, url, clientName, (byte[] data) -> {
+            if (data == null || data.length == 0) {
+                callback.onResult(null);
+            }
+
+            callback.onResult(new BaseGifImage(data));
+        });
+    }
+
     /**
      * Fetch the image from native.
      *
@@ -78,14 +98,30 @@ class CachedImageFetcherBridge {
         nativeReportCacheHitTime(mNativeCachedImageFetcherBridge, clientName, startTimeMillis);
     }
 
+    /**
+     * Report a timing event for a call to native
+     *
+     * @param clientName The UMA client name to report the metrics to.
+     * @param startTimeMillis The start time (in milliseconds) of the request, used to measure the
+     * total duration.
+     */
+    public void reportTotalFetchTimeFromNative(String clientName, long startTimeMillis) {
+        assert mNativeCachedImageFetcherBridge != 0;
+        nativeReportCacheHitTime(mNativeCachedImageFetcherBridge, clientName, startTimeMillis);
+    }
+
     // Native methods
     private static native long nativeInit(Profile profile);
     private native void nativeDestroy(long nativeCachedImageFetcherBridge);
     private native String nativeGetFilePath(long nativeCachedImageFetcherBridge, String url);
+    private native void nativeFetchImageData(long nativeCachedImageFetcherBridge, String url,
+            String clientName, Callback<byte[]> callback);
     private native void nativeFetchImage(long nativeCachedImageFetcherBridge, String url,
             String clientName, Callback<Bitmap> callback);
     private native void nativeReportEvent(
             long nativeCachedImageFetcherBridge, String clientName, int eventId);
     private native void nativeReportCacheHitTime(
             long nativeCachedImageFetcherBridge, String clientName, long startTimeMillis);
+    private native void nativeReportTotalFetchTimeFromNative(
+            long nativeCachedImageFetcherBridge, String clientName, long startTimeMillis);
 }

chrome/android/java/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherImpl.java

@@ -8,17 +8,23 @@ import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 
 import org.chromium.base.Callback;
+import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.task.AsyncTask;
 import org.chromium.chrome.browser.profiles.Profile;
 
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+
+import jp.tomorrowkey.android.gifplayer.BaseGifImage;
 
 /**
  * Implementation that uses a disk cache.
  */
 public class CachedImageFetcherImpl implements CachedImageFetcher {
+    private static final String TAG = "CachedImageFetcher";
     private static CachedImageFetcherImpl sInstance;
 
     public static CachedImageFetcherImpl getInstance() {
@@ -63,6 +69,34 @@ public class CachedImageFetcherImpl implements CachedImageFetcher {
         // Do nothing, this lives for the lifetime of the application.
     }
 
+    @Override
+    public void fetchGif(String url, String clientName, Callback<BaseGifImage> callback) {
+        long startTimeMillis = System.currentTimeMillis();
+        String filePath = mCachedImageFetcherBridge.getFilePath(url);
+        new AsyncTask<BaseGifImage>() {
+            @Override
+            protected BaseGifImage doInBackground() {
+                return tryToLoadGifFromDisk(filePath);
+            }
+
+            @Override
+            protected void onPostExecute(BaseGifImage gif) {
+                if (gif != null) {
+                    callback.onResult(gif);
+                    reportEvent(clientName, CachedImageFetcherEvent.JAVA_DISK_CACHE_HIT);
+                    mCachedImageFetcherBridge.reportCacheHitTime(clientName, startTimeMillis);
+                } else {
+                    mCachedImageFetcherBridge.fetchGif(
+                            url, clientName, (BaseGifImage gifFromNative) -> {
+                                callback.onResult(gifFromNative);
+                                mCachedImageFetcherBridge.reportTotalFetchTimeFromNative(
+                                        clientName, startTimeMillis);
+                            });
+                }
+            }
+        }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
+    }
+
     @Override
     public void fetchImage(
             String url, String clientName, int width, int height, Callback<Bitmap> callback) {
@@ -88,6 +122,7 @@ public class CachedImageFetcherImpl implements CachedImageFetcher {
             String url, String clientName, int width, int height, Callback<Bitmap> callback) {
         long startTimeMillis = System.currentTimeMillis();
         String filePath = mCachedImageFetcherBridge.getFilePath(url);
+        // TODO(wylieb): Transition to new way of doing async tasks in this file.
         new AsyncTask<Bitmap>() {
             @Override
             protected Bitmap doInBackground() {
@@ -101,7 +136,12 @@ public class CachedImageFetcherImpl implements CachedImageFetcher {
                     reportEvent(clientName, CachedImageFetcherEvent.JAVA_DISK_CACHE_HIT);
                     mCachedImageFetcherBridge.reportCacheHitTime(clientName, startTimeMillis);
                 } else {
-                    mCachedImageFetcherBridge.fetchImage(url, clientName, callback);
+                    mCachedImageFetcherBridge.fetchImage(
+                            url, clientName, (Bitmap bitmapFromNative) -> {
+                                callback.onResult(bitmapFromNative);
+                                mCachedImageFetcherBridge.reportTotalFetchTimeFromNative(
+                                        clientName, startTimeMillis);
+                            });
                 }
             }
         }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
@@ -116,4 +156,21 @@ public class CachedImageFetcherImpl implements CachedImageFetcher {
             return null;
         }
     }
+
+    @VisibleForTesting
+    BaseGifImage tryToLoadGifFromDisk(String filePath) {
+        try {
+            File file = new File(filePath);
+            byte[] fileBytes = new byte[(int) file.length()];
+            FileInputStream ios = new FileInputStream(filePath);
+
+            int bytesRead = ios.read(fileBytes);
+            if (bytesRead != fileBytes.length) return null;
+
+            return new BaseGifImage(fileBytes);
+        } catch (IOException e) {
+            Log.w(TAG, "Failed to read: %s", filePath, e);
+            return null;
+        }
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/cached_image_fetcher/InMemoryCachedImageFetcher.java

@@ -14,6 +14,8 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.BitmapCache;
 import org.chromium.chrome.browser.util.ConversionUtils;
 
+import jp.tomorrowkey.android.gifplayer.BaseGifImage;
+
 /**
  * A wrapper around the CachedImageFetcher that also provides in-memory caching.
  */
@@ -61,6 +63,11 @@ public class InMemoryCachedImageFetcher implements CachedImageFetcher {
         mBitmapCache = null;
     }
 
+    @Override
+    public void fetchGif(String url, String clientName, Callback<BaseGifImage> callback) {
+        mCachedImageFetcher.fetchGif(url, clientName, callback);
+    }
+
     @Override
     public void fetchImage(
             String url, String clientName, int width, int height, Callback<Bitmap> callback) {

chrome/android/junit/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherImplTest.java

@@ -33,6 +33,8 @@ import org.chromium.base.task.test.BackgroundShadowAsyncTask;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.contextmenu.ChromeContextMenuPopulatorTest.ShadowUrlUtilities;
 
+import jp.tomorrowkey.android.gifplayer.BaseGifImage;
+
 /**
  * Unit tests for CachedImageFetcherImpl.
  */
@@ -51,6 +53,8 @@ public class CachedImageFetcherImplTest {
     CachedImageFetcherBridge mCachedImageFetcherBridge;
     @Mock
     Bitmap mBitmap;
+    @Mock
+    BaseGifImage mGif;
 
     @Captor
     ArgumentCaptor<Callback<Bitmap>> mCallbackCaptor;
@@ -148,4 +152,33 @@ public class CachedImageFetcherImplTest {
         verify(mCachedImageFetcherBridge).fetchImage(eq(URL), eq(UMA_CLIENT_NAME), any());
         verify(mCachedImageFetcherBridge).fetchImage(eq(URL), eq(UMA_CLIENT_NAME + "2"), any());
     }
+
+    @Test
+    @SmallTest
+    public void testFetchGifFoundOnDisk() throws Exception {
+        Mockito.doReturn(mGif).when(mCachedImageFetcher).tryToLoadGifFromDisk(anyObject());
+        mCachedImageFetcher.fetchGif(
+                URL, UMA_CLIENT_NAME, (BaseGifImage gif) -> { assertEquals(gif, mGif); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+
+        verify(mCachedImageFetcherBridge, never()) // Should never make it to native.
+                .fetchGif(eq(URL), eq(UMA_CLIENT_NAME), any());
+
+        // Verify metrics have been reported.
+        verify(mCachedImageFetcherBridge)
+                .reportEvent(eq(UMA_CLIENT_NAME), eq(CachedImageFetcherEvent.JAVA_DISK_CACHE_HIT));
+        verify(mCachedImageFetcherBridge).reportCacheHitTime(eq(UMA_CLIENT_NAME), anyLong());
+    }
+
+    @Test
+    @SmallTest
+    public void testFetchGifCallToNative() throws Exception {
+        Mockito.doReturn(null).when(mCachedImageFetcher).tryToLoadGifFromDisk(anyObject());
+        mCachedImageFetcher.fetchGif(URL, UMA_CLIENT_NAME, (BaseGifImage gif) -> {});
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+
+        verify(mCachedImageFetcherBridge).fetchGif(eq(URL), eq(UMA_CLIENT_NAME), any());
+    }
 }

chrome/browser/android/cached_image_fetcher/cached_image_fetcher_bridge.cc

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/android/callback_android.h"
+#include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ -99,6 +100,32 @@ ScopedJavaLocalRef<jstring> CachedImageFetcherBridge::GetFilePath(
   return base::android::ConvertUTF8ToJavaString(j_env, file_path);
 }
 
+void CachedImageFetcherBridge::FetchImageData(
+    JNIEnv* j_env,
+    const JavaRef<jobject>& j_this,
+    const JavaRef<jstring>& j_url,
+    const JavaRef<jstring>& j_client_name,
+    const JavaRef<jobject>& j_callback) {
+  ScopedJavaGlobalRef<jobject> callback(j_callback);
+  std::string url = base::android::ConvertJavaStringToUTF8(j_url);
+  std::string client_name =
+      base::android::ConvertJavaStringToUTF8(j_client_name);
+
+  image_fetcher::ImageFetcherParams params(kTrafficAnnotation, client_name);
+
+  // We can skip transcoding here because this method is used in java as
+  // CachedImageFetcher.fetchGif, which decodes the data in a Java-only library.
+  params.set_skip_transcoding(true);
+
+  // TODO(wylieb): We checked disk in Java, so provide a way to tell
+  // CachedImageFetcher to skip checking disk in native.
+  cached_image_fetcher_->FetchImageData(
+      GURL(url),
+      base::BindOnce(&CachedImageFetcherBridge::OnImageDataFetched,
+                     weak_ptr_factory_.GetWeakPtr(), callback),
+      std::move(params));
+}
+
 void CachedImageFetcherBridge::FetchImage(JNIEnv* j_env,
                                           const JavaRef<jobject>& j_this,
                                           const JavaRef<jstring>& j_url,
@@ -144,6 +171,29 @@ void CachedImageFetcherBridge::ReportCacheHitTime(
       client_name, start_time);
 }
 
+void CachedImageFetcherBridge::ReportTotalFetchTimeFromNative(
+    JNIEnv* j_env,
+    const base::android::JavaRef<jobject>& j_this,
+    const base::android::JavaRef<jstring>& j_client_name,
+    const jlong start_time_millis) {
+  std::string client_name =
+      base::android::ConvertJavaStringToUTF8(j_client_name);
+  base::Time start_time = base::Time::FromJavaTime(start_time_millis);
+  CachedImageFetcherMetricsReporter::ReportTotalFetchFromNativeTimeJava(
+      client_name, start_time);
+}
+
+void CachedImageFetcherBridge::OnImageDataFetched(
+    base::android::ScopedJavaGlobalRef<jobject> callback,
+    const std::string& image_data,
+    const RequestMetadata& request_metadata) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jbyteArray> j_bytes = base::android::ToJavaByteArray(
+      env, reinterpret_cast<const uint8_t*>(image_data.data()),
+      image_data.size());
+  RunObjectCallbackAndroid(callback, j_bytes);
+}
+
 void CachedImageFetcherBridge::OnImageFetched(
     base::android::ScopedJavaGlobalRef<jobject> callback,
     const gfx::Image& image,

chrome/browser/android/cached_image_fetcher/cached_image_fetcher_bridge.h

@@ -32,6 +32,12 @@ class CachedImageFetcherBridge {
       const base::android::JavaRef<jobject>& j_this,
       const base::android::JavaRef<jstring>& j_url);
 
+  void FetchImageData(JNIEnv* j_env,
+                      const base::android::JavaRef<jobject>& j_this,
+                      const base::android::JavaRef<jstring>& j_url,
+                      const base::android::JavaRef<jstring>& j_client_name,
+                      const base::android::JavaRef<jobject>& j_callback);
+
   void FetchImage(JNIEnv* j_env,
                   const base::android::JavaRef<jobject>& j_this,
                   const base::android::JavaRef<jstring>& j_url,
@@ -48,7 +54,17 @@ class CachedImageFetcherBridge {
                           const base::android::JavaRef<jstring>& j_client_name,
                           const jlong start_time_millis);
 
+  void ReportTotalFetchTimeFromNative(
+      JNIEnv* j_env,
+      const base::android::JavaRef<jobject>& j_this,
+      const base::android::JavaRef<jstring>& j_client_name,
+      const jlong start_time_millis);
+
  private:
+  void OnImageDataFetched(base::android::ScopedJavaGlobalRef<jobject> callback,
+                          const std::string& image_data,
+                          const RequestMetadata& request_metadata);
+
   void OnImageFetched(base::android::ScopedJavaGlobalRef<jobject> callback,
                       const gfx::Image& image,
                       const RequestMetadata& request_metadata);

components/image_fetcher/core/cache/cached_image_fetcher_metrics_reporter.cc

@@ -22,6 +22,8 @@ constexpr char kImageLoadFromCacheHistogram[] =
     "CachedImageFetcher.ImageLoadFromCacheTime";
 constexpr char kImageLoadFromCacheJavaHistogram[] =
     "CachedImageFetcher.ImageLoadFromCacheTimeJava";
+constexpr char kTotalFetchFromNativeTimeJavaHistogram[] =
+    "CachedImageFetcher.ImageLoadFromNativeTimeJava";
 constexpr char kImageLoadFromNetworkHistogram[] =
     "CachedImageFetcher.ImageLoadFromNetworkTime";
 constexpr char kImageLoadFromNetworkAfterCacheHitHistogram[] =
@@ -75,6 +77,17 @@ void CachedImageFetcherMetricsReporter::ReportImageLoadFromCacheTimeJava(
       ->Add(time_delta.InMilliseconds());
 }
 
+// static
+void CachedImageFetcherMetricsReporter::ReportTotalFetchFromNativeTimeJava(
+    const std::string& client_name,
+    base::Time start_time) {
+  DCHECK(!client_name.empty());
+  base::TimeDelta time_delta = base::Time::Now() - start_time;
+  UMA_HISTOGRAM_TIMES(kTotalFetchFromNativeTimeJavaHistogram, time_delta);
+  GetTimeHistogram(kTotalFetchFromNativeTimeJavaHistogram, client_name)
+      ->Add(time_delta.InMilliseconds());
+}
+
 // static
 void CachedImageFetcherMetricsReporter::ReportImageLoadFromNetworkTime(
     const std::string& client_name,

components/image_fetcher/core/cache/cached_image_fetcher_metrics_reporter.h

@@ -47,6 +47,10 @@ class CachedImageFetcherMetricsReporter {
   static void ReportImageLoadFromCacheTimeJava(const std::string& client_name,
                                                base::Time start_time);
 
+  // Report the time it takes to load an image from native code.
+  static void ReportTotalFetchFromNativeTimeJava(const std::string& client_name,
+                                                 base::Time start_time);
+
   // Report the time it takes to load an image from the network.
   static void ReportImageLoadFromNetworkTime(const std::string& client_name,
                                              base::Time start_time);

components/image_fetcher/core/cache/cached_image_fetcher_metrics_reporter_unittest.cc

@@ -22,6 +22,8 @@ const char kCacheLoadHistogramName[] =
     "CachedImageFetcher.ImageLoadFromCacheTime";
 const char kCacheLoadHistogramNameJava[] =
     "CachedImageFetcher.ImageLoadFromCacheTimeJava";
+constexpr char kTotalFetchFromNativeHistogramNameJava[] =
+    "CachedImageFetcher.ImageLoadFromNativeTimeJava";
 const char kNetworkLoadHistogramName[] =
     "CachedImageFetcher.ImageLoadFromNetworkTime";
 const char kNetworkLoadAfterCacheHitHistogram[] =
@@ -96,6 +98,26 @@ TEST_F(CachedImageFetcherMetricsReporterTest,
                                       1);
 }
 
+TEST_F(CachedImageFetcherMetricsReporterTest,
+       TestReportTotalFetchFromNativeTimeJava) {
+  CachedImageFetcherMetricsReporter::ReportTotalFetchFromNativeTimeJava(
+      kUmaClientName, base::Time());
+  CachedImageFetcherMetricsReporter::ReportTotalFetchFromNativeTimeJava(
+      kUmaClientNameOther, base::Time());
+  histogram_tester().ExpectTotalCount(kTotalFetchFromNativeHistogramNameJava,
+                                      2);
+  histogram_tester().ExpectTotalCount(
+      std::string(kTotalFetchFromNativeHistogramNameJava)
+          .append(".")
+          .append(kUmaClientName),
+      1);
+  histogram_tester().ExpectTotalCount(
+      std::string(kTotalFetchFromNativeHistogramNameJava)
+          .append(".")
+          .append(kUmaClientNameOther),
+      1);
+}
+
 TEST_F(CachedImageFetcherMetricsReporterTest,
        TestReportImageLoadFromNetworkTime) {
   CachedImageFetcherMetricsReporter::ReportImageLoadFromNetworkTime(

components/image_fetcher/core/cached_image_fetcher.cc

@@ -57,14 +57,21 @@ void ImageCallbackIfPresent(ImageFetcherCallback image_callback,
   std::move(image_callback).Run(image, metadata);
 }
 
-std::string EncodeSkBitmapToPNG(const SkBitmap& bitmap) {
+std::string EncodeSkBitmapToPNG(const std::string& uma_client_name,
+                                const SkBitmap& bitmap) {
   std::vector<unsigned char> encoded_data;
   bool result = gfx::PNGCodec::Encode(
       static_cast<const unsigned char*>(bitmap.getPixels()),
       gfx::PNGCodec::FORMAT_RGBA, gfx::Size(bitmap.width(), bitmap.height()),
       static_cast<int>(bitmap.rowBytes()), /* discard_transparency */ false,
       std::vector<gfx::PNGCodec::Comment>(), &encoded_data);
-  return result ? std::string(encoded_data.begin(), encoded_data.end()) : "";
+  if (!result) {
+    CachedImageFetcherMetricsReporter::ReportEvent(
+        uma_client_name, CachedImageFetcherEvent::kTranscodingError);
+    return "";
+  } else {
+    return std::string(encoded_data.begin(), encoded_data.end());
+  }
 }
 
 }  // namespace
@@ -183,23 +190,55 @@ void CachedImageFetcher::FetchImageFromNetwork(
     ImageDataFetcherCallback image_data_callback,
     ImageFetcherCallback image_callback) {
   const GURL& url = request.url;
-  bool cache_hit = request.cache_hit_before_network_request;
-  image_fetcher_->FetchImageAndData(
-      url,
-      cache_hit ? ImageDataFetcherCallback() : std::move(image_data_callback),
-      base::BindOnce(&CachedImageFetcher::OnImageFetchedFromNetwork,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(request),
-                     std::move(image_callback)),
-      std::move(request.params));
+
+  ImageDataFetcherCallback wrapper_data_callback;
+  ImageFetcherCallback wrapper_image_callback;
+
+  bool skip_transcoding = request.params.skip_transcoding();
+  if (skip_transcoding) {
+    wrapper_data_callback =
+        base::BindOnce(&CachedImageFetcher::StoreImageDataWithoutTranscoding,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(request),
+                       std::move(image_data_callback));
+  } else {
+    // Transcode the image when its downloaded from the network.
+    // 1. Download the data.
+    // 2. Decode the data to a gfx::Image in a utility process.
+    // 3. Encode the data as a PNG in the browser process using base::PostTask.
+    // 3. Cache the result.
+    wrapper_data_callback = std::move(image_data_callback);
+    wrapper_image_callback =
+        base::BindOnce(&CachedImageFetcher::StoreImageDataWithTranscoding,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(request),
+                       std::move(image_callback));
+  }
+  image_fetcher_->FetchImageAndData(url, std::move(wrapper_data_callback),
+                                    std::move(wrapper_image_callback),
+                                    std::move(request.params));
+}
+
+void CachedImageFetcher::StoreImageDataWithoutTranscoding(
+    CachedImageFetcherRequest request,
+    ImageDataFetcherCallback image_data_callback,
+    const std::string& image_data,
+    const RequestMetadata& request_metadata) {
+  DataCallbackIfPresent(std::move(image_data_callback), image_data,
+                        request_metadata);
+
+  if (image_data.empty()) {
+    CachedImageFetcherMetricsReporter::ReportEvent(
+        request.params.uma_client_name(),
+        CachedImageFetcherEvent::kTotalFailure);
+  }
+
+  StoreData(std::move(request), image_data);
 }
 
-void CachedImageFetcher::OnImageFetchedFromNetwork(
+void CachedImageFetcher::StoreImageDataWithTranscoding(
     CachedImageFetcherRequest request,
     ImageFetcherCallback image_callback,
     const gfx::Image& image,
     const RequestMetadata& request_metadata) {
-  // The image has been deocded by the fetcher already, return straight to the
-  // caller.
   ImageCallbackIfPresent(std::move(image_callback), image, request_metadata);
 
   // Report to different histograms depending upon if there was a cache hit.
@@ -218,24 +257,23 @@ void CachedImageFetcher::OnImageFetchedFromNetwork(
     CachedImageFetcherMetricsReporter::ReportEvent(
         request.params.uma_client_name(),
         CachedImageFetcherEvent::kTotalFailure);
-    StoreEncodedData(std::move(request), "");
+    StoreData(std::move(request), "");
   } else {
+    std::string uma_client_name = request.params.uma_client_name();
     // Post a task to another thread to encode the image data downloaded.
     base::PostTaskAndReplyWithResult(
-        FROM_HERE, base::BindOnce(&EncodeSkBitmapToPNG, *bitmap),
-        base::BindOnce(&CachedImageFetcher::StoreEncodedData,
+        FROM_HERE,
+        base::BindOnce(&EncodeSkBitmapToPNG, uma_client_name, *bitmap),
+        base::BindOnce(&CachedImageFetcher::StoreData,
                        weak_ptr_factory_.GetWeakPtr(), std::move(request)));
   }
 }
 
-void CachedImageFetcher::StoreEncodedData(CachedImageFetcherRequest request,
-                                          std::string image_data) {
+void CachedImageFetcher::StoreData(CachedImageFetcherRequest request,
+                                   std::string image_data) {
   std::string url = request.url.spec();
   // If the image is empty, delete the image.
   if (image_data.empty()) {
-    CachedImageFetcherMetricsReporter::ReportEvent(
-        request.params.uma_client_name(),
-        CachedImageFetcherEvent::kTranscodingError);
     image_cache_->DeleteImage(std::move(url));
     return;
   }

components/image_fetcher/core/cached_image_fetcher.h

@@ -66,12 +66,16 @@ class CachedImageFetcher : public ImageFetcher {
       CachedImageFetcherRequest request,
       ImageDataFetcherCallback image_data_callback,
       ImageFetcherCallback image_callback);
-  void OnImageFetchedFromNetwork(CachedImageFetcherRequest request,
-                                 ImageFetcherCallback image_callback,
-                                 const gfx::Image& image,
-                                 const RequestMetadata& request_metadata);
-  void StoreEncodedData(CachedImageFetcherRequest request,
-                        std::string image_data);
+  void StoreImageDataWithoutTranscoding(
+      CachedImageFetcherRequest request,
+      ImageDataFetcherCallback image_data_callback,
+      const std::string& image_data,
+      const RequestMetadata& request_metadata);
+  void StoreImageDataWithTranscoding(CachedImageFetcherRequest request,
+                                     ImageFetcherCallback image_data_callback,
+                                     const gfx::Image& image,
+                                     const RequestMetadata& request_metadata);
+  void StoreData(CachedImageFetcherRequest request, std::string image_data);
 
   // Whether the ImageChache is allowed to be modified in any way from requests
   // made by this CachedImageFetcher. This includes updating last used times,

components/image_fetcher/core/cached_image_fetcher_unittest.cc

@@ -57,8 +57,6 @@ const char kCacheLoadHistogramName[] =
     "CachedImageFetcher.ImageLoadFromCacheTime";
 const char kNetworkLoadHistogramName[] =
     "CachedImageFetcher.ImageLoadFromNetworkTime";
-const char kNetworkLoadAfterCacheHitHistogram[] =
-    "CachedImageFetcher.ImageLoadFromNetworkAfterCacheHit";
 
 }  // namespace
 
@@ -312,26 +310,31 @@ TEST_F(CachedImageFetcherTest, FetchImagePopulatesCacheReadOnly) {
   }
 }
 
-TEST_F(CachedImageFetcherTest, FetchDecodingErrorDeletesCache) {
-  // Save the image in the database.
-  image_cache()->SaveImage(kImageUrl.spec(), kImageData);
-  RunUntilIdle();
+TEST_F(CachedImageFetcherTest, FetchImageWithoutTranscodingDoesNotDecode) {
+  {
+    test_url_loader_factory()->AddResponse(kImageUrl.spec(), kImageData);
+    image_decoder()->SetDecodingValid(false);
 
-  image_decoder()->SetDecodingValid(false);
-  base::MockCallback<ImageDataFetcherCallback> data_callback;
-  base::MockCallback<ImageFetcherCallback> image_callback;
-  EXPECT_CALL(data_callback, Run(NonEmptyString(), _));
-  EXPECT_CALL(image_callback, Run(EmptyImage(), _));
-  test_url_loader_factory()->AddResponse(kImageUrl.spec(), kImageData);
-  cached_image_fetcher()->FetchImageAndData(
-      kImageUrl, data_callback.Get(), image_callback.Get(),
-      ImageFetcherParams(TRAFFIC_ANNOTATION_FOR_TESTS, kUmaClientName));
-  RunUntilIdle();
+    base::MockCallback<ImageDataFetcherCallback> data_callback;
+
+    EXPECT_CALL(data_callback, Run(kImageData, _));
+    ImageFetcherParams params(TRAFFIC_ANNOTATION_FOR_TESTS, kUmaClientName);
+    params.set_skip_transcoding_for_testing(true);
+    cached_image_fetcher()->FetchImageAndData(kImageUrl, data_callback.Get(),
+                                              ImageFetcherCallback(), params);
+
+    RunUntilIdle();
+  }
+  {
+    test_url_loader_factory()->ClearResponses();
+    base::MockCallback<ImageDataFetcherCallback> data_callback;
+    EXPECT_CALL(data_callback, Run(kImageData, _));
+    cached_image_fetcher()->FetchImageAndData(
+        kImageUrl, data_callback.Get(), ImageFetcherCallback(),
+        ImageFetcherParams(TRAFFIC_ANNOTATION_FOR_TESTS, kUmaClientName));
 
-  histogram_tester().ExpectTotalCount(kNetworkLoadAfterCacheHitHistogram, 1);
-  histogram_tester().ExpectBucketCount(
-      kCachedImageFetcherEventHistogramName,
-      CachedImageFetcherEvent::kTranscodingError, 1);
+    RunUntilIdle();
+  }
 }
 
 }  // namespace image_fetcher

components/image_fetcher/core/image_fetcher.cc

@@ -10,7 +10,8 @@ ImageFetcherParams::ImageFetcherParams(
     const net::NetworkTrafficAnnotationTag network_traffic_annotation_tag,
     std::string uma_client_name)
     : network_traffic_annotation_tag_(network_traffic_annotation_tag),
-      uma_client_name_(uma_client_name) {}
+      uma_client_name_(uma_client_name),
+      skip_transcoding_(false) {}
 
 ImageFetcherParams::ImageFetcherParams(const ImageFetcherParams& params) =
     default;

components/image_fetcher/core/image_fetcher.h

@@ -32,6 +32,10 @@ class ImageDecoder;
 //   that's closest to the given size (only useful for .icos). Does NOT resize
 //   the downloaded image to the given dimensions.
 class ImageFetcherParams {
+  // Only allow the bridge to access the private function set_skip_transcoding
+  // used for gif download.
+  friend class CachedImageFetcherBridge;
+
  public:
   // Sets the UMA client name to report feature-specific metrics. Make sure
   // |uma_client_name| is also present in histograms.xml.
@@ -63,12 +67,28 @@ class ImageFetcherParams {
 
   const std::string& uma_client_name() const { return uma_client_name_; }
 
+  bool skip_transcoding() const { return skip_transcoding_; }
+
+  // Only to be used in unittests.
+  void set_skip_transcoding_for_testing(bool skip_transcoding) {
+    skip_transcoding_ = skip_transcoding;
+  }
+
  private:
+  void set_skip_transcoding(bool skip_transcoding) {
+    skip_transcoding_ = skip_transcoding;
+  }
+
   const net::NetworkTrafficAnnotationTag network_traffic_annotation_tag_;
 
   base::Optional<int64_t> max_download_bytes_;
   gfx::Size desired_frame_size_;
   std::string uma_client_name_;
+  // When true, the image fetcher will skip transcoding whenever possible. Only
+  // use this if you've considered the security implications. For instance, in
+  // some java clients we decode GIFs entirely in Java which is safe to do
+  // in-process without transcoding.
+  bool skip_transcoding_;
 };
 
 // A class used to fetch server images. It can be called from any thread and the

tools/metrics/histograms/histograms.xml

@@ -13652,6 +13652,16 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="CachedImageFetcher.ImageLoadFromNativeTimeJava" units="ms"
+    expires_after="2019-12-01">
+  <owner>fgorski@chromium.org</owner>
+  <owner>wylieb@chromium.org</owner>
+  <summary>
+    The time it takes for cached_image_fetcher to load an image from native
+    code. Only recorded on successful loads.
+  </summary>
+</histogram>
+
 <histogram name="CachedImageFetcher.ImageLoadFromNetworkAfterCacheHit"
     units="ms" expires_after="2019-12-01">
   <owner>fgorski@chromium.org</owner>
@@ -134267,6 +134277,7 @@ uploading your change for review.
   <affected-histogram name="CachedImageFetcher.Events"/>
   <affected-histogram name="CachedImageFetcher.ImageLoadFromCacheTime"/>
   <affected-histogram name="CachedImageFetcher.ImageLoadFromCacheTimeJava"/>
+  <affected-histogram name="CachedImageFetcher.ImageLoadFromNativeTimeJava"/>
   <affected-histogram
       name="CachedImageFetcher.ImageLoadFromNetworkAfterCacheHit"/>
   <affected-histogram name="CachedImageFetcher.ImageLoadFromNetworkTime"/>