From 39b6acbb55b037ba6dbeb092eb93caddd277a2cd Mon Sep 17 00:00:00 2001
From: Rhed Jao <rhedjao@google.com>
Date: Mon, 7 Dec 2020 17:58:26 +0800
Subject: [PATCH] Fixes an error handling in BugreportProgressService

Calls onError function when bugreport is finished and file is empty.

Bug: 174314124
Bug: 175287931
Test: atest BugreportReceiverTest
Change-Id: I4542568fd2d2ad1c75c7c3b223accca4995938a3
Merged-In: I4542568fd2d2ad1c75c7c3b223accca4995938a3
---
 .../android/shell/BugreportProgressService.java   |  9 +++++----
 .../com/android/shell/BugreportReceiverTest.java  | 15 ++++++++++++++-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java
index f7b1c5bf9c5b1..2b71892ee8a55 100644
--- a/packages/Shell/src/com/android/shell/BugreportProgressService.java
+++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java
@@ -380,6 +380,11 @@ public class BugreportProgressService extends Service {
         public void onFinished() {
             mInfo.renameBugreportFile();
             mInfo.renameScreenshots();
+            if (mInfo.bugreportFile.length() == 0) {
+                Log.e(TAG, "Bugreport file empty. File path = " + mInfo.bugreportFile);
+                onError(BUGREPORT_ERROR_RUNTIME);
+                return;
+            }
             synchronized (mLock) {
                 sendBugreportFinishedBroadcastLocked();
                 mMainThreadHandler.post(() -> mInfoDialog.onBugreportFinished(mInfo));
@@ -408,10 +413,6 @@ public class BugreportProgressService extends Service {
         @GuardedBy("mLock")
         private void sendBugreportFinishedBroadcastLocked() {
             final String bugreportFilePath = mInfo.bugreportFile.getAbsolutePath();
-            if (mInfo.bugreportFile.length() == 0) {
-                Log.e(TAG, "Bugreport file empty. File path = " + bugreportFilePath);
-                return;
-            }
             if (mInfo.type == BugreportParams.BUGREPORT_MODE_REMOTE) {
                 sendRemoteBugreportFinishedBroadcast(mContext, bugreportFilePath,
                         mInfo.bugreportFile);
diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
index 43147cdfc86dd..89cdeaea199c7 100644
--- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
+++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
@@ -512,6 +512,17 @@ public class BugreportReceiverTest {
         assertEquals("Didn't change state", STATE_HIDE, newState);
     }
 
+    @Test
+    public void testBugreportFinished_withEmptyBugreportFile() throws Exception {
+        sendBugreportStarted();
+
+        IoUtils.closeQuietly(mBugreportFd);
+        mBugreportFd = null;
+        sendBugreportFinished();
+
+        assertServiceNotRunning();
+    }
+
     @Test
     public void testShareBugreportAfterServiceDies() throws Exception {
         sendBugreportStarted();
@@ -647,7 +658,9 @@ public class BugreportReceiverTest {
      * Callbacks to service to finish the bugreport.
      */
     private void sendBugreportFinished() throws Exception {
-        writeZipFile(mBugreportFd, BUGREPORT_FILE, BUGREPORT_CONTENT);
+        if (mBugreportFd != null) {
+            writeZipFile(mBugreportFd, BUGREPORT_FILE, BUGREPORT_CONTENT);
+        }
         if (mScreenshotFd != null) {
             writeScreenshotFile(mScreenshotFd, SCREENSHOT_CONTENT);
         }