Fix UnsupportedOperationException if a leaked Activity.recreate is ca…

…lled Previously, ActivityController.recreate determined the current state of the underlying Activity by calling the global ActivityLifecycleMonitorRegistry.getLifecycleStageOf This method is unreliable for leaked Activities as a new ActivityLifecycleMonitor is created before each test. This means that, for a leaked Activity, the ActivityLifecycleMonitor from the previous test has knowledge if its lifecycle state, not the new one. When the Activity state was queried using the current ActivityLifecycleMonitor, an UnsupportedOperationException occurred. To fix this, avoid using ActivityLifecycleMonitor as the source of truth for Activity State in ActivityController, and instead add a new member variable to ActivityController that maintains the state. Having ActivityController rely on ActivityLifecycleMonitor for Activity state is circuitous -- AndroidX Test is already using ActivityController to drive Activity lifecycles, so the source-of-truth state should exist inside of ActivityController itself. PiperOrigin-RevId: 410168815
robolectric · Nov 17, 2021 · adb1588 · adb1588
1 parent 326991f
commit adb1588
Show file tree

Hide file tree

Showing 20 changed files with 255 additions and 89 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/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())) {