Delay final notification for bugreport generation

System discards notifications in case it comes too early.

Bug: 344946064
Test: atest com.android.shell.BugreportReceiverTest#testStressProgress
Change-Id: I9193bbfe487e81bf711112d673961cb285c81bbb

3 files changed, 75 insertions, 22 deletions

diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java
index bcfd8f620f9c..75156bac3dc4 100644
--- a/packages/Shell/src/com/android/shell/BugreportProgressService.java
+++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java
@@ -236,6 +236,9 @@ public class BugreportProgressService extends Service {
     /** Always keep remote bugreport files created in the last day. */
     private static final long REMOTE_MIN_KEEP_AGE = DateUtils.DAY_IN_MILLIS;
 
+    /** Minimum delay for sending last update notification */
+    private static final int DELAY_NOTIFICATION_MS = 250;
+
     private final Object mLock = new Object();
 
     /** Managed bugreport info (keyed by id) */
@@ -849,6 +852,7 @@ public class BugreportProgressService extends Service {
             Log.d(TAG, "Progress #" + info.id + ": " + percentageText);
         }
         info.lastProgress.set(progress);
+        info.lastUpdate.set(System.currentTimeMillis());
 
         sendForegroundabledNotification(info.id, builder.build());
     }
@@ -1368,6 +1372,16 @@ public class BugreportProgressService extends Service {
      */
     private void sendBugreportNotification(BugreportInfo info, boolean takingScreenshot) {
 
+        final long lastUpdate = System.currentTimeMillis() - info.lastUpdate.longValue();
+        if (lastUpdate < DELAY_NOTIFICATION_MS) {
+            Log.d(TAG, "Delaying final notification for "
+                    + (DELAY_NOTIFICATION_MS - lastUpdate) + " ms ");
+            mMainThreadHandler.postDelayed(() -> {
+                sendBugreportNotification(info, takingScreenshot);
+            }, DELAY_NOTIFICATION_MS - lastUpdate);
+            return;
+        }
+
         // Since adding the details can take a while, do it before notifying user.
         addDetailsToZipFile(info);
 
@@ -1388,6 +1402,7 @@ public class BugreportProgressService extends Service {
         final Notification.Builder builder = newBaseNotification(mContext)
                 .setContentTitle(title)
                 .setTicker(title)
+                .setProgress(100 /* max value of progress percentage */, 100, false)
                 .setOnlyAlertOnce(false)
                 .setContentText(content);
 
@@ -2426,7 +2441,6 @@ public class BugreportProgressService extends Service {
             }
         }
         info.progress.set(progress);
-        info.lastUpdate.set(System.currentTimeMillis());
 
         updateProgress(info);
     }
diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
index 050a3704df1f..7bda2ea790b0 100644
--- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
+++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
@@ -193,7 +193,7 @@ public class BugreportReceiverTest {
         mService.mScreenshotDelaySec = SCREENSHOT_DELAY_SECONDS;
         // Dup the fds which are passing to startBugreport function.
         Mockito.doAnswer(invocation -> {
-            final boolean isScreenshotRequested = invocation.getArgument(6);
+            final boolean isScreenshotRequested = invocation.getArgument(7);
             if (isScreenshotRequested) {
                 mScreenshotFd = ParcelFileDescriptor.dup(invocation.getArgument(3));
             }
@@ -250,7 +250,22 @@ public class BugreportReceiverTest {
         mIDumpstateListener.onProgress(300);
         assertProgressNotification(mProgressTitle, 99);
 
-        Bundle extras = sendBugreportFinishedAndGetSharedIntent(mBugreportId);
+        Bundle extras = sendBugreportFinishedAndGetSharedIntent(mBugreportId, 1);
+        assertActionSendMultiple(extras);
+
+        assertServiceNotRunning();
+    }
+
+    @Test
+    public void testStressProgress() throws Exception {
+        sendBugreportStarted();
+        waitForScreenshotButtonEnabled(true);
+
+        for (int i = 0; i <= 1000; i++) {
+            mIDumpstateListener.onProgress(i);
+        }
+        sendBugreportFinished();
+        Bundle extras = acceptBugreportAndGetSharedIntent(mBugreportId, 1);
         assertActionSendMultiple(extras);
 
         assertServiceNotRunning();
@@ -277,7 +292,7 @@ public class BugreportReceiverTest {
         assertScreenshotButtonEnabled(false);
         waitForScreenshotButtonEnabled(true);
 
-        Bundle extras = sendBugreportFinishedAndGetSharedIntent(mBugreportId);
+        Bundle extras = sendBugreportFinishedAndGetSharedIntent(mBugreportId, 2);
         assertActionSendMultiple(extras, NO_NAME, NO_TITLE, NO_DESCRIPTION, 1);
 
         assertServiceNotRunning();
@@ -294,7 +309,7 @@ public class BugreportReceiverTest {
         // There's no indication in the UI about the screenshot finish, so just sleep like a baby...
         sleep(SAFE_SCREENSHOT_DELAY * DateUtils.SECOND_IN_MILLIS);
 
-        Bundle extras = acceptBugreportAndGetSharedIntent(mBugreportId);
+        Bundle extras = acceptBugreportAndGetSharedIntent(mBugreportId, 2);
         assertActionSendMultiple(extras, NO_NAME, NO_TITLE, NO_DESCRIPTION, 1);
 
         assertServiceNotRunning();
@@ -328,7 +343,7 @@ public class BugreportReceiverTest {
 
         assertProgressNotification(NEW_NAME, 00.00f);
 
-        Bundle extras = sendBugreportFinishedAndGetSharedIntent(TITLE);
+        Bundle extras = sendBugreportFinishedAndGetSharedIntent(TITLE, 1);
         assertActionSendMultiple(extras, NEW_NAME, TITLE, mDescription, 0);
 
         assertServiceNotRunning();
@@ -363,7 +378,7 @@ public class BugreportReceiverTest {
 
         assertProgressNotification(NEW_NAME, 00.00f);
 
-        Bundle extras = sendBugreportFinishedAndGetSharedIntent(TITLE);
+        Bundle extras = sendBugreportFinishedAndGetSharedIntent(TITLE, 1);
         assertActionSendMultiple(extras, NEW_NAME, TITLE, mDescription, 0);
 
         assertServiceNotRunning();
@@ -390,7 +405,7 @@ public class BugreportReceiverTest {
         detailsUi.descField.setText(mDescription);
         detailsUi.clickOk();
 
-        Bundle extras = sendBugreportFinishedAndGetSharedIntent(mBugreportId);
+        Bundle extras = sendBugreportFinishedAndGetSharedIntent(mBugreportId, 1);
         assertActionSendMultiple(extras, NO_NAME, NO_TITLE, mDescription, 0);
 
         assertServiceNotRunning();
@@ -441,7 +456,7 @@ public class BugreportReceiverTest {
         detailsUi.clickOk();
 
         // Finally, share bugreport.
-        Bundle extras = acceptBugreportAndGetSharedIntent(mBugreportId);
+        Bundle extras = acceptBugreportAndGetSharedIntent(mBugreportId, 1);
         assertActionSendMultiple(extras, NO_NAME, TITLE, mDescription, 0);
 
         assertServiceNotRunning();
@@ -504,7 +519,7 @@ public class BugreportReceiverTest {
         mUiBot.click(ok, "ok");
 
         // Share the bugreport.
-        mUiBot.chooseActivity(UI_NAME);
+        mUiBot.chooseActivity(UI_NAME, mContext, 1);
         Bundle extras = mListener.getExtras();
         assertActionSendMultiple(extras);
 
@@ -531,7 +546,7 @@ public class BugreportReceiverTest {
         sendBugreportFinished();
         killService();
         assertServiceNotRunning();
-        Bundle extras = acceptBugreportAndGetSharedIntent(mBugreportId);
+        Bundle extras = acceptBugreportAndGetSharedIntent(mBugreportId, 1);
         assertActionSendMultiple(extras);
     }
 
@@ -618,45 +633,49 @@ public class BugreportReceiverTest {
      * Sends a "bugreport finished" event and waits for the result.
      *
      * @param id The bugreport id for finished notification string title substitution.
+     * @param count Number of files to be shared
      * @return extras sent in the shared intent.
      */
-    private Bundle sendBugreportFinishedAndGetSharedIntent(int id) throws Exception {
+    private Bundle sendBugreportFinishedAndGetSharedIntent(int id, int count) throws Exception {
         sendBugreportFinished();
-        return acceptBugreportAndGetSharedIntent(id);
+        return acceptBugreportAndGetSharedIntent(id, count);
     }
 
     /**
      * Sends a "bugreport finished" event and waits for the result.
      *
      * @param notificationTitle The title of finished notification.
+     * @param count Number of files to be shared
      * @return extras sent in the shared intent.
      */
-    private Bundle sendBugreportFinishedAndGetSharedIntent(String notificationTitle)
+    private Bundle sendBugreportFinishedAndGetSharedIntent(String notificationTitle, int count)
             throws Exception {
         sendBugreportFinished();
-        return acceptBugreportAndGetSharedIntent(notificationTitle);
+        return acceptBugreportAndGetSharedIntent(notificationTitle, count);
     }
 
     /**
      * Accepts the notification to share the finished bugreport and waits for the result.
      *
      * @param id The bugreport id for finished notification string title substitution.
+     * @param count Number of files to be shared
      * @return extras sent in the shared intent.
      */
-    private Bundle acceptBugreportAndGetSharedIntent(int id) {
+    private Bundle acceptBugreportAndGetSharedIntent(int id, int count) {
         final String notificationTitle = mContext.getString(R.string.bugreport_finished_title, id);
-        return acceptBugreportAndGetSharedIntent(notificationTitle);
+        return acceptBugreportAndGetSharedIntent(notificationTitle, count);
     }
 
     /**
      * Accepts the notification to share the finished bugreport and waits for the result.
      *
      * @param notificationTitle The title of finished notification.
+     * @param count Number of files to be shared
      * @return extras sent in the shared intent.
      */
-    private Bundle acceptBugreportAndGetSharedIntent(String notificationTitle) {
+    private Bundle acceptBugreportAndGetSharedIntent(String notificationTitle, int count) {
         mUiBot.clickOnNotification(notificationTitle);
-        mUiBot.chooseActivity(UI_NAME);
+        mUiBot.chooseActivity(UI_NAME, mContext, count);
         return mListener.getExtras();
     }
 
diff --git a/packages/Shell/tests/src/com/android/shell/UiBot.java b/packages/Shell/tests/src/com/android/shell/UiBot.java
index ce9f70d8b977..60008a353d81 100644
--- a/packages/Shell/tests/src/com/android/shell/UiBot.java
+++ b/packages/Shell/tests/src/com/android/shell/UiBot.java
@@ -18,9 +18,12 @@ package com.android.shell;
 
 import android.app.Instrumentation;
 import android.app.StatusBarManager;
+import android.content.Context;
+import android.content.res.Resources;
 import android.os.SystemClock;
 import android.text.format.DateUtils;
 import android.util.Log;
+import android.util.PluralsMessageFormatter;
 
 import androidx.test.uiautomator.By;
 import androidx.test.uiautomator.UiDevice;
@@ -34,7 +37,9 @@ import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertNotNull;
 import static junit.framework.Assert.assertTrue;
 
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 /**
  * A helper class for UI-related testing tasks.
@@ -206,11 +211,26 @@ final class UiBot {
      *
      * @param name name of the activity as displayed in the UI (typically the value set by
      *            {@code android:label} in the manifest).
+     * @param context Context of the target application
+     * @param count Number of files to be shared
      */
-    public void chooseActivity(String name) {
+    public void chooseActivity(String name, Context context, int count) {
         // It uses an intent chooser now, so just getting the activity by text is enough...
-        final String share = mInstrumentation.getContext().getString(
-                com.android.internal.R.string.share);
+        Resources res = null;
+        try {
+            res = context.getPackageManager()
+                    .getResourcesForApplication("com.android.intentresolver");
+        } catch (Exception e)  {
+            assertNotNull("could not get resources for com.android.intentresolver", res);
+        }
+        /* Resource read is defined as a string which contains a plural
+         * which needs some formatting */
+        Map<String, Object> arguments = new HashMap<>();
+        arguments.put("count", count);
+        final String share = PluralsMessageFormatter.format(
+                res,
+                arguments,
+                res.getIdentifier("sharing_files", "string", "com.android.intentresolver"));
         boolean gotIt = mDevice.wait(Until.hasObject(By.text(share)), mTimeout);
         assertTrue("could not get share activity (" + share + ")", gotIt);
         swipeUp();