diff --git a/packages/Shell/res/layout/dialog_bugreport_info.xml b/packages/Shell/res/layout/dialog_bugreport_info.xml new file mode 100644 index 0000000000000..5d1e9f9949469 --- /dev/null +++ b/packages/Shell/res/layout/dialog_bugreport_info.xml @@ -0,0 +1,43 @@ + + + + + + + + diff --git a/packages/Shell/res/values/strings.xml b/packages/Shell/res/values/strings.xml index 5c2557643121b..a7f2df5a739af 100644 --- a/packages/Shell/res/values/strings.xml +++ b/packages/Shell/res/values/strings.xml @@ -42,4 +42,19 @@ unnamed + + Details + + + Bug report details + + + Short name + + 1-line summary + + Detailed description diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java index e902589ddc514..82ee710f5bcce 100644 --- a/packages/Shell/src/com/android/shell/BugreportProgressService.java +++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java @@ -29,16 +29,18 @@ import java.io.InputStream; import java.io.PrintWriter; import java.text.NumberFormat; import java.util.ArrayList; -import java.util.Date; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; import libcore.io.Streams; +import com.android.internal.annotations.VisibleForTesting; import com.google.android.collect.Lists; import android.accounts.Account; import android.accounts.AccountManager; +import android.annotation.SuppressLint; +import android.app.AlertDialog; import android.app.Notification; import android.app.Notification.Action; import android.app.NotificationManager; @@ -46,6 +48,7 @@ import android.app.PendingIntent; import android.app.Service; import android.content.ClipData; import android.content.Context; +import android.content.DialogInterface; import android.content.Intent; import android.content.res.Configuration; import android.net.Uri; @@ -59,10 +62,17 @@ import android.os.Parcelable; import android.os.Process; import android.os.SystemProperties; import android.support.v4.content.FileProvider; +import android.text.TextUtils; import android.text.format.DateUtils; import android.util.Log; import android.util.Patterns; import android.util.SparseArray; +import android.view.View; +import android.view.WindowManager; +import android.view.View.OnFocusChangeListener; +import android.view.inputmethod.EditorInfo; +import android.widget.Button; +import android.widget.EditText; import android.widget.Toast; /** @@ -103,19 +113,23 @@ public class BugreportProgressService extends Service { // Internal intents used on notification actions. static final String INTENT_BUGREPORT_CANCEL = "android.intent.action.BUGREPORT_CANCEL"; static final String INTENT_BUGREPORT_SHARE = "android.intent.action.BUGREPORT_SHARE"; + static final String INTENT_BUGREPORT_INFO_LAUNCH = + "android.intent.action.BUGREPORT_INFO_LAUNCH"; static final String EXTRA_BUGREPORT = "android.intent.extra.BUGREPORT"; static final String EXTRA_SCREENSHOT = "android.intent.extra.SCREENSHOT"; static final String EXTRA_PID = "android.intent.extra.PID"; static final String EXTRA_MAX = "android.intent.extra.MAX"; static final String EXTRA_NAME = "android.intent.extra.NAME"; + 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"; private static final int MSG_SERVICE_COMMAND = 1; private static final int MSG_POLL = 2; /** Polling frequency, in milliseconds. */ - static final long POLLING_FREQUENCY = 2000; + static final long POLLING_FREQUENCY = 2 * DateUtils.SECOND_IN_MILLIS; /** How long (in ms) a dumpstate process will be monitored if it didn't show progress. */ private static final long INACTIVITY_TIMEOUT = 3 * DateUtils.MINUTE_IN_MILLIS; @@ -124,8 +138,9 @@ public class BugreportProgressService extends Service { private static final String DUMPSTATE_PREFIX = "dumpstate."; private static final String PROGRESS_SUFFIX = ".progress"; private static final String MAX_SUFFIX = ".max"; + private static final String NAME_SUFFIX = ".name"; - /** System property (and value) used for stop dumpstate. */ + /** System property (and value) used to stop dumpstate. */ private static final String CTL_STOP = "ctl.stop"; private static final String BUGREPORT_SERVICE = "bugreportplus"; @@ -135,6 +150,8 @@ public class BugreportProgressService extends Service { private Looper mServiceLooper; private ServiceHandler mServiceHandler; + private final BugreportInfoDialog mInfoDialog = new BugreportInfoDialog(); + @Override public void onCreate() { HandlerThread thread = new HandlerThread("BugreportProgressServiceThread", @@ -242,6 +259,9 @@ public class BugreportProgressService extends Service { } onBugreportFinished(pid, intent); break; + case INTENT_BUGREPORT_INFO_LAUNCH: + launchBugreportInfoDialog(pid); + break; case INTENT_BUGREPORT_SHARE: shareBugreport(pid); break; @@ -312,6 +332,13 @@ public class BugreportProgressService extends Service { final String percentText = nf.format((double) info.progress / info.max); final Action cancelAction = new Action.Builder(null, context.getString( com.android.internal.R.string.cancel), newCancelIntent(context, info)).build(); + final Intent infoIntent = new Intent(context, BugreportProgressService.class); + infoIntent.setAction(INTENT_BUGREPORT_INFO_LAUNCH); + infoIntent.putExtra(EXTRA_PID, info.pid); + final Action infoAction = new Action.Builder(null, + context.getString(R.string.bugreport_info_action), + PendingIntent.getService(context, info.pid, infoIntent, + PendingIntent.FLAG_UPDATE_CURRENT)).build(); final String title = context.getString(R.string.bugreport_in_progress_title); final String name = @@ -328,6 +355,7 @@ public class BugreportProgressService extends Service { .setLocalOnly(true) .setColor(context.getColor( com.android.internal.R.color.system_notification_accent_color)) + .addAction(infoAction) .addAction(cancelAction) .build(); @@ -341,7 +369,8 @@ public class BugreportProgressService extends Service { final Intent intent = new Intent(INTENT_BUGREPORT_CANCEL); intent.setClass(context, BugreportProgressService.class); intent.putExtra(EXTRA_PID, info.pid); - return PendingIntent.getService(context, 0, intent, PendingIntent.FLAG_CANCEL_CURRENT); + return PendingIntent.getService(context, info.pid, intent, + PendingIntent.FLAG_UPDATE_CURRENT); } /** @@ -356,7 +385,7 @@ public class BugreportProgressService extends Service { } stopSelfWhenDone(); } - if (DEBUG) Log.v(TAG, "stopProgress(" + pid + "): cancel notification"); + Log.v(TAG, "stopProgress(" + pid + "): cancel notification"); NotificationManager.from(getApplicationContext()).cancel(TAG, pid); } @@ -364,8 +393,14 @@ public class BugreportProgressService extends Service { * Cancels a bugreport upon user's request. */ private void cancel(int pid) { - Log.i(TAG, "Cancelling PID " + pid + " on user's request"); - SystemProperties.set(CTL_STOP, BUGREPORT_SERVICE); + Log.v(TAG, "cancel: pid=" + pid); + synchronized (mProcesses) { + BugreportInfo info = mProcesses.get(pid); + if (info != null && !info.finished) { + Log.i(TAG, "Cancelling bugreport service (pid=" + pid + ") on user's request"); + setSystemProperty(CTL_STOP, BUGREPORT_SERVICE); + } + } stopProgress(pid); } @@ -393,7 +428,6 @@ public class BugreportProgressService extends Service { final int progress = SystemProperties.getInt(progressKey, 0); if (progress == 0) { Log.v(TAG, "System property " + progressKey + " is not set yet"); - continue; } final int max = SystemProperties.getInt(DUMPSTATE_PREFIX + pid + MAX_SUFFIX, 0); final boolean maxChanged = max > 0 && max != info.max; @@ -426,6 +460,30 @@ public class BugreportProgressService extends Service { } } + /** + * Fetches a {@link BugreportInfo} for a given process and launches a dialog where the user can + * change its values. + */ + private void launchBugreportInfoDialog(int pid) { + // Copy values so it doesn't lock mProcesses while UI is being updated + final String name, title, description; + synchronized (mProcesses) { + final BugreportInfo info = mProcesses.get(pid); + if (info == null) { + Log.w(TAG, "No bugreport info for PID " + pid); + return; + } + name = info.name; + title = info.title; + description = info.description; + } + + // Closes the notification bar first. + sendBroadcast(new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS)); + + mInfoDialog.initialize(getApplicationContext(), pid, name, title, description); + } + /** * Finishes the service when it's not monitoring any more processes. */ @@ -440,7 +498,11 @@ public class BugreportProgressService extends Service { } } + /** + * Handles the BUGREPORT_FINISHED intent sent by {@code dumpstate}. + */ private void onBugreportFinished(int pid, Intent intent) { + mInfoDialog.onBugreportFinished(pid); final Context context = getApplicationContext(); BugreportInfo info; synchronized (mProcesses) { @@ -453,6 +515,7 @@ public class BugreportProgressService extends Service { } info.bugreportFile = getFileExtra(intent, EXTRA_BUGREPORT); info.screenshotFile = getFileExtra(intent, EXTRA_SCREENSHOT); + info.finished = true; } final Configuration conf = context.getResources().getConfiguration(); @@ -494,21 +557,32 @@ public class BugreportProgressService extends Service { /** * Build {@link Intent} that can be used to share the given bugreport. */ - private static Intent buildSendIntent(Context context, Uri bugreportUri, Uri screenshotUri) { + private static Intent buildSendIntent(Context context, BugreportInfo info) { + // Files are kept on private storage, so turn into Uris that we can + // grant temporary permissions for. + final Uri bugreportUri = getUri(context, info.bugreportFile); + final Uri screenshotUri = getUri(context, info.screenshotFile); + final Intent intent = new Intent(Intent.ACTION_SEND_MULTIPLE); final String mimeType = "application/vnd.android.bugreport"; intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION); intent.addCategory(Intent.CATEGORY_DEFAULT); intent.setType(mimeType); - intent.putExtra(Intent.EXTRA_SUBJECT, bugreportUri.getLastPathSegment()); + final String subject = info.title != null ? info.title : bugreportUri.getLastPathSegment(); + intent.putExtra(Intent.EXTRA_SUBJECT, subject); // EXTRA_TEXT should be an ArrayList, but some clients are expecting a single String. // So, to avoid an exception on Intent.migrateExtraStreamToClipData(), we need to manually // create the ClipData object with the attachments URIs. - String messageBody = String.format("Build info: %s\nSerial number:%s", - SystemProperties.get("ro.build.description"), SystemProperties.get("ro.serialno")); - intent.putExtra(Intent.EXTRA_TEXT, messageBody); + StringBuilder messageBody = new StringBuilder("Build info: ") + .append(SystemProperties.get("ro.build.description")) + .append("\nSerial number: ") + .append(SystemProperties.get("ro.serialno")); + if (!TextUtils.isEmpty(info.description)) { + messageBody.append("\nDescription: ").append(info.description); + } + intent.putExtra(Intent.EXTRA_TEXT, messageBody.toString()); final ClipData clipData = new ClipData(null, new String[] { mimeType }, new ClipData.Item(null, null, null, bugreportUri)); final ArrayList attachments = Lists.newArrayList(bugreportUri); @@ -542,12 +616,7 @@ public class BugreportProgressService extends Service { return; } } - // Files are kept on private storage, so turn into Uris that we can - // grant temporary permissions for. - final Uri bugreportUri = getUri(context, info.bugreportFile); - final Uri screenshotUri = getUri(context, info.screenshotFile); - - final Intent sendIntent = buildSendIntent(context, bugreportUri, screenshotUri); + final Intent sendIntent = buildSendIntent(context, info); final Intent notifIntent; // Send through warning dialog by default @@ -580,13 +649,17 @@ public class BugreportProgressService extends Service { .setContentTitle(title) .setTicker(title) .setContentText(context.getString(R.string.bugreport_finished_text)) - .setContentIntent(PendingIntent.getService(context, 0, shareIntent, - PendingIntent.FLAG_CANCEL_CURRENT)) + .setContentIntent(PendingIntent.getService(context, info.pid, shareIntent, + PendingIntent.FLAG_UPDATE_CURRENT)) .setDeleteIntent(newCancelIntent(context, info)) .setLocalOnly(true) .setColor(context.getColor( com.android.internal.R.color.system_notification_accent_color)); + if (!TextUtils.isEmpty(info.name)) { + builder.setContentInfo(info.name); + } + NotificationManager.from(context).notify(TAG, info.pid, builder.build()); } @@ -684,6 +757,231 @@ public class BugreportProgressService extends Service { } } + private static boolean setSystemProperty(String key, String value) { + try { + if (DEBUG) Log.v(TAG, "Setting system property" + key + " to " + value); + SystemProperties.set(key, value); + } catch (IllegalArgumentException e) { + Log.e(TAG, "Could not set property " + key + " to " + value, e); + return false; + } + return true; + } + + /** + * Updates the system property used by {@code dumpstate} to rename the final bugreport files. + */ + private boolean setBugreportNameProperty(int pid, String name) { + Log.d(TAG, "Updating bugreport name to " + name); + final String key = DUMPSTATE_PREFIX + pid + NAME_SUFFIX; + return setSystemProperty(key, name); + } + + /** + * Updates the user-provided details of a bugreport. + */ + private void updateBugreportInfo(int pid, String name, String title, String description) { + synchronized (mProcesses) { + final BugreportInfo info = mProcesses.get(pid); + if (info == null) { + Log.w(TAG, "No bugreport info for PID " + pid); + return; + } + info.title = title; + info.description = description; + if (name != null && !info.name.equals(name)) { + info.name = name; + updateProgress(info); + } + } + } + + /** + * Checks whether a character is valid on bugreport names. + */ + @VisibleForTesting + static boolean isValid(char c) { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') + || c == '_' || c == '-'; + } + + /** + * Helper class encapsulating the UI elements and logic used to display a dialog where user + * can change the details of a bugreport. + */ + private final class BugreportInfoDialog { + private EditText mInfoName; + private EditText mInfoTitle; + private EditText mInfoDescription; + private AlertDialog mDialog; + private Button mOkButton; + private int mPid; + + /** + * Last "committed" value of the bugreport name. + *

+ * Once initially set, it's only updated when user clicks the OK button. + */ + private String mSavedName; + + /** + * Last value of the bugreport name as entered by the user. + *

+ * Every time it's changed the equivalent system property is changed as well, but if the + * user clicks CANCEL, the old value (stored on {@code mSavedName} is restored. + *

+ * This logic handles the corner-case scenario where {@code dumpstate} finishes after the + * user changed the name but didn't clicked OK yet (for example, because the user is typing + * the description). The only drawback is that if the user changes the name while + * {@code dumpstate} is running but clicks CANCEL after it finishes, then the final name + * will be the one that has been canceled. But when {@code dumpstate} finishes the {code + * name} UI is disabled and the old name restored anyways, so the user will be "alerted" of + * such drawback. + */ + private String mTempName; + + /** + * Sets its internal state and displays the dialog. + */ + private synchronized void initialize(Context context, int pid, String name, String title, + String description) { + // First initializes singleton. + if (mDialog == null) { + @SuppressLint("InflateParams") + // It's ok pass null ViewRoot on AlertDialogs. + final View view = View.inflate(context, R.layout.dialog_bugreport_info, null); + + mInfoName = (EditText) view.findViewById(R.id.name); + mInfoTitle = (EditText) view.findViewById(R.id.title); + mInfoDescription = (EditText) view.findViewById(R.id.description); + + mInfoName.setOnFocusChangeListener(new OnFocusChangeListener() { + + @Override + public void onFocusChange(View v, boolean hasFocus) { + if (hasFocus) { + return; + } + sanitizeName(); + } + }); + + mDialog = new AlertDialog.Builder(context) + .setView(view) + .setTitle(context.getString(R.string.bugreport_info_dialog_title)) + .setCancelable(false) + .setPositiveButton(context.getString(com.android.internal.R.string.ok), + null) + .setNegativeButton(context.getString(com.android.internal.R.string.cancel), + new DialogInterface.OnClickListener() + { + @Override + public void onClick(DialogInterface dialog, int id) + { + if (!mTempName.equals(mSavedName)) { + // Must restore dumpstate's name since it was changed + // before user clicked OK. + setBugreportNameProperty(mPid, mSavedName); + } + } + }) + .create(); + + mDialog.getWindow().setAttributes( + new WindowManager.LayoutParams( + WindowManager.LayoutParams.TYPE_SYSTEM_DIALOG)); + + } + + // Then set fields. + mSavedName = mTempName = name; + mPid = pid; + if (!TextUtils.isEmpty(name)) { + mInfoName.setText(name); + } + if (!TextUtils.isEmpty(title)) { + mInfoTitle.setText(title); + } + if (!TextUtils.isEmpty(description)) { + mInfoDescription.setText(description); + } + + // And finally display it. + mDialog.show(); + + // TODO: in a traditional AlertDialog, when the positive button is clicked the + // dialog is always closed, but we need to validate the name first, so we need to + // get a reference to it, which is only available after it's displayed. + // It would be cleaner to use a regular dialog instead, but let's keep this + // workaround for now and change it later, when we add another button to take + // extra screenshots. + if (mOkButton == null) { + mOkButton = mDialog.getButton(DialogInterface.BUTTON_POSITIVE); + mOkButton.setOnClickListener(new View.OnClickListener() { + + @Override + public void onClick(View view) { + sanitizeName(); + final String name = mInfoName.getText().toString(); + final String title = mInfoTitle.getText().toString(); + final String description = mInfoDescription.getText().toString(); + + updateBugreportInfo(mPid, name, title, description); + mDialog.dismiss(); + } + }); + } + } + + /** + * Sanitizes the user-provided value for the {@code name} field, automatically replacing + * invalid characters if necessary. + */ + private synchronized void sanitizeName() { + String name = mInfoName.getText().toString(); + if (name.equals(mTempName)) { + if (DEBUG) Log.v(TAG, "name didn't change, no need to sanitize: " + name); + return; + } + final StringBuilder safeName = new StringBuilder(name.length()); + boolean changed = false; + for (int i = 0; i < name.length(); i++) { + final char c = name.charAt(i); + if (isValid(c)) { + safeName.append(c); + } else { + changed = true; + safeName.append('_'); + } + } + if (changed) { + Log.v(TAG, "changed invalid name '" + name + "' to '" + safeName + "'"); + name = safeName.toString(); + mInfoName.setText(name); + } + mTempName = name; + + // Must update system property for the cases where dumpstate finishes + // while the user is still entering other fields (like title or + // description) + setBugreportNameProperty(mPid, name); + } + + /** + * Notifies the dialog that the bugreport has finished so it disables the {@code name} + * field. + *

Once the bugreport is finished dumpstate has already generated the final files, so + * changing the name would have no effect. + */ + private synchronized void onBugreportFinished(int pid) { + if (mInfoName != null) { + mInfoName.setEnabled(false); + mInfoName.setText(mSavedName); + } + } + + } + /** * Information about a bugreport process while its in progress. */ @@ -703,6 +1001,18 @@ public class BugreportProgressService extends Service { */ String name; + /** + * User-provided, one-line summary of the bug; when set, will be used as the subject + * of the {@link Intent#ACTION_SEND_MULTIPLE} intent. + */ + String title; + + /** + * User-provided, detailed description of the bugreport; when set, will be added to the body + * of the {@link Intent#ACTION_SEND_MULTIPLE} intent. + */ + String description; + /** * Maximum progress of the bugreport generation. */ @@ -761,6 +1071,7 @@ public class BugreportProgressService extends Service { public String toString() { final float percent = ((float) progress * 100 / max); return "pid: " + pid + ", name: " + name + ", finished: " + finished + + "\n\ttitle: " + title + "\n\tdescription: " + description + "\n\tfile: " + bugreportFile + "\n\tscreenshot: " + screenshotFile + "\n\tprogress: " + progress + "/" + max + "(" + percent + ")" + "\n\tlast_update: " + getFormattedLastUpdate(); diff --git a/packages/Shell/tests/Android.mk b/packages/Shell/tests/Android.mk index 62a37bc70f4ee..1e0eaace35dac 100644 --- a/packages/Shell/tests/Android.mk +++ b/packages/Shell/tests/Android.mk @@ -8,9 +8,7 @@ LOCAL_SRC_FILES := $(call all-java-files-under, src) LOCAL_JAVA_LIBRARIES := android.test.runner -# TODO: update and/or remove LOCAL_STATIC_JAVA_LIBRARIES := ub-uiautomator -#LOCAL_STATIC_JAVA_LIBRARIES := android-support-v4 mockito-target ub-uiautomator LOCAL_PACKAGE_NAME := ShellTests LOCAL_INSTRUMENTATION_FOR := Shell diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java index 1f4d749f39928..7f609faac83d7 100644 --- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java +++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java @@ -94,7 +94,11 @@ public class BugreportReceiverTest extends InstrumentationTestCase { private static final int PID = 42; private static final String PROGRESS_PROPERTY = "dumpstate.42.progress"; private static final String MAX_PROPERTY = "dumpstate.42.max"; + private static final String NAME_PROPERTY = "dumpstate.42.name"; 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 String mDescription; private String mPlainTextPath; private String mZipPath; @@ -120,10 +124,17 @@ public class BugreportReceiverTest extends InstrumentationTestCase { createTextFile(mScreenshotPath, SCREENSHOT_CONTENT); createZipFile(mZipPath, BUGREPORT_FILE, BUGREPORT_CONTENT); + // Creates a multi-line description. + StringBuilder sb = new StringBuilder(); + for (int i = 1; i <= 20; i++) { + sb.append("All work and no play makes Shell a dull app!\n"); + } + mDescription = sb.toString(); + BugreportPrefs.setWarningState(mContext, BugreportPrefs.STATE_HIDE); } - public void testFullWorkflow() throws Exception { + public void testProgress() throws Exception { resetProperties(); sendBugreportStarted(1000); @@ -145,6 +156,81 @@ public class BugreportReceiverTest extends InstrumentationTestCase { assertServiceNotRunning(); } + public void testProgress_changeDetails() throws Exception { + resetProperties(); + sendBugreportStarted(1000); + + DetailsUi detailsUi = new DetailsUi(mUiBot); + + // Check initial name. + String actualName = detailsUi.nameField.getText().toString(); + assertEquals("Wrong value on field 'name'", NAME, actualName); + + // Change name - it should have changed system property once focus is changed. + detailsUi.nameField.setText(NEW_NAME); + detailsUi.focusAwayFromName(); + assertPropertyValue(NAME_PROPERTY, NEW_NAME); + + // Cancel the dialog to make sure property was restored. + detailsUi.clickCancel(); + assertPropertyValue(NAME_PROPERTY, NAME); + + // Now try to set an invalid name. + detailsUi.reOpen(); + detailsUi.nameField.setText("/etc/passwd"); + detailsUi.clickOk(); + assertPropertyValue(NAME_PROPERTY, "_etc_passwd"); + + // Finally, make the real changes. + 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, mPlainTextPath, + mScreenshotPath); + assertActionSendMultiple(extras, TITLE, mDescription, BUGREPORT_CONTENT, SCREENSHOT_CONTENT); + + assertServiceNotRunning(); + } + + public void testProgress_bugreportFinishedWhileChangingDetails() throws Exception { + resetProperties(); + sendBugreportStarted(1000); + + DetailsUi detailsUi = new DetailsUi(mUiBot); + + // Finish the bugreport while user's still typing the name. + detailsUi.nameField.setText(NEW_NAME); + sendBugreportFinished(PID, mPlainTextPath, mScreenshotPath); + + // Wait until the share notifcation is received... + mUiBot.getNotification(mContext.getString(R.string.bugreport_finished_title)); + // ...then close notification bar. + mContext.sendBroadcast(new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS)); + + // Make sure UI was updated properly. + assertFalse("didn't disable name on UI", detailsUi.nameField.isEnabled()); + assertEquals("didn't revert name on UI", NAME, detailsUi.nameField.getText().toString()); + + // Finish changing other fields. + detailsUi.titleField.setText(TITLE); + detailsUi.descField.setText(mDescription); + detailsUi.clickOk(); + + // Finally, share bugreport. + Bundle extras = acceptBugreportAndGetSharedIntent(); + assertActionSendMultiple(extras, TITLE, mDescription, BUGREPORT_CONTENT, + SCREENSHOT_CONTENT); + + assertServiceNotRunning(); + } + public void testBugreportFinished_withWarning() throws Exception { // Explicitly shows the warning. BugreportPrefs.setWarningState(mContext, BugreportPrefs.STATE_SHOW); @@ -204,14 +290,18 @@ public class BugreportReceiverTest extends InstrumentationTestCase { private void assertProgressNotification(String name, String percent) { // TODO: it current looks for 3 distinct objects, without taking advantage of their // relationship. - String title = mContext.getString(R.string.bugreport_in_progress_title); - Log.v(TAG, "Looking for progress notification title: '" + title+ "'"); - mUiBot.getNotification(title); + openProgressNotification(); Log.v(TAG, "Looking for progress notification details: '" + name + "-" + percent + "'"); mUiBot.getObject(name); mUiBot.getObject(percent); } + private void openProgressNotification() { + String title = mContext.getString(R.string.bugreport_in_progress_title); + Log.v(TAG, "Looking for progress notification title: '" + title + "'"); + mUiBot.getNotification(title); + } + void resetProperties() { // TODO: call method to remove property instead SystemProperties.set(PROGRESS_PROPERTY, "0"); @@ -270,7 +360,6 @@ public class BugreportReceiverTest extends InstrumentationTestCase { /** * Sends a "bugreport finished" intent. - * */ private void sendBugreportFinished(Integer pid, String bugreportPath, String screenshotPath) { Intent intent = new Intent(INTENT_BUGREPORT_FINISHED); @@ -292,13 +381,21 @@ public class BugreportReceiverTest extends InstrumentationTestCase { */ private void assertActionSendMultiple(Bundle extras, String bugreportContent, String screenshotContent) throws IOException { + assertActionSendMultiple(extras, ZIP_FILE, null, bugreportContent, screenshotContent); + } + + private void assertActionSendMultiple(Bundle extras, String subject, String description, + String bugreportContent, String screenshotContent) throws IOException { String body = extras.getString(Intent.EXTRA_TEXT); assertContainsRegex("missing build info", SystemProperties.get("ro.build.description"), body); assertContainsRegex("missing serial number", SystemProperties.get("ro.serialno"), body); + if (description != null) { + assertContainsRegex("missing description", description, body); + } - assertEquals("wrong subject", ZIP_FILE, extras.getString(Intent.EXTRA_SUBJECT)); + assertEquals("wrong subject", subject, extras.getString(Intent.EXTRA_SUBJECT)); List attachments = extras.getParcelableArrayList(Intent.EXTRA_STREAM); int expectedSize = screenshotContent != null ? 2 : 1; @@ -355,6 +452,11 @@ public class BugreportReceiverTest extends InstrumentationTestCase { fail("Did not find entry '" + entryName + "' on file '" + uri + "'"); } + private void assertPropertyValue(String key, String expectedValue) { + String actualValue = SystemProperties.get(key); + assertEquals("Wrong value for property '" + key + "'", expectedValue, actualValue); + } + private void assertServiceNotRunning() { String service = BugreportProgressService.class.getName(); assertFalse("Service '" + service + "' is still running", isServiceRunning(service)); @@ -402,4 +504,55 @@ public class BugreportReceiverTest extends InstrumentationTestCase { Log.v(TAG, "Path for '" + file + "': " + path); return path; } + + /** + * Helper class containing the UiObjects present in the bugreport info dialog. + */ + private final class DetailsUi { + + final UiObject detailsButton; + final UiObject nameField; + final UiObject titleField; + final UiObject descField; + final UiObject okButton; + final UiObject cancelButton; + + /** + * Gets the UI objects by opening the progress notification and clicking DETAILS. + */ + DetailsUi(UiBot uiBot) { + openProgressNotification(); + detailsButton = mUiBot.getVisibleObject( + mContext.getString(R.string.bugreport_info_action).toUpperCase()); + mUiBot.click(detailsButton, "details_button"); + // TODO: unhardcode resource ids + nameField = mUiBot.getVisibleObjectById("com.android.shell:id/name"); + titleField = mUiBot.getVisibleObjectById("com.android.shell:id/title"); + descField = mUiBot.getVisibleObjectById("com.android.shell:id/description"); + okButton = mUiBot.getObjectById("android:id/button1"); + cancelButton = mUiBot.getObjectById("android:id/button2"); + } + + /** + * Takes focus away from the name field so it can be validated. + */ + void focusAwayFromName() { + mUiBot.click(titleField, "title_field"); // Change focus. + mUiBot.pressBack(); // Dismiss keyboard. + } + + void reOpen() { + openProgressNotification(); + mUiBot.click(detailsButton, "details_button"); + + } + + void clickOk() { + mUiBot.click(okButton, "details_ok_button"); + } + + void clickCancel() { + mUiBot.click(cancelButton, "details_cancel_button"); + } + } } diff --git a/packages/Shell/tests/src/com/android/shell/UiBot.java b/packages/Shell/tests/src/com/android/shell/UiBot.java index c87172720ad71..384c3daa51c92 100644 --- a/packages/Shell/tests/src/com/android/shell/UiBot.java +++ b/packages/Shell/tests/src/com/android/shell/UiBot.java @@ -78,6 +78,17 @@ final class UiBot { return getVisibleObject(text); } + /** + * Gets an object that might not yet be available in current UI. + * + * @param id Object's fully-qualified resource id (like {@code android:id/button1}) + */ + public UiObject getObjectById(String id) { + boolean gotIt = mDevice.wait(Until.hasObject(By.res(id)), mTimeout); + assertTrue("object with id '(" + id + "') not visible yet", gotIt); + return getVisibleObjectById(id); + } + /** * Gets an object which is guaranteed to be present in the current UI. * @@ -89,6 +100,18 @@ final class UiBot { return uiObject; } + /** + * Gets an object which is guaranteed to be present in the current UI. + * + * @param text Object's text as displayed by the UI. + */ + public UiObject getVisibleObjectById(String id) { + UiObject uiObject = mDevice.findObject(new UiSelector().resourceId(id)); + assertTrue("could not find object with id '" + id+ "'", uiObject.exists()); + return uiObject; + } + + /** * Clicks on a UI element. * @@ -151,4 +174,8 @@ final class UiBot { click(activity, name); } } + + public void pressBack() { + mDevice.pressBack(); + } } diff --git a/packages/Shell/tests/src/com/android/shell/UtilitiesTest.java b/packages/Shell/tests/src/com/android/shell/UtilitiesTest.java new file mode 100644 index 0000000000000..51b7ba8529a82 --- /dev/null +++ b/packages/Shell/tests/src/com/android/shell/UtilitiesTest.java @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.shell; + +import android.test.suitebuilder.annotation.SmallTest; +import junit.framework.TestCase; +import static com.android.shell.BugreportProgressService.isValid; + +@SmallTest +public class UtilitiesTest extends TestCase { + + public void testIsValidChar_valid() { + String validChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_"; + for (int i = 0; i < validChars.length(); i++) { + char c = validChars.charAt(i); + assertTrue("char '" + c + "' should be valid", isValid(c)); + } + } + + public void testIsValidChar_invalid() { + String validChars = "/.<>;:'\'\"\\+=*&^%$#@!`~áéíóúãñÂÊÎÔÛ"; + for (int i = 0; i < validChars.length(); i++) { + char c = validChars.charAt(i); + assertFalse("char '" + c + "' should not be valid", isValid(c)); + } + } +}