Merge "Save bugreport info on share intent."
diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java
index 5c807e1..df8fad4 100644
--- a/packages/Shell/src/com/android/shell/BugreportProgressService.java
+++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java
@@ -65,6 +65,7 @@
 import android.os.IBinder;
 import android.os.Looper;
 import android.os.Message;
+import android.os.Parcel;
 import android.os.Parcelable;
 import android.os.SystemProperties;
 import android.os.Vibrator;
@@ -108,7 +109,7 @@
  * </ol>
  */
 public class BugreportProgressService extends Service {
-    static final String TAG = "Shell";
+    private static final String TAG = "BugreportProgressService";
     private static final boolean DEBUG = false;
 
     private static final String AUTHORITY = "com.android.shell";
@@ -137,6 +138,7 @@
     static final String EXTRA_TITLE = "android.intent.extra.TITLE";
     static final String EXTRA_DESCRIPTION = "android.intent.extra.DESCRIPTION";
     static final String EXTRA_ORIGINAL_INTENT = "android.intent.extra.ORIGINAL_INTENT";
+    static final String EXTRA_INFO = "android.intent.extra.INFO";
 
     private static final int MSG_SERVICE_COMMAND = 1;
     private static final int MSG_POLL = 2;
@@ -325,7 +327,7 @@
                     takeScreenshot(pid, true);
                     break;
                 case INTENT_BUGREPORT_SHARE:
-                    shareBugreport(pid);
+                    shareBugreport(pid, (BugreportInfo) intent.getParcelableExtra(EXTRA_INFO));
                     break;
                 case INTENT_BUGREPORT_CANCEL:
                     cancel(pid);
@@ -483,6 +485,7 @@
         if (mProcesses.indexOfKey(pid) < 0) {
             Log.w(TAG, "PID not watched: " + pid);
         } else {
+            Log.d(TAG, "Removing PID " + pid);
             mProcesses.remove(pid);
         }
         stopSelfWhenDone();
@@ -675,6 +678,11 @@
         final int msgId;
         if (taken) {
             info.addScreenshot(screenshotFile);
+            if (info.finished) {
+                Log.d(TAG, "Screenshot finished after bugreport; updating share notification");
+                info.renameScreenshots(mScreenshotsDir);
+                sendBugreportNotification(mContext, info);
+            }
             msgId = R.string.bugreport_screenshot_taken;
         } else {
             // TODO: try again using Framework APIs instead of relying on screencap.
@@ -814,12 +822,13 @@
      * Shares the bugreport upon user's request by issuing a {@link Intent#ACTION_SEND_MULTIPLE}
      * intent, but issuing a warning dialog the first time.
      */
-    private void shareBugreport(int pid) {
-        final BugreportInfo info = getInfo(pid);
+    private void shareBugreport(int pid, BugreportInfo sharedInfo) {
+        BugreportInfo info = getInfo(pid);
         if (info == null) {
-            // Should not happen, so log if it does...
-            Log.e(TAG, "INTERNAL ERROR: no info for PID " + pid + ": " + mProcesses);
-            return;
+            // Service was terminated but notification persisted
+            info = sharedInfo;
+            Log.d(TAG, "shareBugreport(): no info for PID " + pid + " on managed processes ("
+                    + mProcesses + "), using info from intent instead (" + info + ")");
         }
 
         addDetailsToZipFile(info);
@@ -850,6 +859,7 @@
         shareIntent.setClass(context, BugreportProgressService.class);
         shareIntent.setAction(INTENT_BUGREPORT_SHARE);
         shareIntent.putExtra(EXTRA_PID, info.pid);
+        shareIntent.putExtra(EXTRA_INFO, info);
 
         final String title = context.getString(R.string.bugreport_finished_title);
         final Notification.Builder builder = new Notification.Builder(context)
@@ -919,6 +929,11 @@
      * description will be saved on {@code description.txt}.
      */
     private void addDetailsToZipFile(BugreportInfo info) {
+        if (info.bugreportFile == null) {
+            // One possible reason is a bug in the Parcelization code.
+            Log.e(TAG, "INTERNAL ERROR: no bugreportFile on " + info);
+            return;
+        }
         // It's not possible to add a new entry into an existing file, so we need to create a new
         // zip, copy all entries, then rename it.
         final File dir = info.bugreportFile.getParentFile();
@@ -1281,7 +1296,7 @@
     /**
      * Information about a bugreport process while its in progress.
      */
-    private static final class BugreportInfo {
+    private static final class BugreportInfo implements Parcelable {
         private final Context context;
 
         /**
@@ -1325,6 +1340,11 @@
         long lastUpdate = System.currentTimeMillis();
 
         /**
+         * Time of the last progress update when Parcel was created.
+         */
+        String formattedLastUpdate;
+
+        /**
          * Path of the main bugreport file.
          */
         File bugreportFile;
@@ -1403,6 +1423,11 @@
         }
 
         String getFormattedLastUpdate() {
+            if (context == null) {
+                // Restored from Parcel
+                return formattedLastUpdate == null ?
+                        Long.toString(lastUpdate) : formattedLastUpdate;
+            }
             return DateUtils.formatDateTime(context, lastUpdate,
                     DateUtils.FORMAT_SHOW_DATE | DateUtils.FORMAT_SHOW_TIME);
         }
@@ -1416,5 +1441,74 @@
                     + "\n\tprogress: " + progress + "/" + max + "(" + percent + ")"
                     + "\n\tlast_update: " + getFormattedLastUpdate();
         }
+
+        // Parcelable contract
+        protected BugreportInfo(Parcel in) {
+            context = null;
+            pid = in.readInt();
+            name = in.readString();
+            title = in.readString();
+            description = in.readString();
+            max = in.readInt();
+            progress = in.readInt();
+            lastUpdate = in.readLong();
+            formattedLastUpdate = in.readString();
+            bugreportFile = readFile(in);
+
+            int screenshotSize = in.readInt();
+            for (int i = 1; i <= screenshotSize; i++) {
+                  screenshotFiles.add(readFile(in));
+            }
+
+            finished = in.readInt() == 1;
+            screenshotCounter = in.readInt();
+        }
+
+        @Override
+        public void writeToParcel(Parcel dest, int flags) {
+            dest.writeInt(pid);
+            dest.writeString(name);
+            dest.writeString(title);
+            dest.writeString(description);
+            dest.writeInt(max);
+            dest.writeInt(progress);
+            dest.writeLong(lastUpdate);
+            dest.writeString(getFormattedLastUpdate());
+            writeFile(dest, bugreportFile);
+
+            dest.writeInt(screenshotFiles.size());
+            for (File screenshotFile : screenshotFiles) {
+                writeFile(dest, screenshotFile);
+            }
+
+            dest.writeInt(finished ? 1 : 0);
+            dest.writeInt(screenshotCounter);
+        }
+
+        @Override
+        public int describeContents() {
+            return 0;
+        }
+
+        private void writeFile(Parcel dest, File file) {
+            dest.writeString(file == null ? null : file.getPath());
+        }
+
+        private File readFile(Parcel in) {
+            final String path = in.readString();
+            return path == null ? null : new File(path);
+        }
+
+        public static final Parcelable.Creator<BugreportInfo> CREATOR =
+                new Parcelable.Creator<BugreportInfo>() {
+            public BugreportInfo createFromParcel(Parcel source) {
+                return new BugreportInfo(source);
+            }
+
+            public BugreportInfo[] newArray(int size) {
+                return new BugreportInfo[size];
+            }
+        };
+
     }
 }
diff --git a/packages/Shell/src/com/android/shell/BugreportReceiver.java b/packages/Shell/src/com/android/shell/BugreportReceiver.java
index c8898b9..9afa900 100644
--- a/packages/Shell/src/com/android/shell/BugreportReceiver.java
+++ b/packages/Shell/src/com/android/shell/BugreportReceiver.java
@@ -19,7 +19,6 @@
 import static com.android.shell.BugreportProgressService.EXTRA_BUGREPORT;
 import static com.android.shell.BugreportProgressService.EXTRA_ORIGINAL_INTENT;
 import static com.android.shell.BugreportProgressService.INTENT_BUGREPORT_FINISHED;
-import static com.android.shell.BugreportProgressService.TAG;
 import static com.android.shell.BugreportProgressService.getFileExtra;
 
 import java.io.File;
@@ -37,6 +36,7 @@
  * {@link Intent#ACTION_SEND_MULTIPLE}.
  */
 public class BugreportReceiver extends BroadcastReceiver {
+    private static final String TAG = "BugreportReceiver";
 
     /**
      * Always keep the newest 8 bugreport files; 4 reports and 4 screenshots are
@@ -74,7 +74,11 @@
         new AsyncTask<Void, Void, Void>() {
             @Override
             protected Void doInBackground(Void... params) {
-                FileUtils.deleteOlderFiles(bugreportFile.getParentFile(), minCount, minAge);
+                try {
+                    FileUtils.deleteOlderFiles(bugreportFile.getParentFile(), minCount, minAge);
+                } catch (RuntimeException e) {
+                    Log.e(TAG, "RuntimeException deleting old files", e);
+                }
                 result.finish();
                 return null;
             }
diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
index d1a07ea..52e1b56 100644
--- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
+++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
@@ -65,6 +65,7 @@
 import android.util.Log;
 
 import com.android.shell.ActionSendMultipleConsumerActivity.CustomActionSendMultipleListener;
+import com.android.shell.BugreportProgressService;
 
 /**
  * Integration tests for {@link BugreportReceiver}.
@@ -89,6 +90,9 @@
     // Timeout for UI operations, in milliseconds.
     private static final int TIMEOUT = (int) BugreportProgressService.POLLING_FREQUENCY * 4;
 
+    // Timeout for when waiting for a screenshot to finish.
+    private static final int SAFE_SCREENSHOT_DELAY = SCREENSHOT_DELAY_SECONDS + 10;
+
     private static final String BUGREPORTS_DIR = "bugreports";
     private static final String BUGREPORT_FILE = "test_bugreport.txt";
     private static final String ZIP_FILE = "test_bugreport.zip";
@@ -109,6 +113,7 @@
     private static final String NO_NAME = null;
     private static final String NO_SCREENSHOT = null;
     private static final String NO_TITLE = null;
+    private static final Integer NO_PID = null;
 
     private String mDescription;
 
@@ -122,6 +127,7 @@
 
     @Override
     protected void setUp() throws Exception {
+        Log.i(TAG, "#### setup() on " + getName());
         Instrumentation instrumentation = getInstrumentation();
         mContext = instrumentation.getTargetContext();
         mUiBot = new UiBot(UiDevice.getInstance(instrumentation), TIMEOUT);
@@ -164,13 +170,21 @@
 
         Bundle extras =
                 sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath, mScreenshotPath);
-        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, ZIP_FILE,
+        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, PID, ZIP_FILE,
                 NAME, NO_TITLE, NO_DESCRIPTION, 1, true);
 
         assertServiceNotRunning();
     }
 
     public void testProgress_takeExtraScreenshot() throws Exception {
+        takeExtraScreenshotTest(false);
+    }
+
+    public void testProgress_takeExtraScreenshotServiceDiesAfterScreenshotTaken() throws Exception {
+        takeExtraScreenshotTest(true);
+    }
+
+    private void takeExtraScreenshotTest(boolean serviceDies) throws Exception {
         resetProperties();
         sendBugreportStarted(1000);
 
@@ -179,14 +193,49 @@
         assertScreenshotButtonEnabled(false);
         waitForScreenshotButtonEnabled(true);
 
-        Bundle extras =
-                sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath, mScreenshotPath);
-        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, ZIP_FILE,
+        sendBugreportFinished(PID, mPlainTextPath, mScreenshotPath);
+
+        if (serviceDies) {
+            waitShareNotification();
+            killService();
+        }
+
+        Bundle extras = acceptBugreportAndGetSharedIntent();
+        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, PID, ZIP_FILE,
                 NAME, NO_TITLE, NO_DESCRIPTION, 2, true);
 
         assertServiceNotRunning();
     }
 
+    public void testScreenshotFinishesAfterBugreport() throws Exception {
+        screenshotFinishesAfterBugreportTest(false);
+    }
+
+    public void testScreenshotFinishesAfterBugreportAndServiceDiesBeforeSharing() throws Exception {
+        screenshotFinishesAfterBugreportTest(true);
+    }
+
+    private void screenshotFinishesAfterBugreportTest(boolean serviceDies) throws Exception {
+        resetProperties();
+
+        sendBugreportStarted(1000);
+        sendBugreportFinished(PID, mPlainTextPath, NO_SCREENSHOT);
+        waitShareNotification();
+
+        // There's no indication in the UI about the screenshot finish, so just sleep like a baby...
+        Thread.sleep(SAFE_SCREENSHOT_DELAY * DateUtils.SECOND_IN_MILLIS);
+
+        if (serviceDies) {
+            killService();
+        }
+
+        Bundle extras = acceptBugreportAndGetSharedIntent();
+        assertActionSendMultiple(extras, BUGREPORT_CONTENT, NO_SCREENSHOT, PID, ZIP_FILE,
+                NAME, NO_TITLE, NO_DESCRIPTION, 1, true);
+
+        assertServiceNotRunning();
+    }
+
     public void testProgress_changeDetailsInvalidInput() throws Exception {
 
         resetProperties();
@@ -227,7 +276,7 @@
 
         Bundle extras = sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath,
                 mScreenshotPath);
-        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, TITLE,
+        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, PID, TITLE,
                 NEW_NAME, TITLE, mDescription, 1, true);
 
         assertServiceNotRunning();
@@ -266,7 +315,7 @@
 
         Bundle extras = sendBugreportFinishedAndGetSharedIntent(PID,
                 plainText? mPlainTextPath : mZipPath, mScreenshotPath);
-        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, TITLE,
+        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, PID, TITLE,
                 NEW_NAME, TITLE, mDescription, 1, true);
 
         assertServiceNotRunning();
@@ -317,8 +366,8 @@
 
         // Finally, share bugreport.
         Bundle extras = acceptBugreportAndGetSharedIntent();
-        assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, TITLE,
-                NAME, TITLE, mDescription, 1, waitScreenshot);
+        assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT, PID, TITLE,
+                NAME, TITLE, mDescription, 1, true);
 
         assertServiceNotRunning();
     }
@@ -328,7 +377,7 @@
         BugreportPrefs.setWarningState(mContext, BugreportPrefs.STATE_SHOW);
 
         // Send notification and click on share.
-        sendBugreportFinished(null, mPlainTextPath, null);
+        sendBugreportFinished(NO_PID, mPlainTextPath, null);
         acceptBugreport();
 
         // Handle the warning
@@ -350,6 +399,13 @@
         assertEquals("Didn't change state", BugreportPrefs.STATE_HIDE, newState);
     }
 
+    public void testShareBugreportAfterServiceDies() throws Exception {
+        sendBugreportFinished(NO_PID, mPlainTextPath, NO_SCREENSHOT);
+        killService();
+        Bundle extras = acceptBugreportAndGetSharedIntent();
+        assertActionSendMultiple(extras, BUGREPORT_CONTENT, NO_SCREENSHOT);
+    }
+
     public void testBugreportFinished_plainBugreportAndScreenshot() throws Exception {
         Bundle extras = sendBugreportFinishedAndGetSharedIntent(mPlainTextPath, mScreenshotPath);
         assertActionSendMultiple(extras, BUGREPORT_CONTENT, SCREENSHOT_CONTENT);
@@ -444,6 +500,13 @@
     }
 
     /**
+     * Waits for the notification to share the finished bugreport.
+     */
+    private void waitShareNotification() {
+        mUiBot.getNotification(mContext.getString(R.string.bugreport_finished_title));
+    }
+
+    /**
      * Accepts the notification to share the finished bugreport.
      */
     private void acceptBugreport() {
@@ -512,7 +575,8 @@
             expectedNumberScreenshots ++; // Add screenshot received by dumpstate
         }
         int expectedSize = expectedNumberScreenshots + 1; // All screenshots plus the bugreport file
-        assertEquals("wrong number of attachments", expectedSize, attachments.size());
+        assertEquals("wrong number of attachments (" + attachments + ")",
+                expectedSize, attachments.size());
 
         // Need to interact through all attachments, since order is not guaranteed.
         Uri zipUri = null;
@@ -607,6 +671,15 @@
         assertFalse("Service '" + service + "' is still running", isServiceRunning(service));
     }
 
+    private void killService() {
+        waitForService(true);
+        Log.v(TAG, "Stopping service");
+        boolean stopped = mContext.stopService(new Intent(mContext, BugreportProgressService.class));
+        Log.d(TAG, "stopService returned " + stopped);
+        waitForService(false);
+        assertServiceNotRunning();  // Sanity check.
+    }
+
     private boolean isServiceRunning(String name) {
         ActivityManager manager = (ActivityManager) mContext
                 .getSystemService(Context.ACTIVITY_SERVICE);
@@ -618,6 +691,33 @@
         return false;
     }
 
+    private void waitForService(boolean expectRunning) {
+        String service = BugreportProgressService.class.getName();
+        boolean actualRunning;
+        for (int i = 1; i <= 5; i++) {
+            actualRunning = isServiceRunning(service);
+            Log.d(TAG, "Attempt " + i + " to check status of service '"
+                    + service + "': expected=" + expectRunning + ", actual= " + actualRunning);
+            if (actualRunning == expectRunning) {
+                return;
+            }
+            try {
+                Thread.sleep(DateUtils.SECOND_IN_MILLIS);
+            } catch (InterruptedException e) {
+                Log.w(TAG, "thread interrupted");
+                Thread.currentThread().interrupt();
+            }
+        }
+        if (!expectRunning) {
+            // Typically happens when service is waiting for a screenshot to finish.
+            Log.w(TAG, "Service didn't stop; try to kill it again");
+            killService();
+            return;
+        }
+
+        fail("Service status didn't change to " + expectRunning);
+    }
+
     private static void createTextFile(String path, String content) throws IOException {
         Log.v(TAG, "createFile(" + path + ")");
         try (Writer writer = new BufferedWriter(new OutputStreamWriter(
@@ -669,7 +769,7 @@
 
     private UiObject waitForScreenshotButtonEnabled(boolean expectedEnabled) throws Exception {
         UiObject screenshotButton = getScreenshotButton();
-        int maxAttempts = SCREENSHOT_DELAY_SECONDS + 5;
+        int maxAttempts = SAFE_SCREENSHOT_DELAY;
         int i = 0;
         do {
             boolean enabled = screenshotButton.isEnabled();