Move existing @IntDef elements inside existing interface (javatests)

@IntDef/@StringDef annotation are preferred way for declaring
set of String/int values.

1. they need less space in APK than enum, see
https://developer.android.com/topic/performance/reduce-apk-size#remove-enums
2. they give more control over allowed values than "static final" values

Main goal of patch is writing "static final" values, enum
and some classes in one common @IntDef/@StringDef form:

1. with @IntDef/@StringDef first, @Retention second
   and related @interface third
2. with values inside @interface
3. with NUM_ENTRIES declaring number of entries if necessary
4. with comment about numbering from 0 without gaps when necessary
5. with @Retention(RetentionPolicy.SOURCE)
6. without "static final" in the @interface

Change-Id: I700514afca44a71999b6b316992cdcf37022b983
Reviewed-on: https://chromium-review.googlesource.com/1154223Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Commit-Queue: Marcin Wiącek <marcin@mwiacek.com>
Cr-Commit-Position: refs/heads/master@{#579238}

chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

@@ -327,13 +327,14 @@ public class SavePasswordsPreferencesTest {
         });
     }
 
+    @IntDef({MenuItemState.DISABLED, MenuItemState.ENABLED})
     @Retention(RetentionPolicy.SOURCE)
-    @IntDef({MENU_ITEM_STATE_DISABLED, MENU_ITEM_STATE_ENABLED})
-    private @interface MenuItemState {}
-    /** Represents the state of an enabled menu item. */
-    private static final int MENU_ITEM_STATE_DISABLED = 0;
-    /** Represents the state of a disabled menu item. */
-    private static final int MENU_ITEM_STATE_ENABLED = 1;
+    private @interface MenuItemState {
+        /** Represents the state of an enabled menu item. */
+        int DISABLED = 0;
+        /** Represents the state of a disabled menu item. */
+        int ENABLED = 1;
+    }
 
     /**
      * Checks that the menu item for exporting passwords is enabled or disabled as expected.
@@ -343,7 +344,7 @@ public class SavePasswordsPreferencesTest {
         openActionBarOverflowOrOptionsMenu(
                 InstrumentationRegistry.getInstrumentation().getTargetContext());
         final Matcher<View> stateMatcher =
-                expectedState == MENU_ITEM_STATE_ENABLED ? isEnabled() : not(isEnabled());
+                expectedState == MenuItemState.ENABLED ? isEnabled() : not(isEnabled());
         // The text matches a text view, but the disabled entity is some wrapper two levels up in
         // the view hierarchy, hence the two withParent matchers.
         Espresso.onView(allOf(withText(R.string.save_password_preferences_export_action_title),
@@ -552,7 +553,7 @@ public class SavePasswordsPreferencesTest {
                 PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(),
                         SavePasswordsPreferences.class.getName());
 
-        checkExportMenuItemState(MENU_ITEM_STATE_DISABLED);
+        checkExportMenuItemState(MenuItemState.DISABLED);
     }
 
     /**
@@ -571,7 +572,7 @@ public class SavePasswordsPreferencesTest {
                 PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(),
                         SavePasswordsPreferences.class.getName());
 
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**
@@ -717,7 +718,7 @@ public class SavePasswordsPreferencesTest {
                 .check(doesNotExist());
 
         // Check that the export menu item is enabled, because the current export was cancelled.
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**
@@ -774,7 +775,7 @@ public class SavePasswordsPreferencesTest {
                 .perform(click());
 
         // Check that for re-triggering, the export menu item is enabled.
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**
@@ -807,7 +808,7 @@ public class SavePasswordsPreferencesTest {
                 preferences.getFragmentForTest().onResume();
             }
         });
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**
@@ -994,7 +995,7 @@ public class SavePasswordsPreferencesTest {
 
         // Check that the cancellation succeeded by checking that the export menu is available and
         // enabled.
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**
@@ -1034,7 +1035,7 @@ public class SavePasswordsPreferencesTest {
 
         // Check that the cancellation succeeded by checking that the export menu is available and
         // enabled.
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**
@@ -1081,7 +1082,7 @@ public class SavePasswordsPreferencesTest {
 
         // Check that the export flow was cancelled automatically by checking that the export menu
         // is available and enabled.
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**
@@ -1114,7 +1115,7 @@ public class SavePasswordsPreferencesTest {
 
         // Check that the cancellation succeeded by checking that the export menu is available and
         // enabled.
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**
@@ -1279,7 +1280,7 @@ public class SavePasswordsPreferencesTest {
 
         // Check that the cancellation succeeded by checking that the export menu is available and
         // enabled.
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
 
         Assert.assertEquals(1, userAbortedDelta.getDelta());
     }
@@ -1322,7 +1323,7 @@ public class SavePasswordsPreferencesTest {
 
         // Check that the cancellation succeeded by checking that the export menu is available and
         // enabled.
-        checkExportMenuItemState(MENU_ITEM_STATE_ENABLED);
+        checkExportMenuItemState(MenuItemState.ENABLED);
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/ssl/CaptivePortalTest.java

@@ -49,29 +49,30 @@ public class CaptivePortalTest {
     private static final int INTERSTITIAL_TITLE_UPDATE_TIMEOUT_SECONDS = 5;
 
     // UMA events copied from ssl_error_handler.h.
-    @IntDef({
-            HANDLE_ALL, SHOW_CAPTIVE_PORTAL_INTERSTITIAL_NONOVERRIDABLE,
-            SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE, SHOW_SSL_INTERSTITIAL_NONOVERRIDABLE,
-            SHOW_SSL_INTERSTITIAL_OVERRIDABLE, WWW_MISMATCH_FOUND, WWW_MISMATCH_URL_AVAILABLE,
-            WWW_MISMATCH_URL_NOT_AVAILABLE, SHOW_BAD_CLOCK, CAPTIVE_PORTAL_CERT_FOUND,
-            WWW_MISMATCH_FOUND_IN_SAN, SHOW_MITM_SOFTWARE_INTERSTITIAL, OS_REPORTS_CAPTIVE_PORTAL,
-    })
+    @IntDef({UMAEvent.HANDLE_ALL, UMAEvent.SHOW_CAPTIVE_PORTAL_INTERSTITIAL_NONOVERRIDABLE,
+            UMAEvent.SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE,
+            UMAEvent.SHOW_SSL_INTERSTITIAL_NONOVERRIDABLE,
+            UMAEvent.SHOW_SSL_INTERSTITIAL_OVERRIDABLE, UMAEvent.WWW_MISMATCH_FOUND,
+            UMAEvent.WWW_MISMATCH_URL_AVAILABLE, UMAEvent.WWW_MISMATCH_URL_NOT_AVAILABLE,
+            UMAEvent.SHOW_BAD_CLOCK, UMAEvent.CAPTIVE_PORTAL_CERT_FOUND,
+            UMAEvent.WWW_MISMATCH_FOUND_IN_SAN, UMAEvent.SHOW_MITM_SOFTWARE_INTERSTITIAL,
+            UMAEvent.OS_REPORTS_CAPTIVE_PORTAL})
     @Retention(RetentionPolicy.SOURCE)
-    private @interface UMAEvent {}
-    private static final int HANDLE_ALL = 0;
-    private static final int SHOW_CAPTIVE_PORTAL_INTERSTITIAL_NONOVERRIDABLE = 1;
-    private static final int SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE = 2;
-    private static final int SHOW_SSL_INTERSTITIAL_NONOVERRIDABLE = 3;
-    private static final int SHOW_SSL_INTERSTITIAL_OVERRIDABLE = 4;
-    private static final int WWW_MISMATCH_FOUND =
-            5; // Deprecated in M59 by WWW_MISMATCH_FOUND_IN_SAN.
-    private static final int WWW_MISMATCH_URL_AVAILABLE = 6;
-    private static final int WWW_MISMATCH_URL_NOT_AVAILABLE = 7;
-    private static final int SHOW_BAD_CLOCK = 8;
-    private static final int CAPTIVE_PORTAL_CERT_FOUND = 9;
-    private static final int WWW_MISMATCH_FOUND_IN_SAN = 10;
-    private static final int SHOW_MITM_SOFTWARE_INTERSTITIAL = 11;
-    private static final int OS_REPORTS_CAPTIVE_PORTAL = 12;
+    private @interface UMAEvent {
+        int HANDLE_ALL = 0;
+        int SHOW_CAPTIVE_PORTAL_INTERSTITIAL_NONOVERRIDABLE = 1;
+        int SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE = 2;
+        int SHOW_SSL_INTERSTITIAL_NONOVERRIDABLE = 3;
+        int SHOW_SSL_INTERSTITIAL_OVERRIDABLE = 4;
+        int WWW_MISMATCH_FOUND = 5; // Deprecated in M59 by WWW_MISMATCH_FOUND_IN_SAN.
+        int WWW_MISMATCH_URL_AVAILABLE = 6;
+        int WWW_MISMATCH_URL_NOT_AVAILABLE = 7;
+        int SHOW_BAD_CLOCK = 8;
+        int CAPTIVE_PORTAL_CERT_FOUND = 9;
+        int WWW_MISMATCH_FOUND_IN_SAN = 10;
+        int SHOW_MITM_SOFTWARE_INTERSTITIAL = 11;
+        int OS_REPORTS_CAPTIVE_PORTAL = 12;
+    }
 
     @Rule
     public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
@@ -136,16 +137,16 @@ public class CaptivePortalTest {
 
         Assert.assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", HANDLE_ALL));
+                        "interstitial.ssl_error_handler", UMAEvent.HANDLE_ALL));
         Assert.assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting("interstitial.ssl_error_handler",
-                        SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE));
+                        UMAEvent.SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE));
         Assert.assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", CAPTIVE_PORTAL_CERT_FOUND));
+                        "interstitial.ssl_error_handler", UMAEvent.CAPTIVE_PORTAL_CERT_FOUND));
         Assert.assertEquals(0,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", OS_REPORTS_CAPTIVE_PORTAL));
+                        "interstitial.ssl_error_handler", UMAEvent.OS_REPORTS_CAPTIVE_PORTAL));
     }
 
     @Test
@@ -155,16 +156,16 @@ public class CaptivePortalTest {
 
         Assert.assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", HANDLE_ALL));
+                        "interstitial.ssl_error_handler", UMAEvent.HANDLE_ALL));
         Assert.assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting("interstitial.ssl_error_handler",
-                        SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE));
+                        UMAEvent.SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE));
         Assert.assertEquals(0,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", CAPTIVE_PORTAL_CERT_FOUND));
+                        "interstitial.ssl_error_handler", UMAEvent.CAPTIVE_PORTAL_CERT_FOUND));
         Assert.assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", OS_REPORTS_CAPTIVE_PORTAL));
+                        "interstitial.ssl_error_handler", UMAEvent.OS_REPORTS_CAPTIVE_PORTAL));
     }
 
     /** When CaptivePortalInterstitial feature is disabled, the result of OS captive portal
@@ -186,15 +187,15 @@ public class CaptivePortalTest {
 
         Assert.assertEquals(1,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", HANDLE_ALL));
+                        "interstitial.ssl_error_handler", UMAEvent.HANDLE_ALL));
         Assert.assertEquals(1,
-                RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", SHOW_SSL_INTERSTITIAL_OVERRIDABLE));
+                RecordHistogram.getHistogramValueCountForTesting("interstitial.ssl_error_handler",
+                        UMAEvent.SHOW_SSL_INTERSTITIAL_OVERRIDABLE));
         Assert.assertEquals(0,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", CAPTIVE_PORTAL_CERT_FOUND));
+                        "interstitial.ssl_error_handler", UMAEvent.CAPTIVE_PORTAL_CERT_FOUND));
         Assert.assertEquals(0,
                 RecordHistogram.getHistogramValueCountForTesting(
-                        "interstitial.ssl_error_handler", OS_REPORTS_CAPTIVE_PORTAL));
+                        "interstitial.ssl_error_handler", UMAEvent.OS_REPORTS_CAPTIVE_PORTAL));
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/vr/XrTestFramework.java

@@ -62,12 +62,13 @@ public abstract class XrTestFramework {
     static final String TEST_DIR = "chrome/test/data/xr/e2e_test_files";
 
     // Test status enum
-    private static final int STATUS_RUNNING = 0;
-    private static final int STATUS_PASSED = 1;
-    private static final int STATUS_FAILED = 2;
+    @IntDef({TestStatus.RUNNING, TestStatus.PASSED, TestStatus.FAILED})
     @Retention(RetentionPolicy.SOURCE)
-    @IntDef({STATUS_RUNNING, STATUS_PASSED, STATUS_FAILED})
-    private @interface TestStatus {}
+    private @interface TestStatus {
+        int RUNNING = 0;
+        int PASSED = 1;
+        int FAILED = 2;
+    }
 
     private ChromeActivityTestRule mRule;
     protected WebContents mFirstTabWebContents;
@@ -188,8 +189,9 @@ public abstract class XrTestFramework {
 
         // Check what state we're in to make sure javascriptDone wasn't called because the test
         // failed.
+        @TestStatus
         int testStatus = checkTestStatus(webContents);
-        if (!success || testStatus == STATUS_FAILED) {
+        if (!success || testStatus == TestStatus.FAILED) {
             // Failure states: Either polling failed or polling succeeded, but because the test
             // failed.
             String reason;
@@ -225,12 +227,12 @@ public abstract class XrTestFramework {
         boolean testPassed = Boolean.parseBoolean(
                 runJavaScriptOrFail("testPassed", POLL_TIMEOUT_SHORT_MS, webContents));
         if (testPassed) {
-            return STATUS_PASSED;
+            return TestStatus.PASSED;
         } else if (!testPassed && resultString.equals("\"\"")) {
-            return STATUS_RUNNING;
+            return TestStatus.RUNNING;
         } else {
             // !testPassed && !resultString.equals("\"\"")
-            return STATUS_FAILED;
+            return TestStatus.FAILED;
         }
     }
 
@@ -242,14 +244,14 @@ public abstract class XrTestFramework {
      */
     public static void endTest(WebContents webContents) {
         switch (checkTestStatus(webContents)) {
-            case STATUS_PASSED:
+            case TestStatus.PASSED:
                 break;
-            case STATUS_FAILED:
+            case TestStatus.FAILED:
                 String resultString =
                         runJavaScriptOrFail("resultString", POLL_TIMEOUT_SHORT_MS, webContents);
                 Assert.fail("JavaScript testharness failed with result: " + resultString);
                 break;
-            case STATUS_RUNNING:
+            case TestStatus.RUNNING:
                 Assert.fail("Attempted to end test in Java without finishing in JavaScript.");
                 break;
             default:
@@ -267,7 +269,7 @@ public abstract class XrTestFramework {
      * @param webContents The Webcontents for the tab to check for failures in.
      */
     public static void assertNoJavaScriptErrors(WebContents webContents) {
-        Assert.assertNotEquals(checkTestStatus(webContents), STATUS_FAILED);
+        Assert.assertNotEquals(checkTestStatus(webContents), TestStatus.FAILED);
     }
 
     /**