customtabs: UMA histogram tracking whether clients call warmup().

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.customtabs;
 
 import android.app.ActivityManager;
 import android.app.Application;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.content.ServiceConnection;
 import android.content.pm.PackageManager;
 import android.content.res.Resources;
 import android.graphics.Point;
 import android.net.ConnectivityManager;
 import android.net.Uri;
 import android.os.AsyncTask;
 import android.os.Binder;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.IBinder;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.SystemClock;
 import android.support.customtabs.ICustomTabsCallback;
 import android.support.customtabs.ICustomTabsService;
 import android.text.TextUtils;
 import android.util.SparseArray;
+import android.util.SparseBooleanArray;
 import android.view.WindowManager;
 
 import org.chromium.base.FieldTrialList;
 import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.ProcessInitException;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
@@ skipping 32 matching lines @@
     private static final String TAG = "cr.ChromeConnection";
     @VisibleForTesting
     static final String NO_PRERENDERING_KEY =
             "android.support.customtabs.maylaunchurl.NO_PRERENDERING";
 
     // Values for the "CustomTabs.PredictionStatus" UMA histogram. Append-only.
     private static final int NO_PREDICTION = 0;
     private static final int GOOD_PREDICTION = 1;
     private static final int BAD_PREDICTION = 2;
     private static final int PREDICTION_STATUS_COUNT = 3;
+    // Values for the "CustomTabs.CalledWarmup" UMA histogram. Append-only.

[pasko, 2015/09/03 18:02:19]

nittynit: empty line above would look nicer

[Benoit L, 2015/09/29 09:04:23]

Ack.

+    private static final int NO_SESSION_NO_WARMUP = 0;
+    private static final int NO_SESSION_WARMUP = 1;
+    private static final int SESSION_NO_WARMUP_ALREADY_CALLED = 2;
+    private static final int SESSION_NO_WARMUP_NOT_CALLED = 3;
+    private static final int SESSION_WARMUP = 4;
+    private static final int SESSION_WARMUP_COUNT = 5;
 
     private static AtomicReference<CustomTabsConnection> sInstance =
             new AtomicReference<CustomTabsConnection>();
 
     private static final class PrerenderedUrlParams {
         public final IBinder mSession;
         public final WebContents mWebContents;
         public final String mUrl;
         public final String mReferrer;
         public final Bundle mExtras;
@@ skipping 92 matching lines @@
 
         public String getPredictedUrl() {
             return mPredictedUrl;
         }
 
         public long getLastMayLaunchUrlTimestamp() {
             return mLastMayLaunchUrlTimestamp;
         }
     }
 
     private final Object mLock = new Object();

[pasko, 2015/09/03 18:02:20]

please add a comment saying something like: 'mLock protects all the members
below'.

Locking is a bit complex in this class. How can we simplify reading it?

I think it will be cleaner to create a private inner class to keep this state,
remove all other synchronized blocks and make the class thread-safe.

Something like: SessionManager {
newSession(...)
updateForMayLaunchUrl(...)
evaluatePredictionOnLaunch(...)
getCallbackForSession(...)
...
// not sure what to do about keepalive, for example: if it runs on an arbitrary
thread, why is there no race when accessing mApplication?

We could create another class to manage the UI thread data. And this class would
just manage sending data to appropriate threads.

WDYT?

[Benoit L, 2015/09/29 09:04:23]

Refactored this, so... done?

     private final Map<IBinder, SessionParams> mSessionParams = new HashMap<>();
     // Prediction tracking is done by UID and not by session, since a
     // mis-behaving application can create a large number of sessions.
     private SparseArray<PredictionStats> mUidToPredictionsStats = new SparseArray<>();
+    // Whether a client application has called warmup() in this instance lifetime.
+    private SparseBooleanArray mUidHasCalledWarmup = new SparseBooleanArray();
 
     /**
      * <strong>DO NOT CALL</strong>
      * Public to be instanciable from {@link ChromeApplication}. This is however
      * intended to be private.
      */
     public CustomTabsConnection(Application application) {
         super();
         mApplication = application;
     }
@@ skipping 45 matching lines @@
                 mUidToPredictionsStats.put(uid, new PredictionStats());
             }
         }
         return true;
     }
 
     @Override
     public boolean warmup(long flags) {
         // Here and in mayLaunchUrl(), don't do expensive work for background applications.
         if (!isCallerForegroundOrSelf()) return false;
+
+        synchronized (mLock) {
+            mUidHasCalledWarmup.put(Binder.getCallingUid(), true);
+        }
+
         if (!mWarmupHasBeenCalled.compareAndSet(false, true)) return true;
         // The call is non-blocking and this must execute on the UI thread, post a task.
         ThreadUtils.postOnUiThread(new Runnable() {
             @Override
             @SuppressFBWarnings("DM_EXIT")
             public void run() {
                 ChromeApplication app = (ChromeApplication) mApplication;
                 try {
                     app.startBrowserProcessesAndLoadLibrariesSync(true);
                 } catch (ProcessInitException e) {
@@ skipping 103 matching lines @@
     }
 
     /**
      * Registers a launch of a |url| for a given |session|.
      *
      * This is used for accounting.
      */
     void registerLaunch(IBinder session, String url) {
         int outcome;
         long elapsedTimeMs = -1;
+        boolean hasValidSession = false;
+        boolean hasUidCalledWarmup = false;
         synchronized (mLock) {
             SessionParams sessionParams = mSessionParams.get(session);
             if (sessionParams == null) {
                 outcome = NO_PREDICTION;
             } else {
+                hasValidSession = true;
+                hasUidCalledWarmup = mUidHasCalledWarmup.get(sessionParams.mUid);
+
                 String predictedUrl = sessionParams.getPredictedUrl();
                 outcome = predictedUrl == null ? NO_PREDICTION
                         : predictedUrl.equals(url) ? GOOD_PREDICTION : BAD_PREDICTION;
                 elapsedTimeMs = SystemClock.elapsedRealtime()
                         - sessionParams.getLastMayLaunchUrlTimestamp();
                 sessionParams.setPredictionMetrics(null, 0);
                 if (outcome == GOOD_PREDICTION) {
                     // If the prediction was correct, back to the smallest
                     // throttling level.
                     mUidToPredictionsStats.put(sessionParams.mUid, new PredictionStats());
                 }
             }
         }
         RecordHistogram.recordEnumeratedHistogram(
                 "CustomTabs.PredictionStatus", outcome, PREDICTION_STATUS_COUNT);
         if (outcome == GOOD_PREDICTION) {
             RecordHistogram.recordCustomTimesHistogram("CustomTabs.PredictionToLaunch",
                     elapsedTimeMs, 1, TimeUnit.MINUTES.toMillis(3), TimeUnit.MILLISECONDS, 100);
         }
+        // CustomTabs.CalledWarmup histogram.
+        boolean warmupCalled = mWarmupHasBeenCalled.get();
+        int warmupKind = warmupCalled ? NO_SESSION_WARMUP : NO_SESSION_NO_WARMUP;
+        if (hasValidSession) {
+            if (hasUidCalledWarmup) {
+                warmupKind = SESSION_WARMUP;
+            } else {
+                warmupKind = warmupCalled ? SESSION_NO_WARMUP_ALREADY_CALLED
+                                          : SESSION_NO_WARMUP_NOT_CALLED;
+            }
+        }
+        RecordHistogram.recordEnumeratedHistogram(
+                "CustomTabs.CalledWarmup", warmupKind, SESSION_WARMUP_COUNT);
     }
 
     /**
      * Transfers a prerendered WebContents if one exists.
      *
      * This resets the internal WebContents; a subsequent call to this method
      * returns null. Must be called from the UI thread.
      * If a prerender exists for a different URL with the same sessionId or with
      * a different referrer, then this is treated as a mispredict from the
      * client application, and cancels the previous prerender. This is done to
@@ skipping 209 matching lines @@
     }
 
     /**
      * Called when a remote client has died.
      */
     private void cleanupAlreadyLocked(IBinder session) {
         ThreadUtils.assertOnUiThread();
         SessionParams params = mSessionParams.get(session);
         if (params == null) return;
         mSessionParams.remove(session);
+        mUidHasCalledWarmup.delete(params.mUid);
         IBinder binder = params.mCallback.asBinder();
         binder.unlinkToDeath(params.mDeathRecipient, 0);
         if (mPrerender != null && session.equals(mPrerender.mSession)) {
             prerenderUrl(session, null, null); // Cancels the pre-render.
         }
     }
 
     private boolean mayPrerender() {
         if (FieldTrialList.findFullName("CustomTabs").equals("DisablePrerender")) return false;
         if (!DeviceClassManager.enablePrerendering()) return false;
@@ skipping 67 matching lines @@
         return screenSize;
     }
 
     @VisibleForTesting
     void resetThrottling(int uid) {
         synchronized (mLock) {
             mUidToPredictionsStats.put(uid, new PredictionStats());
         }
     }
 }