[IC] Java CachedImageFetcher metrics reporting

Reports metrics occurring in Java to the native CachedImageFetcher
reporting infra.

Bug: 905732
Change-Id: I9f25527fa9459bf1e433f6169de0640b6600248a
Reviewed-on: https://chromium-review.googlesource.com/c/1347510
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613390}

chrome/android/BUILD.gn

@@ -367,6 +367,7 @@ android_library("chrome_java") {
     "//components/favicon_base:favicon_base_enums_java",
     "//components/dom_distiller/core:distiller_type_java",
     "//components/infobars/core:infobar_enums_java",
+    "//components/image_fetcher/core:cached_image_fetcher_java_enums_srcjar",
     "//components/ntp_snippets:ntp_snippets_java_enums_srcjar",
     "//components/ntp_tiles:ntp_tiles_enums_java",
     "//components/offline_pages/core:offline_page_model_enums_java",

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

@@ -19,6 +19,13 @@ public interface CachedImageFetcher {
         return CachedImageFetcherImpl.getInstance();
     }
 
+    /**
+     * Report an event metric.
+     *
+     * @param eventId The event to be reported
+     */
+    void reportEvent(@CachedImageFetcherEvent int eventId);
+
     /**
      * Fetches the image at url with the desired size. Image is null if not
      * found or fails decoding.
@@ -42,4 +49,4 @@ public interface CachedImageFetcher {
      * Destroy method, called to clear resources to prevent leakage.
      */
     void destroy();
-}
\ No newline at end of file
+}

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

@@ -56,10 +56,34 @@ class CachedImageFetcherBridge {
         nativeFetchImage(mNativeCachedImageFetcherBridge, url, width, height, callback);
     }
 
+    /**
+     * Report a metrics event.
+     *
+     * @param eventId The event to report.
+     */
+    public void reportEvent(@CachedImageFetcherEvent int eventId) {
+        assert mNativeCachedImageFetcherBridge != 0;
+        nativeReportEvent(mNativeCachedImageFetcherBridge, eventId);
+    }
+
+    /**
+     * Report a timing event for a cache hit.
+     *
+     * @param startTimeMillis The start time (in milliseconds) of the request, used to measure the
+     * total duration.
+     */
+    public void reportCacheHitTime(long startTimeMillis) {
+        assert mNativeCachedImageFetcherBridge != 0;
+        nativeReportCacheHitTime(mNativeCachedImageFetcherBridge, 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 nativeFetchImage(long nativeCachedImageFetcherBridge, String url,
             int widthPx, int heightPx, Callback<Bitmap> callback);
+    private native void nativeReportEvent(long nativeCachedImageFetcherBridge, int eventId);
+    private native void nativeReportCacheHitTime(
+            long nativeCachedImageFetcherBridge, long startTimeMillis);
 }

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

@@ -21,7 +21,7 @@ import java.io.File;
 public class CachedImageFetcherImpl implements CachedImageFetcher {
     private static CachedImageFetcherImpl sInstance;
 
-    public static CachedImageFetcher getInstance() {
+    public static CachedImageFetcherImpl getInstance() {
         ThreadUtils.assertOnUiThread();
 
         if (sInstance == null) {
@@ -53,6 +53,11 @@ public class CachedImageFetcherImpl implements CachedImageFetcher {
         mCachedImageFetcherBridge = bridge;
     }
 
+    @Override
+    public void reportEvent(@CachedImageFetcherEvent int eventId) {
+        mCachedImageFetcherBridge.reportEvent(eventId);
+    }
+
     @Override
     public void destroy() {
         // Do nothing, this lives for the lifetime of the application.
@@ -79,6 +84,7 @@ public class CachedImageFetcherImpl implements CachedImageFetcher {
      */
     @VisibleForTesting
     void fetchImageImpl(String url, int width, int height, Callback<Bitmap> callback) {
+        long startTimeMillis = System.currentTimeMillis();
         String filePath = mCachedImageFetcherBridge.getFilePath(url);
         new AsyncTask<Bitmap>() {
             @Override
@@ -90,10 +96,11 @@ public class CachedImageFetcherImpl implements CachedImageFetcher {
             protected void onPostExecute(Bitmap bitmap) {
                 if (bitmap != null) {
                     callback.onResult(bitmap);
-                    return;
+                    reportEvent(CachedImageFetcherEvent.JAVA_DISK_CACHE_HIT);
+                    mCachedImageFetcherBridge.reportCacheHitTime(startTimeMillis);
+                } else {
+                    mCachedImageFetcherBridge.fetchImage(url, width, height, callback);
                 }
-
-                mCachedImageFetcherBridge.fetchImage(url, width, height, callback);
             }
         }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
     }

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

@@ -14,12 +14,13 @@ import org.chromium.chrome.browser.BitmapCache;
 import org.chromium.chrome.browser.util.ConversionUtils;
 
 /**
- * A wrapper around the static CachedImageFetcher that also provides in-memory cahcing.
+ * A wrapper around the CachedImageFetcher that also provides in-memory caching.
  */
 public class InMemoryCachedImageFetcher implements CachedImageFetcher {
     private static final int DEFAULT_CACHE_SIZE = 20 * ConversionUtils.BYTES_PER_MEGABYTE; // 20mb
     private static final float PORTION_OF_AVAILABLE_MEMORY = 1.f / 8.f;
 
+    // Will do the work if the image isn't cached in memory.
     private CachedImageFetcher mCachedImageFetcher;
     private BitmapCache mBitmapCache;
 
@@ -45,6 +46,11 @@ public class InMemoryCachedImageFetcher implements CachedImageFetcher {
         mCachedImageFetcher = CachedImageFetcher.getInstance();
     }
 
+    @Override
+    public void reportEvent(@CachedImageFetcherEvent int eventId) {
+        mCachedImageFetcher.reportEvent(eventId);
+    }
+
     @Override
     public void destroy() {
         mCachedImageFetcher.destroy();
@@ -69,6 +75,7 @@ public class InMemoryCachedImageFetcher implements CachedImageFetcher {
                 callback.onResult(bitmap);
             });
         } else {
+            reportEvent(CachedImageFetcherEvent.JAVA_IN_MEMORY_CACHE_HIT);
             callback.onResult(cachedBitmap);
         }
     }

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

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.cached_image_fetcher;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyObject;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
@@ -84,6 +85,11 @@ public class CachedImageFetcherImplTest {
         verify(mCachedImageFetcher).fetchImageImpl(eq(URL), eq(WIDTH_PX), eq(HEIGHT_PX), any());
         verify(mCachedImageFetcherBridge, never()) // Should never make it to native.
                 .fetchImage(eq(URL), anyInt(), anyInt(), any());
+
+        // Verify metrics have been reported.
+        verify(mCachedImageFetcherBridge)
+                .reportEvent(eq(CachedImageFetcherEvent.JAVA_DISK_CACHE_HIT));
+        verify(mCachedImageFetcherBridge).reportCacheHitTime(anyLong());
     }
 
     @Test

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

@@ -117,6 +117,10 @@ public class InMemoryCachedImageFetcherTest {
 
         verify(mCachedImageFetcherImpl, /* Should only go to native the first time. */ times(1))
                 .fetchImage(eq(URL), eq(WIDTH_PX), eq(HEIGHT_PX), any());
+
+        // Verify metrics are reported.
+        verify(mCachedImageFetcherImpl)
+                .reportEvent(CachedImageFetcherEvent.JAVA_IN_MEMORY_CACHE_HIT);
     }
 
     @Test

chrome/browser/android/cached_image_fetcher/cached_image_fetcher_bridge.cc

@@ -115,6 +115,23 @@ void CachedImageFetcherBridge::FetchImage(JNIEnv* j_env,
       kTrafficAnnotation);
 }
 
+void CachedImageFetcherBridge::ReportEvent(
+    JNIEnv* j_env,
+    const base::android::JavaRef<jobject>& j_this,
+    const jint j_event_id) {
+  CachedImageFetcherEvent event =
+      static_cast<CachedImageFetcherEvent>(j_event_id);
+  CachedImageFetcherMetricsReporter::ReportEvent(event);
+}
+
+void CachedImageFetcherBridge::ReportCacheHitTime(
+    JNIEnv* j_env,
+    const base::android::JavaRef<jobject>& j_this,
+    const jlong start_time_millis) {
+  base::Time start_time = base::Time::FromJavaTime(start_time_millis);
+  CachedImageFetcherMetricsReporter::ReportImageLoadFromCacheTime(start_time);
+}
+
 void CachedImageFetcherBridge::OnImageFetched(
     base::android::ScopedJavaGlobalRef<jobject> callback,
     const std::string& id,

chrome/browser/android/cached_image_fetcher/cached_image_fetcher_bridge.h

@@ -41,6 +41,14 @@ class CachedImageFetcherBridge {
                   const jint height_px,
                   const base::android::JavaRef<jobject>& j_callback);
 
+  void ReportEvent(JNIEnv* j_env,
+                   const base::android::JavaRef<jobject>& j_this,
+                   const jint j_event_id);
+
+  void ReportCacheHitTime(JNIEnv* j_env,
+                          const base::android::JavaRef<jobject>& j_this,
+                          const jlong start_time_millis);
+
  private:
   void OnImageFetched(base::android::ScopedJavaGlobalRef<jobject> callback,
                       const std::string& id,

components/image_fetcher/core/BUILD.gn

@@ -2,6 +2,10 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+if (is_android) {
+  import("//build/config/android/rules.gni")
+}
+
 static_library("core") {
   sources = [
     "cached_image_fetcher.cc",
@@ -75,3 +79,11 @@ source_set("unit_tests") {
     "//ui/gfx:test_support",
   ]
 }
+
+if (is_android) {
+  java_cpp_enum("cached_image_fetcher_java_enums_srcjar") {
+    sources = [
+      "cache/cached_image_fetcher_metrics_reporter.h",
+    ]
+  }
+}

components/image_fetcher/core/cache/cached_image_fetcher_metrics_reporter.h

@@ -12,6 +12,8 @@ namespace image_fetcher {
 // Enum for the result of the fetch, reported through UMA. Present in enums.xml
 // as CachedImageFetcherEvent. New values should be added at the end and things
 // should not be renumbered.
+// GENERATED_JAVA_ENUM_PACKAGE: (
+// org.chromium.chrome.browser.cached_image_fetcher)
 enum class CachedImageFetcherEvent {
   kImageRequest = 0,
   kCacheHit = 1,
@@ -21,7 +23,9 @@ enum class CachedImageFetcherEvent {
   kTotalFailure = 5,
   kCacheStartupEvictionStarted = 6,
   kCacheStartupEvictionFinished = 7,
-  kMaxValue = kCacheStartupEvictionFinished,
+  kJavaInMemoryCacheHit = 8,
+  kJavaDiskCacheHit = 9,
+  kMaxValue = kJavaDiskCacheHit,
 };
 
 class CachedImageFetcherMetricsReporter {
@@ -29,10 +33,17 @@ class CachedImageFetcherMetricsReporter {
   // Report cache events, used by CachedImageFetcher and composing classes.
   static void ReportEvent(CachedImageFetcherEvent event);
 
-  // Report timing for various Cache events related to CachedImageFetcher.
+  // Report the time it takes to load an image from the cache in native code.
   static void ReportImageLoadFromCacheTime(base::Time start_time);
+
+  // Report the time it takes to load an image from the network.
   static void ReportImageLoadFromNetworkTime(base::Time start_time);
+
+  // Report the time it takes to load an image from the network after a cache
+  // hit.
   static void ReportImageLoadFromNetworkAfterCacheHit(base::Time start_time);
+
+  // Report the time between cache evictions.
   static void ReportTimeSinceLastCacheLRUEviction(base::Time start_time);
 };
 

tools/metrics/histograms/enums.xml

@@ -5484,6 +5484,10 @@ uploading your change for review.  These are checked by presubmit scripts.
   <int value="3" label="Cache decoding error"/>
   <int value="4" label="Network transcoding error"/>
   <int value="5" label="Failure to fetch the image from the network"/>
+  <int value="6" label="Cache startup eviction began"/>
+  <int value="7" label="Cache startup eviction finished"/>
+  <int value="8" label="Java in-memory cache hit"/>
+  <int value="9" label="Java disk cache hit"/>
 </enum>
 
 <enum name="CacheResult">