OSDN Git Service

Allows users to add details about a bugreport in progress.
authorFelipe Leme <felipeal@google.com>
Fri, 11 Dec 2015 23:07:14 +0000 (15:07 -0800)
committerFelipe Leme <felipeal@google.com>
Wed, 16 Dec 2015 19:36:04 +0000 (11:36 -0800)
The "bugreport in progress" notification now have a "DETAILS" button
that when clicked opens a dialog window displaying the following fields:

  - Name: short name for the bugreport, will be used as part of the
    final files (and by default is the timestamp sent by dumpstate)
  - Title: a 1-line title for the bugreport, will be used as the subject
    in the final message.
  - Description: a detailed description for the bug.

The main advantage of such dialog is that it allows users to enter more
info about a bugreport while it's being generated, rather then when the
bugreport is finished (since of the user doesn't remember what the
context was when the problem happened).

BUG: 25794470
BUG: 10676443
Change-Id: I0d1dba2a94ad989e541415a2a59475619a2e3d13

packages/Shell/res/layout/dialog_bugreport_info.xml [new file with mode: 0644]
packages/Shell/res/values/strings.xml
packages/Shell/src/com/android/shell/BugreportProgressService.java
packages/Shell/tests/Android.mk
packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
packages/Shell/tests/src/com/android/shell/UiBot.java
packages/Shell/tests/src/com/android/shell/UtilitiesTest.java [new file with mode: 0644]

diff --git a/packages/Shell/res/layout/dialog_bugreport_info.xml b/packages/Shell/res/layout/dialog_bugreport_info.xml
new file mode 100644 (file)
index 0000000..5d1e9f9
--- /dev/null
@@ -0,0 +1,43 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- 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.
+-->
+
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:orientation="vertical"
+    android:layout_width="wrap_content"
+    android:layout_height="wrap_content">
+    <EditText
+        android:id="@+id/name"
+        android:maxLength="30"
+        android:singleLine="true"
+        android:inputType="textNoSuggestions"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:hint="@string/bugreport_info_name"/>
+    <EditText
+        android:id="@+id/title"
+        android:maxLength="80"
+        android:singleLine="true"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:hint="@string/bugreport_info_title"/>
+    <EditText
+        android:id="@+id/description"
+        android:singleLine="false"
+        android:inputType="textMultiLine"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:hint="@string/bugreport_info_description"/>
+</LinearLayout>
index 5c25576..a7f2df5 100644 (file)
     <!-- Title for bug reports received from dumpstate without a name. [CHAR LIMIT=30]-->
     <string name="bugreport_unnamed">unnamed</string>
 
+    <!-- Title of the notification action that opens the dialog for the user-defined bug report details. -->
+    <string name="bugreport_info_action">Details</string>
+
+    <!--  Title of the dialog asking for user-defined bug report details like name, title, and description. -->
+    <string name="bugreport_info_dialog_title">Bug report details</string>
+
+    <!-- Text of the hint asking for the bug report name, which when set will define a suffix in the
+         bug report file names. [CHAR LIMIT=30] -->
+    <string name="bugreport_info_name">Short name</string>
+    <!-- Text of hint asking for the bug report title, which when set will define the
+         Subject of the email message. [CHAR LIMIT=60] -->
+    <string name="bugreport_info_title">1-line summary</string>
+    <!-- Text of hint asking for the bug report description, which when set will describe
+         what the bug report is about. [CHAR LIMIT=NONE] -->
+    <string name="bugreport_info_description">Detailed description</string>
 </resources>
index e902589..82ee710 100644 (file)
@@ -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;
@@ -427,6 +461,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.
      */
     private void stopSelfWhenDone() {
@@ -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<Uri> 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.
+         * <p>
+         * 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.
+         * <p>
+         * 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.
+         * <p>
+         * 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.
+         * <p>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.
      */
@@ -704,6 +1002,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.
          */
         int max;
@@ -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();
index 62a37bc..1e0eaac 100644 (file)
@@ -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
index 1f4d749..7f609fa 100644 (file)
@@ -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<Uri> 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");
+        }
+    }
 }
index c871727..384c3da 100644 (file)
@@ -79,6 +79,17 @@ final class UiBot {
     }
 
     /**
+     * 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.
      *
      * @param text Object's text as displayed by the UI.
@@ -90,6 +101,18 @@ final class UiBot {
     }
 
     /**
+     * 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.
      *
      * @param uiObject UI element to be clicked.
@@ -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 (file)
index 0000000..51b7ba8
--- /dev/null
@@ -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));
+        }
+    }
+}