From 8449e85f3ce1185e3bddf78cb6b009f26351b605 Mon Sep 17 00:00:00 2001
From: Abhijeet Kaur <abkaur@google.com>
Date: Mon, 30 Mar 2020 12:23:37 +0100
Subject: Track bugreports immediately after triggering a bugreport

Remove an old hack that was put in place as bugreport ID was updated
a bit later in the bugreport generation phase.
This change removes the race condition around triggering bugreports
almost instantly.

Bug: 152292912
Test: Trigger consecutive bugreports from ActivityManager WAI. (Fails
without this change)

Change-Id: Ib08bc2c7d8ae765894998e5bfa0d9b6f3efa088b
---
 .../com/android/shell/BugreportProgressService.java  | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

(limited to 'packages/Shell/src')

diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java
index 5d363f34bc67..bb6212818de5 100644
--- a/packages/Shell/src/com/android/shell/BugreportProgressService.java
+++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java
@@ -351,9 +351,6 @@ public class BugreportProgressService extends Service {
         @Override
         public void onProgress(float progress) {
             synchronized (mLock) {
-                if (progress == 0) {
-                    trackInfoWithIdLocked();
-                }
                 checkProgressUpdatedLocked(mInfo, (int) progress);
             }
         }
@@ -365,7 +362,6 @@ public class BugreportProgressService extends Service {
         @Override
         public void onError(@BugreportErrorCode int errorCode) {
             synchronized (mLock) {
-                trackInfoWithIdLocked();
                 stopProgressLocked(mInfo.id);
             }
             Log.e(TAG, "Bugreport API callback onError() errorCode = " + errorCode);
@@ -382,10 +378,10 @@ public class BugreportProgressService extends Service {
         }
 
         /**
-         * Reads bugreport id and links it to the bugreport info to track the bugreport's
-         * progress/completion/error. id is incremented in dumpstate code. This function is called
-         * when dumpstate calls one of the callback functions (onProgress, onFinished, onError)
-         * after the id has been incremented.
+         * Reads bugreport id and links it to the bugreport info to track a bugreport that is in
+         * process. id is incremented in the dumpstate code.
+         * We do not track a bugreport if there is already a bugreport with the same id being
+         * tracked.
          */
         @GuardedBy("mLock")
         private void trackInfoWithIdLocked() {
@@ -408,7 +404,6 @@ public class BugreportProgressService extends Service {
                 sendRemoteBugreportFinishedBroadcast(mContext, bugreportFilePath,
                         mInfo.bugreportFile);
             } else {
-                trackInfoWithIdLocked();
                 cleanupOldFiles(MIN_KEEP_COUNT, MIN_KEEP_AGE, mBugreportsDir);
                 final Intent intent = new Intent(INTENT_BUGREPORT_FINISHED);
                 intent.putExtra(EXTRA_BUGREPORT, bugreportFilePath);
@@ -638,8 +633,11 @@ public class BugreportProgressService extends Service {
 
         BugreportCallbackImpl bugreportCallback = new BugreportCallbackImpl(info);
         try {
-            mBugreportManager.startBugreport(bugreportFd, screenshotFd,
-                    new BugreportParams(bugreportType), executor, bugreportCallback);
+            synchronized (mLock) {
+                mBugreportManager.startBugreport(bugreportFd, screenshotFd,
+                        new BugreportParams(bugreportType), executor, bugreportCallback);
+                bugreportCallback.trackInfoWithIdLocked();
+            }
         } catch (RuntimeException e) {
             Log.i(TAG, "Error in generating bugreports: ", e);
             // The binder call didn't go through successfully, so need to close the fds.
-- 
cgit v1.2.3-59-g8ed1b