AW UMA: extend and deprecate variations histograms

This extends two histograms (and changes ownership) and marks two others
as obsolete (removing the relevant code).

Fixed: 1160837
Test: run_webview_instrumentation_test_apk -f "*Variations*"
Change-Id: I97267f2b2c81bebfb5cd8b572f0b0cdb1ecb3d84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2639168Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846247}

android_webview/java/src/org/chromium/android_webview/common/variations/VariationsServiceMetricsHelper.java

@@ -22,10 +22,6 @@ public class VariationsServiceMetricsHelper {
     // The time in milliseconds between scheduling and executing the last seed download job.
     private static final String JOB_QUEUE_TIME = "job_queue_time";
     // The result of the last seed download.
-    // See the VariationsSeedFetchResult variations enum for valid values.
-    private static final String SEED_FETCH_RESULT = "seed_fetch_result";
-    // The duration in milliseconds of the last seed download.
-    private static final String SEED_FETCH_TIME = "seed_fetch_time";
 
     // Name of a SharedPreferences pref that stores the time in milliseconds since UNIX epoch of
     // when the most recent seed download job was scheduled.
@@ -62,12 +58,6 @@ public class VariationsServiceMetricsHelper {
         if (prefs.contains(LAST_JOB_START_TIME)) {
             bundle.putLong(LAST_JOB_START_TIME, prefs.getLong(LAST_JOB_START_TIME, 0));
         }
-        if (prefs.contains(SEED_FETCH_RESULT)) {
-            bundle.putInt(SEED_FETCH_RESULT, prefs.getInt(SEED_FETCH_RESULT, 0));
-        }
-        if (prefs.contains(SEED_FETCH_TIME)) {
-            bundle.putLong(SEED_FETCH_TIME, prefs.getLong(SEED_FETCH_TIME, 0));
-        }
         return new VariationsServiceMetricsHelper(bundle);
     }
 
@@ -96,12 +86,6 @@ public class VariationsServiceMetricsHelper {
         if (hasLastJobStartTime()) {
             prefsEditor.putLong(LAST_JOB_START_TIME, getLastJobStartTime());
         }
-        if (hasSeedFetchResult()) {
-            prefsEditor.putInt(SEED_FETCH_RESULT, getSeedFetchResult());
-        }
-        if (hasSeedFetchTime()) {
-            prefsEditor.putLong(SEED_FETCH_TIME, getSeedFetchTime());
-        }
         return prefsEditor.commit();
     }
 
@@ -157,32 +141,6 @@ public class VariationsServiceMetricsHelper {
         return mBundle.getLong(LAST_JOB_START_TIME);
     }
 
-    public void clearSeedFetchResult() {
-        mBundle.remove(SEED_FETCH_RESULT);
-    }
-    public void setSeedFetchResult(int seedFetchResult) {
-        mBundle.putInt(SEED_FETCH_RESULT, seedFetchResult);
-    }
-    public boolean hasSeedFetchResult() {
-        return mBundle.containsKey(SEED_FETCH_RESULT);
-    }
-    public int getSeedFetchResult() {
-        return mBundle.getInt(SEED_FETCH_RESULT);
-    }
-
-    public void clearSeedFetchTime() {
-        mBundle.remove(SEED_FETCH_TIME);
-    }
-    public void setSeedFetchTime(long seedFetchTime) {
-        mBundle.putLong(SEED_FETCH_TIME, seedFetchTime);
-    }
-    public boolean hasSeedFetchTime() {
-        return mBundle.containsKey(SEED_FETCH_TIME);
-    }
-    public long getSeedFetchTime() {
-        return mBundle.getLong(SEED_FETCH_TIME);
-    }
-
     private VariationsServiceMetricsHelper(Bundle bundle) {
         mBundle = bundle;
     }

android_webview/java/src/org/chromium/android_webview/variations/VariationsSeedLoader.java

@@ -89,12 +89,6 @@ public class VariationsSeedLoader {
     @VisibleForTesting
     public static final String APP_SEED_FRESHNESS_HISTOGRAM_NAME = "Variations.AppSeedFreshness";
     @VisibleForTesting
-    public static final String DOWNLOAD_JOB_FETCH_RESULT_HISTOGRAM_NAME =
-            "Variations.WebViewDownloadJobFetchResult";
-    @VisibleForTesting
-    public static final String DOWNLOAD_JOB_FETCH_TIME_HISTOGRAM_NAME =
-            "Variations.WebViewDownloadJobFetchTime2";
-    @VisibleForTesting
     public static final String DOWNLOAD_JOB_INTERVAL_HISTOGRAM_NAME =
             "Variations.WebViewDownloadJobInterval";
     @VisibleForTesting
@@ -308,19 +302,6 @@ public class VariationsSeedLoader {
                         TimeUnit.MILLISECONDS.toMinutes(metrics.getJobQueueTime()),
                         TimeUnit.DAYS.toMinutes(30));
             }
-            if (metrics.hasSeedFetchResult()) {
-                // This metric stores the same enum as Variations.FirstRun.SeedFetchResult, but is
-                // used for all WebView seed requests rather than just the first-run request.
-                RecordHistogram.recordSparseHistogram(
-                        DOWNLOAD_JOB_FETCH_RESULT_HISTOGRAM_NAME, metrics.getSeedFetchResult());
-            }
-            if (metrics.hasSeedFetchTime()) {
-                // Newer versions of Android limit job execution time to 10 minutes. Set the max
-                // histogram bucket to double that to have some wiggle room.
-                RecordHistogram.recordCustomTimesHistogram(DOWNLOAD_JOB_FETCH_TIME_HISTOGRAM_NAME,
-                        metrics.getSeedFetchTime(), 100, TimeUnit.MINUTES.toMillis(20),
-                        50); // 50 buckets from 100ms to 20min
-            }
         }
     }
 

android_webview/javatests/src/org/chromium/android_webview/test/AwVariationsSeedFetcherTest.java

@@ -419,8 +419,6 @@ public class AwVariationsSeedFetcherTest {
 
             VariationsServiceMetricsHelper metrics =
                     VariationsServiceMetricsHelper.fromVariationsSharedPreferences(mContext);
-            Assert.assertEquals(HTTP_NOT_FOUND, metrics.getSeedFetchResult());
-            Assert.assertEquals(DOWNLOAD_DURATION, metrics.getSeedFetchTime());
             Assert.assertEquals(START_TIME, metrics.getLastJobStartTime());
             Assert.assertFalse(metrics.hasLastEnqueueTime());
             Assert.assertFalse(metrics.hasJobInterval());
@@ -455,8 +453,6 @@ public class AwVariationsSeedFetcherTest {
 
             VariationsServiceMetricsHelper metrics =
                     VariationsServiceMetricsHelper.fromVariationsSharedPreferences(mContext);
-            Assert.assertEquals(HTTP_NOT_FOUND, metrics.getSeedFetchResult());
-            Assert.assertEquals(DOWNLOAD_DURATION, metrics.getSeedFetchTime());
             Assert.assertEquals(START_TIME + JOB_DELAY, metrics.getLastJobStartTime());
             Assert.assertEquals(JOB_DELAY, metrics.getJobQueueTime());
             Assert.assertFalse(metrics.hasLastEnqueueTime());
@@ -494,8 +490,6 @@ public class AwVariationsSeedFetcherTest {
 
             VariationsServiceMetricsHelper metrics =
                     VariationsServiceMetricsHelper.fromVariationsSharedPreferences(mContext);
-            Assert.assertEquals(HTTP_NOT_FOUND, metrics.getSeedFetchResult());
-            Assert.assertEquals(DOWNLOAD_DURATION, metrics.getSeedFetchTime());
             Assert.assertEquals(
                     START_TIME + appRunDelay + JOB_DELAY, metrics.getLastJobStartTime());
             Assert.assertEquals(appRunDelay + JOB_DELAY, metrics.getJobInterval());

android_webview/javatests/src/org/chromium/android_webview/test/VariationsSeedLoaderTest.java

@@ -403,8 +403,6 @@ public class VariationsSeedLoaderTest {
                     VariationsServiceMetricsHelper.fromBundle(new Bundle());
             metrics.setJobInterval(threeWeeksMs);
             metrics.setJobQueueTime(twoWeeksMs);
-            metrics.setSeedFetchResult(200);
-            metrics.setSeedFetchTime(nineMinutesMs);
             MockVariationsSeedServer.setMetricsBundle(metrics.toBundle());
 
             runTestLoaderBlocking();
@@ -414,11 +412,6 @@ public class VariationsSeedLoaderTest {
             assertSingleRecordInHistogram(
                     VariationsSeedLoader.DOWNLOAD_JOB_QUEUE_TIME_HISTOGRAM_NAME,
                     (int) TimeUnit.MILLISECONDS.toMinutes(twoWeeksMs));
-            assertSingleRecordInHistogram(
-                    VariationsSeedLoader.DOWNLOAD_JOB_FETCH_RESULT_HISTOGRAM_NAME, 200);
-            assertSingleRecordInHistogram(
-                    VariationsSeedLoader.DOWNLOAD_JOB_FETCH_TIME_HISTOGRAM_NAME,
-                    (int) nineMinutesMs);
         } finally {
             MockVariationsSeedServer.setMetricsBundle(null);
         }

android_webview/javatests/src/org/chromium/android_webview/test/services/VariationsSeedServerTest.java

@@ -93,8 +93,6 @@ public class VariationsSeedServerTest {
         Context context = ContextUtils.getApplicationContext();
         VariationsServiceMetricsHelper initialMetrics =
                 VariationsServiceMetricsHelper.fromBundle(new Bundle());
-        initialMetrics.setSeedFetchResult(200); // HTTP_OK
-        initialMetrics.setSeedFetchTime(50);
         initialMetrics.setJobInterval(6000);
         initialMetrics.setJobQueueTime(1000);
         initialMetrics.setLastEnqueueTime(4);
@@ -113,8 +111,6 @@ public class VariationsSeedServerTest {
                 "Timed out waiting for reportSeedMetrics() to be called", 0);
         VariationsServiceMetricsHelper metrics =
                 VariationsServiceMetricsHelper.fromBundle(callback.metrics);
-        Assert.assertEquals(200, metrics.getSeedFetchResult());
-        Assert.assertEquals(50, metrics.getSeedFetchTime());
         Assert.assertEquals(6000, metrics.getJobInterval());
         Assert.assertEquals(1000, metrics.getJobQueueTime());
     }

android_webview/nonembedded/java/src/org/chromium/android_webview/services/AwVariationsSeedFetcher.java

@@ -176,7 +176,7 @@ public class AwVariationsSeedFetcher extends JobService {
                         VariationsSeedFetcher.VariationsPlatform.ANDROID_WEBVIEW,
                         /*restrictMode=*/null, milestone, getChannelStr());
 
-                saveMetrics(fetchInfo.seedFetchResult, startTime, /*endTime=*/currentTimeMillis());
+                saveMetrics(startTime, /*endTime=*/currentTimeMillis());
 
                 if (isCancelled()) {
                     return null;
@@ -202,12 +202,10 @@ public class AwVariationsSeedFetcher extends JobService {
             return null;
         }
 
-        private void saveMetrics(int seedFetchResult, long startTime, long endTime) {
+        private void saveMetrics(long startTime, long endTime) {
             Context context = ContextUtils.getApplicationContext();
             VariationsServiceMetricsHelper metrics =
                     VariationsServiceMetricsHelper.fromVariationsSharedPreferences(context);
-            metrics.setSeedFetchResult(seedFetchResult);
-            metrics.setSeedFetchTime(endTime - startTime);
             if (metrics.hasLastEnqueueTime()) {
                 metrics.setJobQueueTime(startTime - metrics.getLastEnqueueTime());
             }

android_webview/nonembedded/java/src/org/chromium/android_webview/services/VariationsSeedServer.java

@@ -62,8 +62,6 @@ public class VariationsSeedServer extends Service {
         // reported a second time.
         metrics.clearJobInterval();
         metrics.clearJobQueueTime();
-        metrics.clearSeedFetchResult();
-        metrics.clearSeedFetchTime();
         if (!metrics.writeMetricsToVariationsSharedPreferences(context)) {
             Log.e(TAG, "Failed to write variations SharedPreferences to disk");
         }

tools/metrics/histograms/histograms_xml/variations/histograms.xml

@@ -541,6 +541,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 
 <histogram name="Variations.WebViewDownloadJobFetchResult"
     enum="VariationsSeedFetchResult" expires_after="2021-01-31">
+  <obsolete>
+    Removed from code January 2021.
+  </obsolete>
   <owner>rmcelrath@chromium.org</owner>
   <owner>src/android_webview/OWNERS</owner>
   <summary>
@@ -551,6 +554,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 
 <histogram name="Variations.WebViewDownloadJobFetchTime2" units="ms"
     expires_after="2021-01-31">
+  <obsolete>
+    Removed from code January 2021.
+  </obsolete>
   <owner>rmcelrath@chromium.org</owner>
   <owner>src/android_webview/OWNERS</owner>
   <summary>
@@ -560,8 +566,8 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 </histogram>
 
 <histogram name="Variations.WebViewDownloadJobInterval" units="minutes"
-    expires_after="2021-01-31">
-  <owner>rmcelrath@chromium.org</owner>
+    expires_after="2022-01-20">
+  <owner>ntfschr@chromium.org</owner>
   <owner>src/android_webview/OWNERS</owner>
   <summary>
     The time between the start of two consecutive WebView seed downloads by
@@ -571,8 +577,8 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 </histogram>
 
 <histogram name="Variations.WebViewDownloadJobQueueTime" units="minutes"
-    expires_after="2021-01-31">
-  <owner>rmcelrath@chromium.org</owner>
+    expires_after="2022-01-20">
+  <owner>ntfschr@chromium.org</owner>
   <owner>src/android_webview/OWNERS</owner>
   <summary>
     The delay between when a WebView seed download was scheduled by the service