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();