diff --git a/android_webview/java/src/org/chromium/android_webview/variations/VariationsSeedLoader.java b/android_webview/java/src/org/chromium/android_webview/variations/VariationsSeedLoader.java index fe0ea08425e386..dcd93c9bfa33bf 100644 --- a/android_webview/java/src/org/chromium/android_webview/variations/VariationsSeedLoader.java +++ b/android_webview/java/src/org/chromium/android_webview/variations/VariationsSeedLoader.java @@ -15,7 +15,6 @@ import android.os.RemoteException; import android.os.SystemClock; -import androidx.annotation.IntDef; import androidx.annotation.VisibleForTesting; import org.chromium.android_webview.AwBrowserProcess; @@ -35,8 +34,6 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; import java.util.Date; import java.util.concurrent.ExecutionException; import java.util.concurrent.FutureTask; @@ -92,9 +89,6 @@ public class VariationsSeedLoader { @VisibleForTesting public static final String APP_SEED_FRESHNESS_HISTOGRAM_NAME = "Variations.AppSeedFreshness"; @VisibleForTesting - public static final String APP_SEED_REQUEST_STATE_HISTOGRAM_NAME = - "Variations.AppSeedRequestState"; - @VisibleForTesting public static final String DOWNLOAD_JOB_FETCH_RESULT_HISTOGRAM_NAME = "Variations.WebViewDownloadJobFetchResult"; @VisibleForTesting @@ -114,23 +108,6 @@ public class VariationsSeedLoader { private SeedLoadAndUpdateRunnable mRunnable; private SeedServerCallback mSeedServerCallback = new SeedServerCallback(); - // UMA histogram values for the result of checking if the app needs a new variations seed. - // Keep in sync with AppSeedRequestState enum in enums.xml. - // - // These values are persisted to logs. Entries should not be renumbered and - // numeric values should never be reused. - @IntDef({AppSeedRequestState.UNKNOWN, AppSeedRequestState.SEED_FRESH, - AppSeedRequestState.SEED_REQUESTED, AppSeedRequestState.SEED_REQUEST_THROTTLED}) - @Retention(RetentionPolicy.SOURCE) - @VisibleForTesting - public @interface AppSeedRequestState { - int UNKNOWN = 0; - int SEED_FRESH = 1; - int SEED_REQUESTED = 2; - int SEED_REQUEST_THROTTLED = 3; - int NUM_ENTRIES = 4; - } - private static void recordLoadSeedResult(@LoadSeedResult int result) { RecordHistogram.recordEnumeratedHistogram( SEED_LOAD_RESULT_HISTOGRAM_NAME, result, LoadSeedResult.ENUM_SIZE); @@ -140,11 +117,6 @@ private static void recordSeedLoadBlockingTime(long timeMs) { RecordHistogram.recordTimesHistogram(SEED_LOAD_BLOCKING_TIME_HISTOGRAM_NAME, timeMs); } - private static void recordSeedRequestState(@AppSeedRequestState int state) { - RecordHistogram.recordEnumeratedHistogram( - APP_SEED_REQUEST_STATE_HISTOGRAM_NAME, state, AppSeedRequestState.NUM_ENTRIES); - } - private static void recordAppSeedFreshness(long freshnessMinutes) { // Bucket parameters should match Variations.SeedFreshness. // See variations::RecordSeedFreshness. @@ -185,12 +157,10 @@ private class SeedLoadAndUpdateRunnable implements Runnable { // epoch, or Long.MIN_VALUE if we have no seed. This value originates from the server. // - mSeedFileTime: The time, in milliseconds since the UNIX epoch, our local copy of the // seed was last written to disk as measured by the device's clock. - // - mSeedRequestState: The result of checking if a new seed is required. private boolean mFoundNewSeed; private boolean mNeedNewSeed; private long mCurrentSeedDate = Long.MIN_VALUE; private long mSeedFileTime; - private int mSeedRequestState = AppSeedRequestState.UNKNOWN; private boolean parseSeedFile(File seedFile) { if (!VariationsSeedLoaderJni.get().parseAndSaveSeedProto(seedFile.getPath())) { @@ -222,15 +192,11 @@ private boolean parseSeedFile(File seedFile) { // avoid delaying FutureTask.get().) if (!loadedSeed || isSeedExpired(mSeedFileTime)) { mNeedNewSeed = true; - mSeedRequestState = AppSeedRequestState.SEED_REQUESTED; // Rate-limit the requests. if (shouldThrottleRequests(getCurrentTimeMillis())) { mNeedNewSeed = false; - mSeedRequestState = AppSeedRequestState.SEED_REQUEST_THROTTLED; } - } else { - mSeedRequestState = AppSeedRequestState.SEED_FRESH; } // Save the date field of whatever seed was loaded, if any. @@ -266,7 +232,6 @@ public void run() { public boolean get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { boolean success = mLoadTask.get(timeout, unit); - recordSeedRequestState(mSeedRequestState); if (mSeedFileTime != 0) { long freshnessMinutes = TimeUnit.MILLISECONDS.toMinutes(getCurrentTimeMillis() - mSeedFileTime); diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/VariationsSeedLoaderTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/VariationsSeedLoaderTest.java index b1b4779c53f0ad..4639e310f2d4d4 100644 --- a/android_webview/javatests/src/org/chromium/android_webview/test/VariationsSeedLoaderTest.java +++ b/android_webview/javatests/src/org/chromium/android_webview/test/VariationsSeedLoaderTest.java @@ -164,18 +164,6 @@ private void assertSingleRecordInHistogram(String histogramName, int expectedVal RecordHistogram.getHistogramValueCountForTesting(histogramName, expectedValue * 2)); } - private void assertNoAppSeedRequestStateValues() { - Assert.assertEquals(0, - RecordHistogram.getHistogramTotalCountForTesting( - VariationsSeedLoader.APP_SEED_REQUEST_STATE_HISTOGRAM_NAME)); - } - - private void assertSingleAppSeedRequestStateValue( - @VariationsSeedLoader.AppSeedRequestState int state) { - assertSingleRecordInHistogram( - VariationsSeedLoader.APP_SEED_REQUEST_STATE_HISTOGRAM_NAME, state); - } - // Test the case that: // VariationsUtils.getSeedFile() - doesn't exist // VariationsUtils.getNewSeedFile() - doesn't exist @@ -364,74 +352,6 @@ public void testDoubleLoad() throws Exception { } } - // Test we record the Variations.AppSeedRequestState metric when the seed is fresh. - @Test - @MediumTest - public void testRecordSeedFresh() throws Exception { - File oldFile = VariationsUtils.getSeedFile(); - Assert.assertTrue("Expected seed file to not already exist", oldFile.createNewFile()); - VariationsTestUtils.writeMockSeed(oldFile); - oldFile.setLastModified(CURRENT_TIME_MILLIS); - assertNoAppSeedRequestStateValues(); - - runTestLoaderBlocking(); - - assertSingleAppSeedRequestStateValue(VariationsSeedLoader.AppSeedRequestState.SEED_FRESH); - } - - // Test we record the Variations.AppSeedRequestState metric when a new seed is requested. - @Test - @MediumTest - public void testRecordSeedRequested() throws Exception { - File oldFile = VariationsUtils.getSeedFile(); - Assert.assertTrue("Expected seed file to not already exist", oldFile.createNewFile()); - VariationsTestUtils.writeMockSeed(oldFile); - oldFile.setLastModified(EXPIRED_TIMESTAMP); - assertNoAppSeedRequestStateValues(); - - runTestLoaderBlocking(); - - assertSingleAppSeedRequestStateValue( - VariationsSeedLoader.AppSeedRequestState.SEED_REQUESTED); - } - - // Test we record the Variations.AppSeedRequestState metric when a seed request is throttled. - @Test - @MediumTest - public void testRecordSeedRequestThrottled() throws Exception { - File oldFile = VariationsUtils.getSeedFile(); - Assert.assertTrue("Expected seed file to not already exist", oldFile.createNewFile()); - VariationsTestUtils.writeMockSeed(oldFile); - oldFile.setLastModified(EXPIRED_TIMESTAMP); - // Update the last modified time of the stamp file to simulate having just requested a - // new seed from the service. - VariationsUtils.updateStampTime(); - assertNoAppSeedRequestStateValues(); - - runTestLoaderBlocking(); - - assertSingleAppSeedRequestStateValue( - VariationsSeedLoader.AppSeedRequestState.SEED_REQUEST_THROTTLED); - } - - // Test we record the Variations.AppSeedFreshness metric with loading a seed. - @Test - @MediumTest - public void testRecordAppSeedFreshness() throws Exception { - long seedAgeHours = 2; - File oldFile = VariationsUtils.getSeedFile(); - Assert.assertTrue("Expected seed file to not already exist", oldFile.createNewFile()); - VariationsTestUtils.writeMockSeed(oldFile); - oldFile.setLastModified(CURRENT_TIME_MILLIS - TimeUnit.HOURS.toMillis(seedAgeHours)); - - runTestLoaderBlocking(); - - Assert.assertEquals(1, - RecordHistogram.getHistogramValueCountForTesting( - VariationsSeedLoader.APP_SEED_FRESHNESS_HISTOGRAM_NAME, - (int) TimeUnit.HOURS.toMinutes(seedAgeHours))); - } - // Tests that the finch-seed-expiration-age flag works. @Test @MediumTest diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index 3517b4a3a419a8..e7c8172bed111d 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -3101,6 +3101,9 @@ Unknown properties are collapsed to zero. --> + + Removed from code October 2020. + diff --git a/tools/metrics/histograms/histograms_xml/variations/histograms.xml b/tools/metrics/histograms/histograms_xml/variations/histograms.xml index faf8fb9e2adc66..0a0ccda881730d 100644 --- a/tools/metrics/histograms/histograms_xml/variations/histograms.xml +++ b/tools/metrics/histograms/histograms_xml/variations/histograms.xml @@ -39,6 +39,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. + + Removed from code October 2020. + rmcelrath@chromium.org ntfschr@chromium.org src/android_webview/OWNERS