Fix handling of activity destruction based on configuration change (e.g. screen rotation). (#4340)

Author: kaibolay

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FeedbackActivity.java

@@ -28,6 +28,7 @@
 import android.widget.ImageView;
 import android.widget.TextView;
 import android.widget.Toast;
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.appcompat.app.AppCompatActivity;
 import java.io.IOException;
@@ -38,11 +39,11 @@ public class FeedbackActivity extends AppCompatActivity {
   private static final int SCREENSHOT_TARGET_WIDTH_PX = 600;
   private static final int SCREENSHOT_TARGET_HEIGHT_PX = -1; // scale proportionally
 
-  public static final String RELEASE_NAME_EXTRA_KEY =
+  public static final String RELEASE_NAME_KEY =
       "com.google.firebase.appdistribution.FeedbackActivity.RELEASE_NAME";
-  public static final String INFO_TEXT_EXTRA_KEY =
+  public static final String INFO_TEXT_KEY =
       "com.google.firebase.appdistribution.FeedbackActivity.INFO_TEXT";
-  public static final String SCREENSHOT_URI_EXTRA_KEY =
+  public static final String SCREENSHOT_URI_KEY =
       "com.google.firebase.appdistribution.FeedbackActivity.SCREENSHOT_URI";
 
   private FeedbackSender feedbackSender;
@@ -53,15 +54,33 @@ public class FeedbackActivity extends AppCompatActivity {
   @Override
   protected void onCreate(Bundle savedInstanceState) {
     super.onCreate(savedInstanceState);
-    releaseName = getIntent().getStringExtra(RELEASE_NAME_EXTRA_KEY);
-    infoText = getIntent().getCharSequenceExtra(INFO_TEXT_EXTRA_KEY);
-    if (getIntent().hasExtra(SCREENSHOT_URI_EXTRA_KEY)) {
-      screenshotUri = Uri.parse(getIntent().getStringExtra(SCREENSHOT_URI_EXTRA_KEY));
-    }
+
     feedbackSender = FeedbackSender.getInstance();
+    if (savedInstanceState != null) {
+      releaseName = savedInstanceState.getString(RELEASE_NAME_KEY);
+      infoText = savedInstanceState.getCharSequence(INFO_TEXT_KEY);
+      String screenshotUriString = savedInstanceState.getString(SCREENSHOT_URI_KEY);
+      if (screenshotUriString != null) {
+        screenshotUri = Uri.parse(screenshotUriString);
+      }
+    } else {
+      releaseName = getIntent().getStringExtra(RELEASE_NAME_KEY);
+      infoText = getIntent().getCharSequenceExtra(INFO_TEXT_KEY);
+      if (getIntent().hasExtra(SCREENSHOT_URI_KEY)) {
+        screenshotUri = Uri.parse(getIntent().getStringExtra(SCREENSHOT_URI_KEY));
+      }
+    }
     setupView();
   }
 
+  @Override
+  protected void onSaveInstanceState(@NonNull Bundle outState) {
+    outState.putString(RELEASE_NAME_KEY, releaseName);
+    outState.putCharSequence(INFO_TEXT_KEY, infoText);
+    outState.putString(SCREENSHOT_URI_KEY, screenshotUri.toString());
+    super.onSaveInstanceState(outState);
+  }
+
   private void setupView() {
     setTheme(R.style.FeedbackTheme);
     setContentView(R.layout.activity_feedback);

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java

@@ -415,10 +415,10 @@ private Task<Void> launchFeedbackActivity(
           LogWrapper.getInstance().i("Launching feedback activity");
           Intent intent = new Intent(activity, FeedbackActivity.class);
           // in development-mode the releaseName might be null
-          intent.putExtra(FeedbackActivity.RELEASE_NAME_EXTRA_KEY, releaseName);
-          intent.putExtra(FeedbackActivity.INFO_TEXT_EXTRA_KEY, infoText);
+          intent.putExtra(FeedbackActivity.RELEASE_NAME_KEY, releaseName);
+          intent.putExtra(FeedbackActivity.INFO_TEXT_KEY, infoText);
           if (screenshotUri != null) {
-            intent.putExtra(FeedbackActivity.SCREENSHOT_URI_EXTRA_KEY, screenshotUri.toString());
+            intent.putExtra(FeedbackActivity.SCREENSHOT_URI_KEY, screenshotUri.toString());
           }
           activity.startActivity(intent);
         });
@@ -476,12 +476,14 @@ void onActivityDestroyed(@NonNull Activity activity) {
 
     if (activity instanceof FeedbackActivity) {
       LogWrapper.getInstance().i("FeedbackActivity destroyed");
-      feedbackInProgress.set(false);
+      if (activity.isFinishing()) {
+        feedbackInProgress.set(false);
 
-      // If the feedback activity finishes, clean up the screenshot that was taken before starting
-      // the activity. If this does not happen for some reason it will be cleaned up the next time
-      // before taking a new screenshot.
-      screenshotTaker.deleteScreenshot();
+        // If the feedback activity finishes, clean up the screenshot that was taken before starting
+        // the activity. If this does not happen for some reason it will be cleaned up the next time
+        // before taking a new screenshot.
+        screenshotTaker.deleteScreenshot();
+      }
     }
   }
 

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java

@@ -27,9 +27,9 @@
 import static com.google.firebase.appdistribution.impl.ErrorMessages.NETWORK_ERROR;
 import static com.google.firebase.appdistribution.impl.ErrorMessages.RELEASE_NOT_FOUND_ERROR;
 import static com.google.firebase.appdistribution.impl.ErrorMessages.UPDATE_CANCELED;
-import static com.google.firebase.appdistribution.impl.FeedbackActivity.INFO_TEXT_EXTRA_KEY;
-import static com.google.firebase.appdistribution.impl.FeedbackActivity.RELEASE_NAME_EXTRA_KEY;
-import static com.google.firebase.appdistribution.impl.FeedbackActivity.SCREENSHOT_URI_EXTRA_KEY;
+import static com.google.firebase.appdistribution.impl.FeedbackActivity.INFO_TEXT_KEY;
+import static com.google.firebase.appdistribution.impl.FeedbackActivity.RELEASE_NAME_KEY;
+import static com.google.firebase.appdistribution.impl.FeedbackActivity.SCREENSHOT_URI_KEY;
 import static com.google.firebase.appdistribution.impl.TestUtils.assertTaskFailure;
 import static java.util.stream.Collectors.toList;
 import static org.junit.Assert.assertEquals;
@@ -660,10 +660,10 @@ public void startFeedback_signsInTesterAndStartsActivity() throws InterruptedExc
     Intent expectedIntent = new Intent(activity, FeedbackActivity.class);
     Intent actualIntent = shadowOf(RuntimeEnvironment.getApplication()).getNextStartedActivity();
     assertEquals(expectedIntent.getComponent(), actualIntent.getComponent());
-    assertThat(actualIntent.getStringExtra(RELEASE_NAME_EXTRA_KEY)).isEqualTo("release-name");
-    assertThat(actualIntent.getStringExtra(SCREENSHOT_URI_EXTRA_KEY))
+    assertThat(actualIntent.getStringExtra(RELEASE_NAME_KEY)).isEqualTo("release-name");
+    assertThat(actualIntent.getStringExtra(SCREENSHOT_URI_KEY))
         .isEqualTo(TEST_SCREENSHOT_URI.toString());
-    assertThat(actualIntent.getStringExtra(INFO_TEXT_EXTRA_KEY))
+    assertThat(actualIntent.getStringExtra(INFO_TEXT_KEY))
         .isEqualTo("Some terms and conditions");
     assertThat(firebaseAppDistribution.isFeedbackInProgress()).isTrue();
   }
@@ -691,10 +691,10 @@ public void startFeedback_withUri_doesNotTakeScreenshot() throws InterruptedExce
     Intent expectedIntent = new Intent(activity, FeedbackActivity.class);
     Intent actualIntent = shadowOf(RuntimeEnvironment.getApplication()).getNextStartedActivity();
     assertEquals(expectedIntent.getComponent(), actualIntent.getComponent());
-    assertThat(actualIntent.getStringExtra(RELEASE_NAME_EXTRA_KEY)).isEqualTo("release-name");
-    assertThat(actualIntent.getStringExtra(SCREENSHOT_URI_EXTRA_KEY))
+    assertThat(actualIntent.getStringExtra(RELEASE_NAME_KEY)).isEqualTo("release-name");
+    assertThat(actualIntent.getStringExtra(SCREENSHOT_URI_KEY))
         .isEqualTo(providedUri.toString());
-    assertThat(actualIntent.getStringExtra(INFO_TEXT_EXTRA_KEY))
+    assertThat(actualIntent.getStringExtra(INFO_TEXT_KEY))
         .isEqualTo("Some terms and conditions");
     assertThat(firebaseAppDistribution.isFeedbackInProgress()).isTrue();
   }
@@ -737,9 +737,9 @@ public void startFeedback_screenshotFails_startActivityWithNoScreenshot()
     Intent expectedIntent = new Intent(activity, FeedbackActivity.class);
     Intent actualIntent = shadowOf(RuntimeEnvironment.getApplication()).getNextStartedActivity();
     assertEquals(expectedIntent.getComponent(), actualIntent.getComponent());
-    assertThat(actualIntent.getStringExtra(RELEASE_NAME_EXTRA_KEY)).isEqualTo("release-name");
-    assertThat(actualIntent.getStringExtra(SCREENSHOT_URI_EXTRA_KEY)).isNull();
-    assertThat(actualIntent.getStringExtra(INFO_TEXT_EXTRA_KEY))
+    assertThat(actualIntent.getStringExtra(RELEASE_NAME_KEY)).isEqualTo("release-name");
+    assertThat(actualIntent.getStringExtra(SCREENSHOT_URI_KEY)).isNull();
+    assertThat(actualIntent.getStringExtra(INFO_TEXT_KEY))
         .isEqualTo("Some terms and conditions");
     assertThat(firebaseAppDistribution.isFeedbackInProgress()).isTrue();
   }