Remove dumpstate error race condition

Removes the race condition between the dumpstate binder service
death recipient and the dumpstate listener error callback. This
is done by adding a small wait when the death recipient is
invoked. This ensures that the caller of the Bugreport API does
not erroneously receive an incorrect runtime error callback.

Alters internal tests to account for this new delay.

Bug: 147703592
Test: atest BugreportManagerTest
Test: Manual. Take bugreport, ensure that the death recipient
      does not send an error callback when dumpstate finishes
      successfully, or fails and exits. Test error case by
      manually invoking signalErrorAndExit in DumpstateService.
Change-Id: I8c5b14c88502f0cf4f4f0e5c41d90d6065bfd1d4

1 files changed, 9 insertions, 3 deletions

diff --git a/core/tests/bugreports/src/com/android/os/bugreports/tests/BugreportManagerTest.java b/core/tests/bugreports/src/com/android/os/bugreports/tests/BugreportManagerTest.java
index 222471a830b1..09f16a82cd7f 100644
--- a/core/tests/bugreports/src/com/android/os/bugreports/tests/BugreportManagerTest.java
+++ b/core/tests/bugreports/src/com/android/os/bugreports/tests/BugreportManagerTest.java
@@ -76,6 +76,12 @@ public class BugreportManagerTest {
     private static final long DUMPSTATE_STARTUP_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10);
     private static final long UIAUTOMATOR_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10);
 
+
+    // A small timeout used when waiting for the result of a BugreportCallback to be received.
+    // This value must be at least 1000ms since there is an intentional delay in
+    // BugreportManagerServiceImpl in the error case.
+    private static final long CALLBACK_RESULT_TIMEOUT_MS = 1500;
+
     // Sent by Shell when its bugreport finishes (contains final bugreport/screenshot file name
     // associated with the bugreport).
     private static final String INTENT_BUGREPORT_FINISHED =
@@ -185,7 +191,7 @@ public class BugreportManagerTest {
         ParcelFileDescriptor bugreportFd2 = parcelFd(bugreportFile2);
         ParcelFileDescriptor screenshotFd2 = parcelFd(screenshotFile2);
         mBrm.startBugreport(bugreportFd2, screenshotFd2, wifi(), mExecutor, callback2);
-        Thread.sleep(500 /* .5s */);
+        Thread.sleep(CALLBACK_RESULT_TIMEOUT_MS);
 
         // Verify #2 encounters an error.
         assertThat(callback2.getErrorCode()).isEqualTo(
@@ -194,7 +200,7 @@ public class BugreportManagerTest {
 
         // Cancel #1 so we can move on to the next test.
         mBrm.cancelBugreport();
-        Thread.sleep(500 /* .5s */);
+        waitTillDoneOrTimeout(callback);
         assertThat(callback.isDone()).isTrue();
         assertFdsAreClosed(mBugreportFd, mScreenshotFd);
     }
@@ -220,7 +226,7 @@ public class BugreportManagerTest {
         // Try again, with DUMP permission.
         getPermissions();
         mBrm.cancelBugreport();
-        Thread.sleep(500 /* .5s */);
+        waitTillDoneOrTimeout(callback);
         assertThat(callback.isDone()).isTrue();
         assertFdsAreClosed(mBugreportFd, mScreenshotFd);
     }