Photo Picker dialog: Add UMA statistics.

chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java

Index: chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java
index ae83641d55fa713814cf056836969bc6a8377405..a2722f2d8dab26ece418148b5dc7feb52520d865 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java
@@ -17,8 +17,10 @@ import android.os.Messenger;
 import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.os.StrictMode;
+import android.os.SystemClock;
 
 import org.chromium.base.Log;
+import org.chromium.base.metrics.RecordHistogram;
 
 import java.io.File;
 import java.io.FileDescriptor;
@@ -26,6 +28,7 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.lang.ref.WeakReference;
 import java.util.LinkedHashMap;
+import java.util.concurrent.TimeUnit;
 
 /**
  * A class to communicate with the {@link DecoderService}.
@@ -93,6 +96,9 @@ public class DecoderServiceHost {
         // The callback to use to communicate the results of the decoding.
         ImageDecodedCallback mCallback;
 
+        // The timestamp for when the request is being sent over for decoding.

[Theresa, 2017/05/31 18:40:02]

nit: ...when the request was sent..

[Finnur, 2017/06/01 15:13:55]

Done.

+        long mTimestamp;
+
         public DecoderServiceParams(String filePath, int size, ImageDecodedCallback callback) {
             mFilePath = filePath;
             mSize = size;
@@ -169,6 +175,7 @@ public class DecoderServiceHost {
     private void dispatchNextDecodeImageRequest() {
         if (mRequests.entrySet().iterator().hasNext()) {
             DecoderServiceParams params = mRequests.entrySet().iterator().next().getValue();
+            params.mTimestamp = SystemClock.elapsedRealtime();
             dispatchDecodeImageRequest(params.mFilePath, params.mSize);
         }
     }
@@ -179,11 +186,21 @@ public class DecoderServiceHost {
      * the request queue).
      * @param filePath The path to the image that was just decoded.
      * @param bitmap The resulting decoded bitmap.
+     * @param decodeTime The length of time it took to decode the bitmap.
      */
-    public void closeRequest(String filePath, Bitmap bitmap) {
+    public void closeRequest(String filePath, Bitmap bitmap, long decodeTime) {
         DecoderServiceParams params = getRequests().get(filePath);
         if (params != null) {
+            long endRpcCall = SystemClock.elapsedRealtime();
+            RecordHistogram.recordTimesHistogram("Android.PhotoPicker.RequestProcessTime",
+                    endRpcCall - params.mTimestamp, TimeUnit.MILLISECONDS);
+
             params.mCallback.imageDecodedCallback(filePath, bitmap);
+
+            if (decodeTime != -1) {
+                RecordHistogram.recordTimesHistogram(
+                        "Android.PhotoPicker.ImageDecodeTime", decodeTime, TimeUnit.MILLISECONDS);
+            }

[Theresa, 2017/05/31 18:40:02]

It would also be interesting to record the size of the bitmap so that we can get
an idea of memory usage per bitmap and the number of images decoded (maybe in
the initial request after we have the file list).

[Finnur, 2017/06/01 15:13:55]

I've added metrics for CacheHits for the high-res photos as well as a counter
for the decode requests. I think it is better to keep track of that over the
lifetime of the dialog, instead of at some point in the middle, because we want
one number to be reported for each stat. Specifically I'm trying to avoid this
scenario:

1. Open dialog (UMA reports 16 decode requests, 0 cache hits).
2. Scroll down (UMA reports 16 decode requests, 0 cache hits).
3. Scroll up   (UMA reports 0 decode requests, 16 cache hits).
4. Close dialog

Probably better to just send one stat at step 4 (32 decode requests, 16 cache
hits).

             getRequests().remove(filePath);
         }
         dispatchNextDecodeImageRequest();
@@ -212,7 +229,7 @@ public class DecoderServiceHost {
                 bundle.putParcelable(DecoderService.KEY_FILE_DESCRIPTOR, pfd);
             } catch (IOException e) {
                 Log.e(TAG, "Unable to obtain FileDescriptor: " + e);
-                closeRequest(filePath, null);
+                closeRequest(filePath, null, -1);
             }
         } finally {
             try {
@@ -236,10 +253,10 @@ public class DecoderServiceHost {
             pfd.close();
         } catch (RemoteException e) {
             Log.e(TAG, "Communications failed (Remote): " + e);
-            closeRequest(filePath, null);
+            closeRequest(filePath, null, -1);
         } catch (IOException e) {
             Log.e(TAG, "Communications failed (IO): " + e);
-            closeRequest(filePath, null);
+            closeRequest(filePath, null, -1);
         }
     }
 
@@ -284,7 +301,8 @@ public class DecoderServiceHost {
                     Bitmap bitmap = success
                             ? (Bitmap) payload.getParcelable(DecoderService.KEY_IMAGE_BITMAP)
                             : null;
-                    host.closeRequest(filePath, bitmap);
+                    long decodeTime = payload.getLong(DecoderService.KEY_DECODE_TIME);
+                    host.closeRequest(filePath, bitmap, decodeTime);
                     break;
                 default:
                     super.handleMessage(msg);