Include title and description on bugreport.zip

Prior to this change, the user-provide title and description were only
used in the ACTION_SEND_MULTIPLE intent, which was fine for the cases
where the user share the bug report with an app that used intent
extras (like an email app).

But if the app did not use the extras, or if
the user did not share the bug report right away, the info supplied by
the user would be lost.

With this change, such info will be saved into 2 new zip entries,
title.txt and description.txt

BUG: 26403310
Change-Id: I888364d14d67fb4e2f2c26cb66b21576d7ce13b4
This commit is contained in:
Felipe Leme
2016-01-06 11:38:53 -08:00
parent 2288129d52
commit 4967f737d9
2 changed files with 155 additions and 36 deletions

View File

@@ -21,6 +21,7 @@ import static com.android.shell.BugreportPrefs.STATE_SHOW;
import static com.android.shell.BugreportPrefs.getWarningState;
import java.io.BufferedOutputStream;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileDescriptor;
import java.io.FileInputStream;
@@ -28,10 +29,13 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintWriter;
import java.nio.charset.StandardCharsets;
import java.text.NumberFormat;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import java.util.zip.ZipOutputStream;
import libcore.io.Streams;
@@ -813,6 +817,9 @@ public class BugreportProgressService extends Service {
Log.e(TAG, "INTERNAL ERROR: no info for PID " + pid + ": " + mProcesses);
return;
}
addDetailsToZipFile(info);
final Intent sendIntent = buildSendIntent(mContext, info);
final Intent notifIntent;
@@ -868,7 +875,7 @@ public class BugreportProgressService extends Service {
new AsyncTask<Void, Void, Void>() {
@Override
protected Void doInBackground(Void... params) {
info.bugreportFile = zipBugreport(info.bugreportFile);
zipBugreport(info);
sendBugreportNotification(context, info);
return null;
}
@@ -879,34 +886,91 @@ public class BugreportProgressService extends Service {
* Zips a bugreport file, returning the path to the new file (or to the
* original in case of failure).
*/
private static File zipBugreport(File bugreportFile) {
String bugreportPath = bugreportFile.getAbsolutePath();
String zippedPath = bugreportPath.replace(".txt", ".zip");
private static void zipBugreport(BugreportInfo info) {
final String bugreportPath = info.bugreportFile.getAbsolutePath();
final String zippedPath = bugreportPath.replace(".txt", ".zip");
Log.v(TAG, "zipping " + bugreportPath + " as " + zippedPath);
File bugreportZippedFile = new File(zippedPath);
try (InputStream is = new FileInputStream(bugreportFile);
final File bugreportZippedFile = new File(zippedPath);
try (InputStream is = new FileInputStream(info.bugreportFile);
ZipOutputStream zos = new ZipOutputStream(
new BufferedOutputStream(new FileOutputStream(bugreportZippedFile)))) {
ZipEntry entry = new ZipEntry(bugreportFile.getName());
entry.setTime(bugreportFile.lastModified());
zos.putNextEntry(entry);
int totalBytes = Streams.copy(is, zos);
Log.v(TAG, "size of original bugreport: " + totalBytes + " bytes");
zos.closeEntry();
// Delete old file;
boolean deleted = bugreportFile.delete();
addEntry(zos, info.bugreportFile.getName(), is);
// Delete old file
final boolean deleted = info.bugreportFile.delete();
if (deleted) {
Log.v(TAG, "deleted original bugreport (" + bugreportPath + ")");
} else {
Log.e(TAG, "could not delete original bugreport (" + bugreportPath + ")");
}
return bugreportZippedFile;
info.bugreportFile = bugreportZippedFile;
} catch (IOException e) {
Log.e(TAG, "exception zipping file " + zippedPath, e);
return bugreportFile; // Return original.
}
}
/**
* Adds the user-provided info into the bugreport zip file.
* <p>
* If user provided a title, it will be saved into a {@code title.txt} entry; similarly, the
* description will be saved on {@code description.txt}.
*/
private void addDetailsToZipFile(BugreportInfo info) {
// 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();
final File tmpZip = new File(dir, "tmp-" + info.bugreportFile.getName());
Log.d(TAG, "Writing temporary zip file (" + tmpZip + ")");
try (ZipFile oldZip = new ZipFile(info.bugreportFile);
ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(tmpZip))) {
// First copy contents from original zip.
Enumeration<? extends ZipEntry> entries = oldZip.entries();
while (entries.hasMoreElements()) {
final ZipEntry entry = entries.nextElement();
final String entryName = entry.getName();
if (!entry.isDirectory()) {
addEntry(zos, entryName, entry.getTime(), oldZip.getInputStream(entry));
} else {
Log.w(TAG, "skipping directory entry: " + entryName);
}
}
// Then add the user-provided info.
addEntry(zos, "title.txt", info.title);
addEntry(zos, "description.txt", info.description);
} catch (IOException e) {
Log.e(TAG, "exception zipping file " + tmpZip, e);
return;
}
if (!tmpZip.renameTo(info.bugreportFile)) {
Log.e(TAG, "Could not rename " + tmpZip + " to " + info.bugreportFile);
}
}
private static void addEntry(ZipOutputStream zos, String entry, String text)
throws IOException {
if (DEBUG) Log.v(TAG, "adding entry '" + entry + "': " + text);
if (!TextUtils.isEmpty(text)) {
addEntry(zos, entry, new ByteArrayInputStream(text.getBytes(StandardCharsets.UTF_8)));
}
}
private static void addEntry(ZipOutputStream zos, String entryName, InputStream is)
throws IOException {
addEntry(zos, entryName, System.currentTimeMillis(), is);
}
private static void addEntry(ZipOutputStream zos, String entryName, long timestamp,
InputStream is) throws IOException {
final ZipEntry entry = new ZipEntry(entryName);
entry.setTime(timestamp);
zos.putNextEntry(entry);
final int totalBytes = Streams.copy(is, zos);
if (DEBUG) Log.v(TAG, "size of '" + entryName + "' entry: " + totalBytes + " bytes");
zos.closeEntry();
}
/**
* Find the best matching {@link Account} based on build properties.
*/

View File

@@ -60,6 +60,7 @@ import android.support.test.uiautomator.UiDevice;
import android.support.test.uiautomator.UiObject;
import android.test.InstrumentationTestCase;
import android.test.suitebuilder.annotation.LargeTest;
import android.text.TextUtils;
import android.text.format.DateUtils;
import android.util.Log;
@@ -103,6 +104,12 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
private static final String NAME = "BUG, Y U NO REPORT?";
private static final String NEW_NAME = "Bug_Forrest_Bug";
private static final String TITLE = "Wimbugdom Champion 2015";
private static final String NO_DESCRIPTION = null;
private static final String NO_NAME = null;
private static final String NO_SCREENSHOT = null;
private static final String NO_TITLE = null;
private String mDescription;
private String mPlainTextPath;
@@ -157,8 +164,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
Bundle extras =
sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath, mScreenshotPath);
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, ZIP_FILE,
null, 1, true);
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, ZIP_FILE,
NAME, NO_TITLE, NO_DESCRIPTION, 1, true);
assertServiceNotRunning();
}
@@ -174,13 +181,14 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
Bundle extras =
sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath, mScreenshotPath);
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, ZIP_FILE,
null, 2, true);
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, ZIP_FILE,
NAME, NO_TITLE, NO_DESCRIPTION, 2, true);
assertServiceNotRunning();
}
public void testProgress_changeDetails() throws Exception {
public void testProgress_changeDetailsInvalidInput() throws Exception {
resetProperties();
sendBugreportStarted(1000);
waitForScreenshotButtonEnabled(true);
@@ -219,8 +227,47 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
Bundle extras = sendBugreportFinishedAndGetSharedIntent(PID, mPlainTextPath,
mScreenshotPath);
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NEW_NAME, TITLE,
mDescription, 1, true);
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, TITLE,
NEW_NAME, TITLE, mDescription, 1, true);
assertServiceNotRunning();
}
public void testProgress_changeDetailsPlainBugreport() throws Exception {
changeDetailsTest(true);
}
public void testProgress_changeDetailsZippedBugreport() throws Exception {
changeDetailsTest(false);
}
public void changeDetailsTest(boolean plainText) throws Exception {
resetProperties();
sendBugreportStarted(1000);
waitForScreenshotButtonEnabled(true);
DetailsUi detailsUi = new DetailsUi(mUiBot);
// Check initial name.
String actualName = detailsUi.nameField.getText().toString();
assertEquals("Wrong value on field 'name'", NAME, actualName);
// Change fields.
detailsUi.reOpen();
detailsUi.nameField.setText(NEW_NAME);
detailsUi.titleField.setText(TITLE);
detailsUi.descField.setText(mDescription);
detailsUi.clickOk();
assertPropertyValue(NAME_PROPERTY, NEW_NAME);
assertProgressNotification(NEW_NAME, "0.00%");
Bundle extras = sendBugreportFinishedAndGetSharedIntent(PID,
plainText? mPlainTextPath : mZipPath, mScreenshotPath);
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, TITLE,
NEW_NAME, TITLE, mDescription, 1, true);
assertServiceNotRunning();
}
@@ -270,8 +317,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
// Finally, share bugreport.
Bundle extras = acceptBugreportAndGetSharedIntent();
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, NAME, TITLE,
mDescription, 1, waitScreenshot);
assertActionSendMultiple(extras, BUGREPORT_FILE, BUGREPORT_CONTENT, PID, TITLE,
NAME, TITLE, mDescription, 1, waitScreenshot);
assertServiceNotRunning();
}
@@ -296,7 +343,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
// Share the bugreport.
mUiBot.chooseActivity(UI_NAME);
Bundle extras = mListener.getExtras();
assertActionSendMultiple(extras, BUGREPORT_CONTENT, null);
assertActionSendMultiple(extras, BUGREPORT_CONTENT, NO_SCREENSHOT);
// Make sure it's hidden now.
int newState = BugreportPrefs.getWarningState(mContext, BugreportPrefs.STATE_UNKNOWN);
@@ -314,13 +361,13 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
}
public void testBugreportFinished_plainBugreportAndNoScreenshot() throws Exception {
Bundle extras = sendBugreportFinishedAndGetSharedIntent(mPlainTextPath, null);
assertActionSendMultiple(extras, BUGREPORT_CONTENT, null);
Bundle extras = sendBugreportFinishedAndGetSharedIntent(mPlainTextPath, NO_SCREENSHOT);
assertActionSendMultiple(extras, BUGREPORT_CONTENT, NO_SCREENSHOT);
}
public void testBugreportFinished_zippedBugreportAndNoScreenshot() throws Exception {
Bundle extras = sendBugreportFinishedAndGetSharedIntent(mZipPath, null);
assertActionSendMultiple(extras, BUGREPORT_CONTENT, null);
Bundle extras = sendBugreportFinishedAndGetSharedIntent(mZipPath, NO_SCREENSHOT);
assertActionSendMultiple(extras, BUGREPORT_CONTENT, NO_SCREENSHOT);
}
private void cancelExistingNotifications() {
@@ -426,8 +473,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
*/
private void assertActionSendMultiple(Bundle extras, String bugreportContent,
String screenshotContent) throws IOException {
assertActionSendMultiple(extras, bugreportContent, screenshotContent, PID, null, ZIP_FILE,
null, 0, false);
assertActionSendMultiple(extras, bugreportContent, screenshotContent, PID, ZIP_FILE,
NO_NAME, NO_TITLE, NO_DESCRIPTION, 0, false);
}
/**
@@ -437,14 +484,16 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
* @param bugreportContent expected content in the bugreport file
* @param screenshotContent expected content in the screenshot file (sent by dumpstate), if any
* @param pid emulated dumpstate pid
* @param name bugreport name as provided by the user
* @param title bugreport name as provided by the user (or received by dumpstate)
* @param name expected subject
* @param name bugreport name as provided by the user (or received by dumpstate)
* @param title bugreport name as provided by the user
* @param description bugreport description as provided by the user
* @param numberScreenshots expected number of screenshots taken by Shell.
* @param renamedScreenshots whether the screenshots are expected to be renamed
*/
private void assertActionSendMultiple(Bundle extras, String bugreportContent,
String screenshotContent, int pid, String name, String title, String description,
String screenshotContent, int pid, String subject,
String name, String title, String description,
int numberScreenshots, boolean renamedScreenshots) throws IOException {
String body = extras.getString(Intent.EXTRA_TEXT);
assertContainsRegex("missing build info",
@@ -455,7 +504,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
assertContainsRegex("missing description", description, body);
}
assertEquals("wrong subject", title, extras.getString(Intent.EXTRA_SUBJECT));
assertEquals("wrong subject", subject, extras.getString(Intent.EXTRA_SUBJECT));
List<Uri> attachments = extras.getParcelableArrayList(Intent.EXTRA_STREAM);
int expectedNumberScreenshots = numberScreenshots;
@@ -478,6 +527,12 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
}
assertNotNull("did not get .zip attachment", zipUri);
assertZipContent(zipUri, BUGREPORT_FILE, BUGREPORT_CONTENT);
if (!TextUtils.isEmpty(title)) {
assertZipContent(zipUri, "title.txt", title);
}
if (!TextUtils.isEmpty(description)) {
assertZipContent(zipUri, "description.txt", description);
}
// URI of the screenshot taken by dumpstate.
Uri externalScreenshotUri = null;