Android: Remove Mockito.mock() R8 work-arounds

R8 now natively supports tracing of Mockito.mock() and Mockito.spy()

Bug: 389166093
Change-Id: Iffa791d1af203f621130f247fa995f038df9d6ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6226560
Reviewed-by: Sam Maier <smaier@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Owners-Override: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1418690}

PRESUBMIT.py

@@ -7473,123 +7473,6 @@ a subclass of it), or use "@Rule BaseRobolectricTestRule".
     return results
 
 
-def CheckMockAnnotation(input_api, output_api):
-    """Checks that we have annotated all Mockito.mock()-ed or Mockito.spy()-ed
-    classes with @Mock or @Spy. If this is not an instrumentation test,
-    disregard."""
-
-    # This is just trying to be approximately correct. We are not writing a
-    # Java parser, so special cases like statically importing mock() then
-    # calling an unrelated non-mockito spy() function will cause a false
-    # positive.
-    package_name = input_api.re.compile(r'^package\s+(\w+(?:\.\w+)+);')
-    mock_static_import = input_api.re.compile(
-        r'^import\s+static\s+org.mockito.Mockito.(?:mock|spy);')
-    import_class = input_api.re.compile(r'import\s+((?:\w+\.)+)(\w+);')
-    mock_annotation = input_api.re.compile(r'^\s*@(?:Mock|Spy)')
-    field_type = input_api.re.compile(r'(\w+)(?:<\w+>)?\s+\w+\s*(?:;|=)')
-    mock_or_spy_function_call = r'(?:mock|spy)\(\s*(?:new\s*)?(\w+)(?:\.class|\()'
-    fully_qualified_mock_function = input_api.re.compile(
-        r'Mockito\.' + mock_or_spy_function_call)
-    statically_imported_mock_function = input_api.re.compile(
-        r'\W' + mock_or_spy_function_call)
-    robolectric_test = input_api.re.compile(r'[rR]obolectric')
-    uiautomator_test = input_api.re.compile(r'[uU]i[aA]utomator')
-
-    def _DoClassLookup(class_name, class_name_map, package):
-        found = class_name_map.get(class_name)
-        if found is not None:
-            return found
-        else:
-            return package + '.' + class_name
-
-    def _FilterFile(affected_file):
-        return input_api.FilterSourceFile(
-            affected_file,
-            files_to_skip=input_api.DEFAULT_FILES_TO_SKIP,
-            files_to_check=[r'.*Test\.java$'])
-
-    mocked_by_function_classes = set()
-    mocked_by_annotation_classes = set()
-    class_to_filename = {}
-    for f in input_api.AffectedSourceFiles(_FilterFile):
-        mock_function_regex = fully_qualified_mock_function
-        next_line_is_annotated = False
-        fully_qualified_class_map = {}
-        package = None
-
-        for line in f.NewContents():
-            if robolectric_test.search(line) or uiautomator_test.search(line):
-                # Skip Robolectric and UiAutomator tests.
-                break
-
-            m = package_name.search(line)
-            if m:
-                package = m.group(1)
-                continue
-
-            if mock_static_import.search(line):
-                mock_function_regex = statically_imported_mock_function
-                continue
-
-            m = import_class.search(line)
-            if m:
-                fully_qualified_class_map[m.group(2)] = m.group(1) + m.group(2)
-                continue
-
-            if next_line_is_annotated:
-                next_line_is_annotated = False
-                fully_qualified_class = _DoClassLookup(
-                    field_type.search(line).group(1), fully_qualified_class_map,
-                    package)
-                mocked_by_annotation_classes.add(fully_qualified_class)
-                continue
-
-            if mock_annotation.search(line):
-                field_type_search = field_type.search(line)
-                if field_type_search:
-                    fully_qualified_class = _DoClassLookup(
-                        field_type_search.group(1), fully_qualified_class_map,
-                        package)
-                    mocked_by_annotation_classes.add(fully_qualified_class)
-                else:
-                    next_line_is_annotated = True
-                continue
-
-            m = mock_function_regex.search(line)
-            if m:
-                fully_qualified_class = _DoClassLookup(m.group(1),
-                    fully_qualified_class_map, package)
-                # Skipping builtin classes, since they don't get optimized.
-                if fully_qualified_class.startswith(
-                        'android.') or fully_qualified_class.startswith(
-                            'java.'):
-                    continue
-                class_to_filename[fully_qualified_class] = str(f.LocalPath())
-                mocked_by_function_classes.add(fully_qualified_class)
-
-    results = []
-    missed_classes = mocked_by_function_classes - mocked_by_annotation_classes
-    if missed_classes:
-        error_locations = []
-        for c in missed_classes:
-            error_locations.append(c + ' in ' + class_to_filename[c])
-        results.append(
-            output_api.PresubmitPromptWarning(
-                """
-Mockito.mock()/spy() cause issues with our Java optimizer. You have 3 options:
-1) If the mocked variable can be a class member, annotate the member with
-   @Mock/@Spy.
-2) If the mocked variable cannot be a class member, create a dummy member
-   variable of that type, annotated with @Mock/@Spy. This dummy does not need
-   to be used or initialized in any way.
-3) If the mocked type is definitely not going to be optimized, whether it's a
-   builtin type which we don't ship, or a class you know R8 will treat
-   specially, you can ignore this warning.
-""", error_locations))
-
-    return results
-
 def CheckNoJsInIos(input_api, output_api):
     """Checks to make sure that JavaScript files are not used on iOS."""
 

PRESUBMIT_test.py

@@ -5116,89 +5116,6 @@ class CheckAndroidTestAnnotations(unittest.TestCase):
         self.assertIn('OneTest.java', errors[0].items[0])
 
 
-class CheckMockAnnotation(unittest.TestCase):
-    """Test the CheckMockAnnotation presubmit check."""
-
-    def testTruePositives(self):
-        """Examples of @Mock or @Spy being used and nothing should be flagged."""
-        mock_input = MockInputApi()
-        mock_input.files = [
-            MockFile('path/OneTest.java', [
-                'import a.b.c.Bar;',
-                'import a.b.c.Foo;',
-                '@Mock public static Foo f = new Foo();',
-                'Mockito.mock(new Bar(a, b, c))'
-            ]),
-            MockFile('path/TwoTest.java', [
-                'package x.y.z;',
-                'import static org.mockito.Mockito.spy;',
-                '@Spy',
-                'public static FooBar<Baz> f;',
-                'a = spy(Baz.class)'
-            ]),
-        ]
-        errors = PRESUBMIT.CheckMockAnnotation(mock_input, MockOutputApi())
-        self.assertEqual(1, len(errors))
-        self.assertEqual(2, len(errors[0].items))
-        self.assertIn('a.b.c.Bar in path/OneTest.java', errors[0].items)
-        self.assertIn('x.y.z.Baz in path/TwoTest.java', errors[0].items)
-
-    def testTrueNegatives(self):
-        """Examples of when we should not be flagging mock() or spy() calls."""
-        mock_input = MockInputApi()
-        mock_input.files = [
-            MockFile('path/OneTest.java', [
-                'package a.b.c;',
-                'import org.chromium.base.test.BaseRobolectricTestRunner;',
-                'Mockito.mock(Abc.class)'
-            ]),
-            MockFile('path/TwoTest.java', [
-                'package a.b.c;',
-                'import androidx.test.uiautomator.UiDevice;',
-                'Mockito.spy(new Def())'
-            ]),
-            MockFile('path/ThreeTest.java', [
-                'package a.b.c;',
-                'import static org.mockito.Mockito.spy;',
-                '@Spy',
-                'public static Foo f = new Abc();',
-                'a = spy(Foo.class)'
-            ]),
-            MockFile('path/FourTest.java', [
-                'package a.b.c;',
-                'import static org.mockito.Mockito.mock;',
-                '@Spy',
-                'public static Bar b = new Abc(a, b, c, d);',
-                ' mock(new Bar(a,b,c))'
-            ]),
-            MockFile('path/FiveTest.java', [
-                'package a.b.c;',
-                '@Mock',
-                'public static Baz<abc> b;',
-                'Mockito.mock(Baz.class)'
-            ]),
-            MockFile('path/SixTest.java', [
-                'package a.b.c;',
-                'import android.view.View;',
-                'import java.ArrayList;',
-                'Mockito.spy(new View())',
-                'Mockito.mock(ArrayList.class)'
-            ]),
-            MockFile('path/SevenTest.java', [
-                'package a.b.c;',
-                '@Mock private static Seven s;',
-                'Mockito.mock(Seven.class)'
-            ]),
-            MockFile('path/EightTest.java', [
-                'package a.b.c;',
-                '@Spy Eight e = new Eight2();',
-                'Mockito.py(new Eight())'
-            ]),
-        ]
-        errors = PRESUBMIT.CheckMockAnnotation(mock_input, MockOutputApi())
-        self.assertEqual(0, len(errors))
-
-
 class AssertNoJsInIosTest(unittest.TestCase):
     def testErrorJs(self):
         input_api = MockInputApi()

chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabListEditorMenuTest.java

@@ -152,10 +152,6 @@ public class TabListEditorMenuTest {
         }
     }
 
-    // For R8 optimizer message
-    @Mock private Tab mTabDoNotUse;
-
-    // Real mocks.
     @Mock private TabModel mTabModel;
     @Mock private TabGroupModelFilter mTabGroupModelFilter;
     private SelectionDelegate<Integer> mSelectionDelegate;

chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulatorTest.java

@@ -93,9 +93,7 @@ public class ChromeContextMenuPopulatorTest {
     @Mock private Profile mProfile;
     @Mock private Profile.Natives mProfileNatives;
 
-    // Despite this being a spy, we add the @Mock annotation so that proguard doesn't strip the
-    // spied class.
-    @Mock private ChromeContextMenuPopulator mPopulator;
+    private ChromeContextMenuPopulator mPopulator;
 
     @Before
     public void setUp() {
@@ -709,8 +707,6 @@ public class ChromeContextMenuPopulatorTest {
                         /* additionalNavigationParams= */ null);
 
         int[] expected = null;
-        checkMenuOptions(expected);
-
         initializePopulator(ChromeContextMenuPopulator.ContextMenuMode.CUSTOM_TAB, params);
         checkMenuOptions(expected);
 

chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchCriticalTest.java

@@ -17,7 +17,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
@@ -43,10 +42,6 @@ import org.chromium.ui.base.DeviceFormFactor;
 @Batch(Batch.PER_CLASS)
 public class ContextualSearchCriticalTest extends ContextualSearchInstrumentationBase {
 
-    // Needed to avoid issues on Release builds where Natives is made final and can not be Spy'd
-    // below.
-    @Mock ContextualSearchManagerJni mUnusedContextualSearchManagerJni;
-
     private ContextualSearchManager.Natives mContextualSearchManagerNatives;
 
     @Override

chrome/android/javatests/src/org/chromium/chrome/browser/device_dialog/BluetoothChooserDialogTest.java

@@ -28,7 +28,6 @@ import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.CommandLineFlags;
@@ -88,11 +87,6 @@ public class BluetoothChooserDialogTest {
     private String mFinishedDeviceId;
     private int mRestartSearchCount;
 
-    // Unused member variables to avoid Java optimizer issues with Mockito.
-    @Mock ModalDialogManager mMockModalDialogManager;
-    @Mock Activity mMockActivity;
-    @Mock WindowAndroid mMockWindowAndroid;
-
     private class TestBluetoothChooserDialogJni implements BluetoothChooserDialog.Natives {
         private BluetoothChooserDialog mBluetoothChooserDialog;
 

chrome/android/javatests/src/org/chromium/chrome/browser/device_dialog/UsbChooserDialogTest.java

@@ -22,7 +22,6 @@ import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.CommandLineFlags;
@@ -60,11 +59,6 @@ public class UsbChooserDialogTest {
 
     private UsbChooserDialog mChooserDialog;
 
-    // Unused member variables to avoid Java optimizer issues with Mockito.
-    @Mock ModalDialogManager mMockModalDialogManager;
-    @Mock Activity mMockActivity;
-    @Mock WindowAndroid mMockWindowAndroid;
-
     private class TestUsbChooserDialogJni implements UsbChooserDialog.Natives {
         @Override
         public void onItemSelected(long nativeUsbChooserDialogAndroid, String deviceId) {

chrome/android/javatests/src/org/chromium/chrome/browser/tab/state/ShoppingPersistedTabDataTest.java

@@ -59,9 +59,6 @@ public class ShoppingPersistedTabDataTest {
 
     @Mock protected NavigationHandle mNavigationHandle;
 
-    // For R8 optimizer - see b/303266326.
-    @Mock private Tab mDoNotUseTab;
-
     @Mock private ShoppingPersistedTabDataService mShoppingPersistedTabDataService;
 
     @Mock ShoppingService mShoppingService;

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java

@@ -19,7 +19,6 @@ import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 
 import org.chromium.base.ActivityState;
@@ -152,7 +151,6 @@ public class TabPersistentStoreTest {
         final MockTabPersistentStoreObserver mTabPersistentStoreObserver;
         private final TabModelOrderController mTabModelOrderController;
         // Required to ensure TabContentManager is not null.
-        @Mock // Annotation required to disable R8 optimization of the class.
         private final TabContentManager mMockTabContentManager;
 
         public TestTabModelSelector(Context context) throws Exception {

chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkInitializationTest.java

@@ -14,11 +14,7 @@ import androidx.test.filters.LargeTest;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.Mockito;
-import org.mockito.junit.MockitoJUnit;
-import org.mockito.junit.MockitoRule;
-import org.mockito.quality.Strictness;
 
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationStatus;
@@ -31,7 +27,6 @@ import org.chromium.chrome.browser.browserservices.ui.controller.webapps.WebappD
 import org.chromium.chrome.browser.customtabs.BaseCustomTabActivity;
 import org.chromium.chrome.browser.customtabs.CustomTabOrientationController;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
-import org.chromium.chrome.browser.init.ActivityLifecycleDispatcherImpl;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.webapps.WebApkIntentDataProviderBuilder;
@@ -46,10 +41,7 @@ import java.util.concurrent.TimeoutException;
 public class WebApkInitializationTest {
     @Rule public final WebApkActivityTestRule mActivityRule = new WebApkActivityTestRule();
 
-    @Rule public MockitoRule mMockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);
-
-    @Mock public ActivityLifecycleDispatcherImpl mUnusedForR8KeepRules;
-    @Mock public ActivityLifecycleDispatcher mActivityLifecycleDispatcher;
+    public ActivityLifecycleDispatcher mActivityLifecycleDispatcher;
 
     class CreationObserver implements ApplicationStatus.ActivityStateListener {
         public CreationObserver() {}

chrome/browser/commerce/price_tracking/android/javatests/src/org/chromium/chrome/browser/price_tracking/PriceDropNotificationManagerTest.java

@@ -87,8 +87,7 @@ public class PriceDropNotificationManagerTest {
     private static final String PRODUCT_CLUSTER_ID = "cluster_id";
     private static final int NOTIFICATION_ID = 123;
 
-    @Mock NotificationManagerProxyImpl mUnusedForR8KeepRules;
-    @Mock NotificationManagerProxy mMockNotificationManager;
+    private NotificationManagerProxy mMockNotificationManager;
 
     private PriceDropNotificationManager mPriceDropNotificationManager;
     private BookmarkModel mMockBookmarkModel;

chrome/browser/paint_preview/android/javatests/src/org/chromium/chrome/browser/paint_preview/DemoPaintPreviewTest.java

@@ -19,7 +19,6 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 
 import org.chromium.base.Callback;
@@ -53,8 +52,7 @@ public class DemoPaintPreviewTest {
 
     private static final String TEST_URL = "/chrome/test/data/android/about.html";
 
-    // @Mock to tell R8 not to break the ability to mock the class.
-    @Mock private static PaintPreviewTabService sMockService;
+    private static PaintPreviewTabService sMockService;
 
     @BeforeClass
     public static void setUp() {

chrome/browser/paint_preview/android/javatests/src/org/chromium/chrome/browser/paint_preview/StartupPaintPreviewTest.java

@@ -25,7 +25,6 @@ import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 
 import org.chromium.base.ThreadUtils;
@@ -51,9 +50,6 @@ public class StartupPaintPreviewTest {
     public static ChromeTabbedActivityTestRule sActivityTestRule =
             new ChromeTabbedActivityTestRule();
 
-    // Tell R8 not to break the ability to mock the class.
-    @Mock private static PaintPreviewTabService sUnused;
-
     @Rule
     public final BlankCTATabInitialStateRule mInitialStateRule =
             new BlankCTATabInitialStateRule(sActivityTestRule, true);

chrome/browser/paint_preview/android/javatests/src/org/chromium/chrome/browser/paint_preview/TabbedPaintPreviewTest.java

@@ -19,7 +19,6 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 
 import org.chromium.base.Callback;
@@ -52,9 +51,6 @@ public class TabbedPaintPreviewTest {
     @Rule
     public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
 
-    // Tell R8 not to break the ability to mock the class.
-    @Mock private PaintPreviewTabService mUnused;
-
     private static final String TEST_URL = "/chrome/test/data/android/about.html";
 
     /** Implementation of {@link PlayerCompositorDelegate.Factory} for tests. */

chrome/browser/ui/android/appmenu/internal/java/src/org/chromium/chrome/browser/ui/appmenu/AppMenuTest.java

@@ -106,8 +106,6 @@ public class AppMenuTest {
     @Mock private WindowAndroid mWindowAndroid;
     @Mock private BrowserControlsStateProvider mBrowserControlsStateProvider;
     @Mock private KeyboardVisibilityDelegate mKeyboardDelegate;
-    // Tell R8 not to break the ability to mock the class.
-    @Mock private AppMenu mUnused;
 
     @Captor
     private ArgumentCaptor<KeyboardVisibilityDelegate.KeyboardVisibilityListener>

components/browser_ui/edge_to_edge/android/java/src/org/chromium/components/browser_ui/edge_to_edge/layout/EdgeToEdgeLayoutViewTest.java

@@ -21,7 +21,6 @@ import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.BaseActivityTestRule;
@@ -65,9 +64,6 @@ public class EdgeToEdgeLayoutViewTest {
     private static final int NAV_BAR_DIVIDER_COLOR = Color.BLUE;
     private static final int BG_COLOR = Color.GRAY;
 
-    // Added to stop R8 from marking WindowInsetsCompat as final and not mock-able.
-    @Mock WindowInsetsCompat mUnused;
-
     private EdgeToEdgeLayoutCoordinator mEdgeToEdgeLayoutCoordinator;
     private FrameLayout mContentView;
     private View mEdgeToEdgeLayout;