Limit instrumentation on interfaces

Previously, Robolectric enabled some of its instrumentation on interfaces in order to support hooking up interceptors on default interface methods. This is because there are now some default interface methods that invoke System.logW (e.g. android.compat.Compatibility$BehaviorChangeDelegate) However, the instrumentation step that makes classes public was also applied to interfaces, and this seems to confuse Mockito/ByteBuddy, but only in certain environments. As a fix, restrict the instrumentation to only rewrite interface method bodies, as well as extends 'InstrumentedInterface', so Robolectric does not re-instrument interfaces when the preinstrumented jars are used. A test case will be added after the preinstrumented jar version is bumped. Fixes #6858 PiperOrigin-RevId: 410440888
robolectric · Nov 17, 2021 · e03710c · e03710c
1 parent 326991f
commit e03710c
Show file tree

Hide file tree

Showing 21 changed files with 267 additions and 101 deletions.
diff --git a/resources/src/main/java/org/robolectric/res/android/Asset.java b/resources/src/main/java/org/robolectric/res/android/Asset.java
@@ -656,7 +656,7 @@ public boolean isNinePatch() {
     // : mStart(0), mLength(0), mOffset(0), mFp(null), mFileName(null), mMap(null), mBuf(null)
     {
       // Register the Asset with the global list here after it is fully constructed and its
-      // vtable pointer points to this concrete type. b/31113965
+      // vtable pointer points to this concrete type.
       registerAsset(this);
     }
 
@@ -668,7 +668,7 @@ protected void finalize() {
       close();
 
       // Unregister the Asset from the global list here before it is destructed and while its vtable
-      // pointer still points to this concrete type. b/31113965
+      // pointer still points to this concrete type.
       unregisterAsset(this);
     }
 
@@ -1136,7 +1136,7 @@ public boolean isNinePatch() {
       mFd = -1;
 
       // Register the Asset with the global list here after it is fully constructed and its
-      // vtable pointer points to this concrete type. b/31113965
+      // vtable pointer points to this concrete type.
       registerAsset(this);
     }
 
@@ -1167,7 +1167,7 @@ protected void finalize() {
       close();
 
       // Unregister the Asset from the global list here before it is destructed and while its vtable
-      // pointer still points to this concrete type. b/31113965
+      // pointer still points to this concrete type.
       unregisterAsset(this);
     }
 

diff --git a/resources/src/main/java/org/robolectric/res/android/Chunk.java b/resources/src/main/java/org/robolectric/res/android/Chunk.java
@@ -166,7 +166,6 @@ Chunk Next() {
       return new Chunk(this_chunk);
     }
 
-    // TODO(b/111401637) remove this and have full resource file verification
     // Returns false if there was an error. For legacy purposes.
     boolean VerifyNextChunkNonFatal() {
       if (len_ < ResChunk_header.SIZEOF) {

diff --git a/resources/src/main/java/org/robolectric/res/android/CppAssetManager.java b/resources/src/main/java/org/robolectric/res/android/CppAssetManager.java
@@ -51,8 +51,8 @@ enum FileType {
     kFileTypeSocket,
   }
 
-
-  // transliterated from https://cs.corp.google.com/android/frameworks/base/libs/androidfw/include/androidfw/AssetManager.h
+  // transliterated from
+  // https://cs.android.com/android/platform/superproject/+/master:frameworks/base/libs/androidfw/include/androidfw/AssetManager.h
   private static class asset_path {
 //    asset_path() : path(""), type(kFileTypeRegular), idmap(""),
 //      isSystemOverlay(false), isSystemAsset(false) {}

diff --git a/resources/src/main/java/org/robolectric/res/android/FileMap.java b/resources/src/main/java/org/robolectric/res/android/FileMap.java
@@ -239,7 +239,7 @@ static ImmutableMap<String, Long> guessDataOffsets(File zipFile, int length) {
       while (true) {
         // Instead of trusting numRecords, read until we find the
         // end-of-central-directory signature.  numRecords may wrap
-        // around with >64K entries (b/5455504).
+        // around with >64K entries.
         int sig = readInt(buffer, offset);
         if (sig == ENDSIG || sig == ENDSIG64) {
           break;

diff --git a/robolectric/src/main/java/org/robolectric/android/internal/LocalUiController.java b/robolectric/src/main/java/org/robolectric/android/internal/LocalUiController.java
@@ -57,7 +57,8 @@ public boolean injectKeyEvent(KeyEvent event) throws InjectEventSecurityExceptio
     return true;
   }
 
-  // TODO(b/80130000): implementation copied from espresso's UIControllerImpl. Refactor code into common location
+  // TODO: implementation copied from espresso's UIControllerImpl. Refactor code into common
+  // location
   @Override
   public boolean injectString(String str) throws InjectEventSecurityException {
     checkNotNull(str);
@@ -72,8 +73,7 @@ public boolean injectString(String str) throws InjectEventSecurityException {
     boolean eventInjected = false;
     KeyCharacterMap keyCharacterMap = getKeyCharacterMap();
 
-    // TODO(b/80130875): Investigate why not use (as suggested in javadoc of
-    // keyCharacterMap.getEvents):
+    // TODO: Investigate why not use (as suggested in javadoc of keyCharacterMap.getEvents):
     // http://developer.android.com/reference/android/view/KeyEvent.html#KeyEvent(long,
     // java.lang.String, int, int)
     KeyEvent[] events = keyCharacterMap.getEvents(str.toCharArray());

diff --git a/...tric/src/test/java/org/robolectric/android/controller/ActivityControllerRecreateTest.java b/...tric/src/test/java/org/robolectric/android/controller/ActivityControllerRecreateTest.java
@@ -0,0 +1,34 @@
+package org.robolectric.android.controller;
+
+import android.app.Activity;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import java.util.concurrent.atomic.AtomicReference;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.Robolectric;
+
+/**
+ * This test captures an issue where {@link ActivityController#recreate()} would throw an {@link
+ * UnsupportedOperationException} if an Activity from a previous test was recreated.
+ */
+@RunWith(AndroidJUnit4.class)
+public class ActivityControllerRecreateTest {
+  private static final AtomicReference<ActivityController<Activity>> createdActivity =
+      new AtomicReference<>();
+
+  @Before
+  public void setUp() {
+    createdActivity.compareAndSet(null, Robolectric.buildActivity(Activity.class).create());
+  }
+
+  @Test
+  public void failsTryingToRecreateActivityFromOtherTest1() {
+    createdActivity.get().recreate();
+  }
+
+  @Test
+  public void failsTryingToRecreateActivityFromOtherTest2() {
+    createdActivity.get().recreate();
+  }
+}
diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowBugreportManagerTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowBugreportManagerTest.java
@@ -4,7 +4,6 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
 import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.anyFloat;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.mock;
@@ -69,18 +68,16 @@ public void startBugreport_noPermission() throws Exception {
     BugreportCallback callback = mock(BugreportCallback.class);
     shadowBugreportManager.setHasPermission(false);
 
-    // TODO(b/179958637) switch to assertThrows once ThrowingRunnable no longer causes a test
-    // instantiation failure.
-    try {
-      shadowBugreportManager.startBugreport(
-          createWriteFile("bugreport"),
-          createWriteFile("screenshot"),
-          new BugreportParams(BugreportParams.BUGREPORT_MODE_FULL),
-          directExecutor(),
-          callback);
-      fail("Expected SecurityException");
-    } catch (SecurityException expected) {
-    }
+    assertThrows(
+        SecurityException.class,
+        () -> {
+          shadowBugreportManager.startBugreport(
+              createWriteFile("bugreport"),
+              createWriteFile("screenshot"),
+              new BugreportParams(BugreportParams.BUGREPORT_MODE_FULL),
+              directExecutor(),
+              callback);
+        });
     shadowMainLooper().idle();
 
     assertThat(shadowBugreportManager.isBugreportInProgress()).isFalse();

diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowBuildTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowBuildTest.java
@@ -2,6 +2,8 @@
 
 import static android.os.Build.VERSION_CODES.M;
 import static android.os.Build.VERSION_CODES.O;
+import static android.os.Build.VERSION_CODES.R;
+import static android.os.Build.VERSION_CODES.S;
 import static com.google.common.truth.Truth.assertThat;
 
 import android.os.Build;
@@ -56,6 +58,13 @@ public void setVersionIncremental() {
     assertThat(VERSION.INCREMENTAL).isEqualTo("robo_incremental");
   }
 
+  @Test
+  @Config(minSdk = S)
+  public void setVersionMediaPerformanceClass() {
+    ShadowBuild.setVersionMediaPerformanceClass(R);
+    assertThat(VERSION.MEDIA_PERFORMANCE_CLASS).isEqualTo(R);
+  }
+
   @Test
   @Config(minSdk = M)
   public void setVersionSecurityPatch() {

diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowIconTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowIconTest.java
@@ -2,11 +2,18 @@
 
 import static android.os.Build.VERSION_CODES.M;
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
 import static org.robolectric.Shadows.shadowOf;
 
+import android.content.Context;
 import android.graphics.Bitmap;
+import android.graphics.drawable.Drawable;
 import android.graphics.drawable.Icon;
 import android.net.Uri;
+import android.os.Handler;
+import android.os.Looper;
+import android.os.Message;
+import androidx.annotation.Nullable;
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import org.junit.Test;
@@ -16,16 +23,30 @@
 @RunWith(AndroidJUnit4.class)
 @Config(minSdk = M)
 public class ShadowIconTest {
+
+  private static final int MSG_ICON_LOADED = 312;
+
   public static final int TYPE_BITMAP = 1;
   public static final int TYPE_RESOURCE = 2;
   public static final int TYPE_DATA = 3;
   public static final int TYPE_URI = 4;
 
+  @Nullable private Drawable loadedDrawable;
+
+  private final Context appContext = ApplicationProvider.getApplicationContext();
+  private final Handler mainHandler =
+      new Handler(Looper.getMainLooper()) {
+        @Override
+        public void handleMessage(Message msg) {
+          if (msg.what == MSG_ICON_LOADED) {
+            loadedDrawable = (Drawable) msg.obj;
+          }
+        }
+      };
+
   @Test
   public void testGetRes() {
-    Icon icon =
-        Icon.createWithResource(
-            ApplicationProvider.getApplicationContext(), android.R.drawable.ic_delete);
+    Icon icon = Icon.createWithResource(appContext, android.R.drawable.ic_delete);
     assertThat(shadowOf(icon).getType()).isEqualTo(TYPE_RESOURCE);
     assertThat(shadowOf(icon).getResId()).isEqualTo(android.R.drawable.ic_delete);
   }
@@ -55,4 +76,32 @@ public void testGetUri() {
     assertThat(shadowOf(icon).getType()).isEqualTo(TYPE_URI);
     assertThat(shadowOf(icon).getUri()).isEqualTo(uri);
   }
+
+  @Test
+  public void testLoadDrawableAsyncWithMessage() {
+    ShadowIcon.overrideExecutor(directExecutor());
+
+    Icon icon = Icon.createWithResource(appContext, android.R.drawable.ic_delete);
+
+    Message andThen = Message.obtain(mainHandler, MSG_ICON_LOADED);
+
+    icon.loadDrawableAsync(appContext, andThen);
+    ShadowLooper.idleMainLooper();
+
+    assertThat(shadowOf(loadedDrawable).getCreatedFromResId())
+        .isEqualTo(android.R.drawable.ic_delete);
+  }
+
+  @Test
+  public void testLoadDrawableAsyncWithListener() {
+    ShadowIcon.overrideExecutor(directExecutor());
+
+    Icon icon = Icon.createWithResource(appContext, android.R.drawable.ic_delete);
+
+    icon.loadDrawableAsync(appContext, drawable -> this.loadedDrawable = drawable, mainHandler);
+    ShadowLooper.idleMainLooper();
+
+    assertThat(shadowOf(loadedDrawable).getCreatedFromResId())
+        .isEqualTo(android.R.drawable.ic_delete);
+  }
 }
diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowLegacyMessageQueueTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowLegacyMessageQueueTest.java
@@ -204,7 +204,7 @@ public void postAndRemoveSyncBarrierToken() {
   }
 
   @Test
-  // TODO(b/74402484): enable once workaround is removed
+  // TODO(https://github.com/robolectric/robolectric/issues/6852): enable once workaround is removed
   @Ignore
   public void removeInvalidSyncBarrierToken() {
     try {

diff --git a/sandbox/src/main/java/org/robolectric/internal/bytecode/ClassInstrumentor.java b/sandbox/src/main/java/org/robolectric/internal/bytecode/ClassInstrumentor.java
@@ -143,18 +143,6 @@ private void recordPackageStats(PerfStatsCollector perfStats, MutableClass mutab
 
   public void instrument(MutableClass mutableClass) {
     try {
-      makeClassPublic(mutableClass.classNode);
-      if ((mutableClass.classNode.access & Opcodes.ACC_FINAL) == Opcodes.ACC_FINAL) {
-        mutableClass
-            .classNode
-            .visitAnnotation("Lcom/google/errorprone/annotations/DoNotMock;", true)
-            .visit(
-                "value",
-                "This class is final. Consider using the real thing, or "
-                    + "adding/enhancing a Robolectric shadow for it.");
-      }
-      mutableClass.classNode.access = mutableClass.classNode.access & ~Opcodes.ACC_FINAL;
-
       // Need Java version >=7 to allow invokedynamic
       mutableClass.classNode.version = Math.max(mutableClass.classNode.version, Opcodes.V1_7);
 
@@ -163,6 +151,18 @@ public void instrument(MutableClass mutableClass) {
       if (mutableClass.isInterface()) {
         mutableClass.addInterface(Type.getInternalName(InstrumentedInterface.class));
       } else {
+        makeClassPublic(mutableClass.classNode);
+        if ((mutableClass.classNode.access & Opcodes.ACC_FINAL) == Opcodes.ACC_FINAL) {
+          mutableClass
+              .classNode
+              .visitAnnotation("Lcom/google/errorprone/annotations/DoNotMock;", true)
+              .visit(
+                  "value",
+                  "This class is final. Consider using the real thing, or "
+                      + "adding/enhancing a Robolectric shadow for it.");
+        }
+        mutableClass.classNode.access = mutableClass.classNode.access & ~Opcodes.ACC_FINAL;
+
         // If there is no constructor, adds one
         addNoArgsConstructor(mutableClass);
 

diff --git a/sandbox/src/main/java/org/robolectric/internal/bytecode/SandboxClassLoader.java b/sandbox/src/main/java/org/robolectric/internal/bytecode/SandboxClassLoader.java
@@ -82,7 +82,7 @@ private static URL[] getClassPathUrls(ClassLoader classloader) {
     return parseJavaClassPath();
   }
 
-  // TODO(b/65488446): Use a public API once one is available.
+  // TODO(https://github.com/google/guava/issues/2956): Use a public API once one is available.
   private static URL[] parseJavaClassPath() {
     ImmutableList.Builder<URL> urls = ImmutableList.builder();
     for (String entry : Splitter.on(PATH_SEPARATOR.value()).split(JAVA_CLASS_PATH.value())) {