diff --git a/packages/Shell/AndroidManifest.xml b/packages/Shell/AndroidManifest.xml index e11c063f57fbf..54e291f904eb9 100644 --- a/packages/Shell/AndroidManifest.xml +++ b/packages/Shell/AndroidManifest.xml @@ -246,23 +246,6 @@ android:excludeFromRecents="true" android:exported="false" /> - - - - - - - - - - - - - diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java index 665bde3415158..1b35770ccbd7e 100644 --- a/packages/Shell/src/com/android/shell/BugreportProgressService.java +++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java @@ -56,16 +56,11 @@ import android.os.FileUtils; import android.os.Handler; import android.os.HandlerThread; import android.os.IBinder; -import android.os.IBinder.DeathRecipient; -import android.os.IDumpstate; -import android.os.IDumpstateListener; -import android.os.IDumpstateToken; import android.os.Looper; import android.os.Message; import android.os.Parcel; import android.os.ParcelFileDescriptor; import android.os.Parcelable; -import android.os.RemoteException; import android.os.ServiceManager; import android.os.SystemProperties; import android.os.UserHandle; @@ -80,7 +75,6 @@ import android.util.SparseArray; import android.view.ContextThemeWrapper; import android.view.IWindowManager; import android.view.View; -import android.view.View.OnFocusChangeListener; import android.view.WindowManager; import android.widget.Button; import android.widget.EditText; @@ -122,31 +116,9 @@ import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; /** - * Service used to keep progress of bugreport processes ({@code dumpstate} and - * {@code BugreportManager}). + * Service used to trigger system bugreports. *

- * There can be 2 workflows. One workflow via ({@code dumpstate}) is: - *

    - *
  1. When {@code dumpstate} starts, it sends a {@code BUGREPORT_STARTED} with a sequential id, - * its pid, and the estimated total effort. - *
  2. {@link BugreportReceiver} receives the intent and delegates it to this service. - *
  3. Upon start, this service: - *
      - *
    1. Issues a system notification so user can watch the progress (which is 0% initially). - *
    2. Polls the {@link SystemProperties} for updates on the {@code dumpstate} progress. - *
    3. If the progress changed, it updates the system notification. - *
    - *
  4. As {@code dumpstate} progresses, it updates the system property. - *
  5. When {@code dumpstate} finishes, it sends a {@code BUGREPORT_FINISHED} intent. - *
  6. {@link BugreportReceiver} receives the intent and delegates it to this service, which in - * turn: - *
      - *
    1. Updates the system notification so user can share the bugreport. - *
    2. Stops monitoring that {@code dumpstate} process. - *
    3. Stops itself if it doesn't have any process left to monitor. - *
    - *
- * The second workflow using Bugreport API({@code BugreportManager}) is: + * The workflow uses Bugreport API({@code BugreportManager}) and is as follows: *
    *
  1. System apps like Settings or SysUI broadcasts {@code BUGREPORT_REQUESTED}. *
  2. {@link BugreportRequestedReceiver} receives the intent and delegates it to this service. @@ -164,18 +136,14 @@ public class BugreportProgressService extends Service { private static final String AUTHORITY = "com.android.shell"; - // External intents sent by dumpstate. - static final String INTENT_BUGREPORT_STARTED = - "com.android.internal.intent.action.BUGREPORT_STARTED"; - static final String INTENT_BUGREPORT_FINISHED = - "com.android.internal.intent.action.BUGREPORT_FINISHED"; - static final String INTENT_REMOTE_BUGREPORT_FINISHED = - "com.android.internal.intent.action.REMOTE_BUGREPORT_FINISHED"; - // External intent used to trigger bugreport API. static final String INTENT_BUGREPORT_REQUESTED = "com.android.internal.intent.action.BUGREPORT_REQUESTED"; + // Intent sent to notify external apps that bugreport finished + static final String INTENT_BUGREPORT_FINISHED = + "com.android.internal.intent.action.BUGREPORT_FINISHED"; + // 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"; @@ -188,7 +156,6 @@ public class BugreportProgressService extends Service { static final String EXTRA_BUGREPORT_TYPE = "android.intent.extra.BUGREPORT_TYPE"; static final String EXTRA_SCREENSHOT = "android.intent.extra.SCREENSHOT"; static final String EXTRA_ID = "android.intent.extra.ID"; - 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"; @@ -218,15 +185,9 @@ public class BugreportProgressService extends Service { */ static final int SCREENSHOT_DELAY_SECONDS = 3; - // TODO: will be gone once fully migrated to Binder - /** System properties used to communicate with dumpstate progress. */ - private static final String DUMPSTATE_PREFIX = "dumpstate."; - private static final String NAME_SUFFIX = ".name"; + /** System property where dumpstate stores last triggered bugreport id */ private static final String PROPERTY_LAST_ID = "dumpstate.last_id"; - /** System property (and value) used to stop dumpstate. */ - // TODO: should call ActiveManager API instead - private static final String CTL_STOP = "ctl.stop"; private static final String BUGREPORT_SERVICE = "bugreport"; /** @@ -273,8 +234,6 @@ public class BugreportProgressService extends Service { private File mScreenshotsDir; - private boolean mUsingBugreportApi; - private BugreportManager mBugreportManager; /** @@ -452,9 +411,9 @@ public class BugreportProgressService extends Service { cleanupOldFiles(MIN_KEEP_COUNT, MIN_KEEP_AGE); final Intent intent = new Intent(INTENT_BUGREPORT_FINISHED); intent.putExtra(EXTRA_BUGREPORT, bugreportFilePath); - addScreenshotToIntent(intent, mInfo); + intent.putExtra(EXTRA_SCREENSHOT, getScreenshotForIntent(mInfo)); mContext.sendBroadcast(intent, android.Manifest.permission.DUMP); - onBugreportFinished(mInfo.id); + onBugreportFinished(mInfo); } } } @@ -475,14 +434,17 @@ public class BugreportProgressService extends Service { android.Manifest.permission.DUMP); } - private static void addScreenshotToIntent(Intent intent, BugreportInfo info) { - final File screenshotFile = info.screenshotFiles.isEmpty() - ? null : info.screenshotFiles.get(0); - if (screenshotFile != null && screenshotFile.length() > 0) { + /** + * Checks if screenshot array is non-empty and returns the first screenshot's path. The first + * screenshot is the default screenshot for the bugreport types that take it. + */ + private static String getScreenshotForIntent(BugreportInfo info) { + if (!info.screenshotFiles.isEmpty()) { + final File screenshotFile = info.screenshotFiles.get(0); final String screenshotFilePath = screenshotFile.getAbsolutePath(); - intent.putExtra(EXTRA_SCREENSHOT, screenshotFilePath); + return screenshotFilePath; } - return; + return null; } private static String generateFileHash(String fileName) { @@ -558,40 +520,24 @@ public class BugreportProgressService extends Service { Log.v(TAG, "handleMessage(): " + dumpIntent((Intent) parcel)); final Intent intent; if (parcel instanceof Intent) { - // The real intent was passed to BugreportReceiver, which delegated to the service. + // The real intent was passed to BugreportRequestedReceiver, + // which delegated to the service. intent = (Intent) parcel; } else { intent = (Intent) msg.obj; } final String action = intent.getAction(); - final int pid = intent.getIntExtra(EXTRA_PID, 0); final int id = intent.getIntExtra(EXTRA_ID, 0); final int max = intent.getIntExtra(EXTRA_MAX, -1); final String name = intent.getStringExtra(EXTRA_NAME); if (DEBUG) - Log.v(TAG, "action: " + action + ", name: " + name + ", id: " + id + ", pid: " - + pid + ", max: " + max); + Log.v(TAG, "action: " + action + ", name: " + name + ", id: " + id + + ", max: " + max); switch (action) { case INTENT_BUGREPORT_REQUESTED: startBugreportAPI(intent); break; - case INTENT_BUGREPORT_STARTED: - if (!mUsingBugreportApi && !startProgress(name, id, pid, max)) { - stopSelfWhenDone(); - return; - } - break; - case INTENT_BUGREPORT_FINISHED: - if (!mUsingBugreportApi) { - if (id == 0) { - // Shouldn't happen, unless BUGREPORT_FINISHED is received - // from a legacy, out-of-sync dumpstate process. - Log.w(TAG, "Missing " + EXTRA_ID + " on intent " + intent); - } - onBugreportFinished(id, intent); - } - break; case INTENT_BUGREPORT_INFO_LAUNCH: launchBugreportInfoDialog(id); break; @@ -633,52 +579,12 @@ public class BugreportProgressService extends Service { private BugreportInfo getInfo(int id) { final BugreportInfo bugreportInfo = mBugreportInfos.get(id); if (bugreportInfo == null) { - Log.w(TAG, "Not monitoring process with ID " + id); + Log.w(TAG, "Not monitoring bugreports with ID " + id); return null; } return bugreportInfo; } - /** - * Creates the {@link BugreportInfo} for a process and issue a system notification to - * indicate its progress. - * - * @return whether it succeeded or not. - */ - private boolean startProgress(String name, int id, int pid, int max) { - if (name == null) { - Log.w(TAG, "Missing " + EXTRA_NAME + " on start intent"); - } - if (id == -1) { - Log.e(TAG, "Missing " + EXTRA_ID + " on start intent"); - return false; - } - if (pid == -1) { - Log.e(TAG, "Missing " + EXTRA_PID + " on start intent"); - return false; - } - if (max <= 0) { - Log.e(TAG, "Invalid value for extra " + EXTRA_MAX + ": " + max); - return false; - } - - final BugreportInfo info = new BugreportInfo(mContext, id, pid, name, max); - if (mBugreportInfos.indexOfKey(id) >= 0) { - // BUGREPORT_STARTED intent was already received; ignore it. - Log.w(TAG, "ID " + id + " already watched"); - return true; - } - final DumpstateListener listener = new DumpstateListener(info); - mBugreportInfos.put(info.id, info); - if (listener.connect()) { - updateProgress(info); - return true; - } else { - Log.w(TAG, "not updating progress because it could not connect to dumpstate"); - return false; - } - } - private String getBugreportBaseName(@BugreportParams.BugreportMode int type) { String buildId = SystemProperties.get("ro.build.id", "UNKNOWN_BUILD"); String deviceName = SystemProperties.get("ro.product.name", "UNKNOWN_DEVICE"); @@ -694,7 +600,6 @@ public class BugreportProgressService extends Service { } private void startBugreportAPI(Intent intent) { - mUsingBugreportApi = true; String shareTitle = intent.getStringExtra(EXTRA_TITLE); String shareDescription = intent.getStringExtra(EXTRA_DESCRIPTION); int bugreportType = intent.getIntExtra(EXTRA_BUGREPORT_TYPE, @@ -702,10 +607,8 @@ public class BugreportProgressService extends Service { String baseName = getBugreportBaseName(bugreportType); String name = new SimpleDateFormat("yyyy-MM-dd-HH-mm-ss").format(new Date()); - // pid not used in this workflow, so setting default = 0 - BugreportInfo info = new BugreportInfo(mContext, 0 /* pid */, baseName, name, - 100 /* max progress*/, shareTitle, shareDescription, bugreportType, - mUsingBugreportApi); + BugreportInfo info = new BugreportInfo(mContext, baseName, name, + 100 /* max progress*/, shareTitle, shareDescription, bugreportType); ParcelFileDescriptor bugreportFd = info.createBugreportFd(); if (bugreportFd == null) { @@ -885,11 +788,7 @@ public class BugreportProgressService extends Service { final BugreportInfo info = getInfo(id); if (info != null && !info.finished) { Log.i(TAG, "Cancelling bugreport service (ID=" + id + ") on user's request"); - if (mUsingBugreportApi) { - mBugreportManager.cancelBugreport(); - } else { - setSystemProperty(CTL_STOP, BUGREPORT_SERVICE); - } + mBugreportManager.cancelBugreport(); deleteScreenshots(info); } synchronized (mLock) { @@ -1086,95 +985,16 @@ public class BugreportProgressService extends Service { stopSelf(); } - /** - * Handles the BUGREPORT_FINISHED intent sent by {@code dumpstate}. - */ - private void onBugreportFinished(int id, Intent intent) { - final File bugreportFile = getFileExtra(intent, EXTRA_BUGREPORT); - if (bugreportFile == null) { - // Should never happen, dumpstate always set the file. - Log.wtf(TAG, "Missing " + EXTRA_BUGREPORT + " on intent " + intent); - return; - } - final int max = intent.getIntExtra(EXTRA_MAX, -1); - final File screenshotFile = getFileExtra(intent, EXTRA_SCREENSHOT); - final String shareTitle = intent.getStringExtra(EXTRA_TITLE); - final String shareDescription = intent.getStringExtra(EXTRA_DESCRIPTION); - onBugreportFinished(id, bugreportFile, screenshotFile, shareTitle, shareDescription, max); - } - - /** - * Handles the onfinish() call by BugreportCallbackImpl using the id - */ - private void onBugreportFinished(int id) { - BugreportInfo info = getInfo(id); - final File bugreportFile = info.bugreportFile; - final int max = -1; // this is to log metrics for dumpstate duration. - File screenshotFile = info.screenshotFiles.isEmpty() - ? null : info.screenshotFiles.get(0); - // If the screenshot file did not get populated implies this type of bugreport does not - // need the screenshot file; setting the file to null so that empty file doesnt get shared - if (screenshotFile != null && screenshotFile.length() == 0) { - if (screenshotFile.delete()) { - Log.d(TAG, "screenshot file deleted successfully."); - } - info.screenshotFiles.remove(0); - // TODO(b/136066578): Will soon stop using the below variable as it is a no-op in - // API flow and the screenshotFile value is read from info.screenshotFiles - screenshotFile = null; - } - // TODO: Since we are passing id to the function, it should be able to find the info linked - // to the id and therefore use the value of shareTitle and shareDescription. - onBugreportFinished(id, bugreportFile, screenshotFile, info.shareTitle, - info.shareDescription, max); - } - - /** * Wraps up bugreport generation and triggers a notification to share the bugreport. */ - private void onBugreportFinished(int id, File bugreportFile, @Nullable File screenshotFile, - String shareTitle, String shareDescription, int max) { - mInfoDialog.onBugreportFinished(); - BugreportInfo info = getInfo(id); - if (info == null) { - // Happens when BUGREPORT_FINISHED was received without a BUGREPORT_STARTED first. - Log.v(TAG, "Creating info for untracked ID " + id); - info = new BugreportInfo(mContext, id); - DumpstateListener dumpstateListener = new DumpstateListener(info); - mBugreportInfos.put(id, info); - } - if (!mUsingBugreportApi) { - // Rename in API flow happens before sending the broadcast for remote bugreport (to - // handle renaming before sending broadcasts) - info.renameScreenshots(mScreenshotsDir); - // API workflow already has the bugreport file. This was required in legacy flow, when - // FINISHED broadcast would send the final bugreport files. - // TODO(b/136066578): Change function definition to not include bugreport/screenshot - // file in params - info.bugreportFile = bugreportFile; - if (screenshotFile != null) { - info.addScreenshot(screenshotFile); - } - } - - if (max != -1) { - MetricsLogger.histogram(this, "dumpstate_duration", max); - info.max = max; - } - - if (!TextUtils.isEmpty(shareTitle)) { - info.title = shareTitle; - if (!TextUtils.isEmpty(shareDescription)) { - info.shareDescription= shareDescription; - } - Log.d(TAG, "Bugreport title is " + info.title + "," - + " shareDescription is " + info.shareDescription); - } + private void onBugreportFinished(BugreportInfo info) { + Log.d(TAG, "Bugreport finished with title: " + info.title + + " and shareDescription: " + info.shareDescription); info.finished = true; // Stop running on foreground, otherwise share notification cannot be dismissed. - stopForegroundWhenDone(id); + stopForegroundWhenDone(info.id); triggerLocalNotification(mContext, info); } @@ -1214,7 +1034,11 @@ public class BugreportProgressService extends Service { /** * Build {@link Intent} that can be used to share the given bugreport. */ - private static Intent buildSendIntent(Context context, BugreportInfo info) { + private static Intent buildSendIntent(Context context, BugreportInfo info, + File screenshotsDir) { + // Rename files (if required) before sharing + info.renameBugreportFile(); + info.renameScreenshots(screenshotsDir); // Files are kept on private storage, so turn into Uris that we can // grant temporary permissions for. final Uri bugreportUri; @@ -1300,7 +1124,7 @@ public class BugreportProgressService extends Service { addDetailsToZipFile(info); - final Intent sendIntent = buildSendIntent(mContext, info); + final Intent sendIntent = buildSendIntent(mContext, info, mScreenshotsDir); if (sendIntent == null) { Log.w(TAG, "Stopping progres on ID " + id + " because share intent could not be built"); synchronized (mLock) { @@ -1628,12 +1452,11 @@ public class BugreportProgressService extends Service { } String action = intent.getAction(); if (action == null) { - // Happens when BugreportReceiver calls startService... + // Happens when startService is called... action = "no action"; } final StringBuilder buffer = new StringBuilder(action).append(" extras: "); addExtra(buffer, intent, EXTRA_ID); - addExtra(buffer, intent, EXTRA_PID); addExtra(buffer, intent, EXTRA_MAX); addExtra(buffer, intent, EXTRA_NAME); addExtra(buffer, intent, EXTRA_DESCRIPTION); @@ -1677,15 +1500,6 @@ public class BugreportProgressService extends Service { 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. */ @@ -1770,30 +1584,6 @@ public class BugreportProgressService extends Service { private AlertDialog mDialog; private Button mOkButton; private int mId; - 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. @@ -1813,18 +1603,6 @@ public class BugreportProgressService extends Service { 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(info); - } - }); - mDialog = new AlertDialog.Builder(themedContext) .setView(view) .setTitle(dialogTitle) @@ -1839,15 +1617,6 @@ public class BugreportProgressService extends Service { { MetricsLogger.action(context, MetricsEvent.ACTION_BUGREPORT_DETAILS_CANCELED); - if (!mTempName.equals(mSavedName)) { - // Must restore bugreport's name since it was changed - // before user clicked OK. - if (mUsingBugreportApi) { - info.name = mSavedName; - } else { - setBugreportNameProperty(mPid, mSavedName); - } - } } }) .create(); @@ -1866,9 +1635,7 @@ public class BugreportProgressService extends Service { } // Then set fields. - mSavedName = mTempName = info.name; mId = info.id; - mPid = info.pid; if (!TextUtils.isEmpty(info.name)) { mInfoName.setText(info.name); } @@ -1895,7 +1662,7 @@ public class BugreportProgressService extends Service { @Override public void onClick(View view) { MetricsLogger.action(context, MetricsEvent.ACTION_BUGREPORT_DETAILS_SAVED); - sanitizeName(info); + sanitizeName(info.name); final String name = mInfoName.getText().toString(); final String title = mInfoTitle.getText().toString(); final String description = mInfoDescription.getText().toString(); @@ -1911,9 +1678,9 @@ public class BugreportProgressService extends Service { * Sanitizes the user-provided value for the {@code name} field, automatically replacing * invalid characters if necessary. */ - private void sanitizeName(BugreportInfo info) { + private void sanitizeName(String savedName) { String name = mInfoName.getText().toString(); - if (name.equals(mTempName)) { + if (name.equals(savedName)) { if (DEBUG) Log.v(TAG, "name didn't change, no need to sanitize: " + name); return; } @@ -1933,28 +1700,6 @@ public class BugreportProgressService extends Service { name = safeName.toString(); mInfoName.setText(name); } - mTempName = name; - if (mUsingBugreportApi) { - info.name = name; - } else { - // 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. - */ - void onBugreportFinished() { - if (mInfoName != null) { - mInfoName.setEnabled(false); - mInfoName.setText(mSavedName); - } } void cancel() { @@ -1975,19 +1720,11 @@ public class BugreportProgressService extends Service { */ int id; - /** - * {@code pid} of the {@code dumpstate} process generating the bugreport. - * pid is unused in the API flow - * TODO(b/136066578): Remove pid - */ - final int pid; - /** * Prefix name of the bugreport, this is uneditable. * The baseName consists of the string "bugreport" + deviceName + buildID * This will end with the string "wifi"/"telephony" for wifi/telephony bugreports. * Bugreport zip file name = "-.zip" - * Bugreport png file name = "-.png" */ String baseName; @@ -1997,6 +1734,12 @@ public class BugreportProgressService extends Service { */ String name; + /** + * Initial value of the field name. This is required to rename the files later on, as they + * are created using initial value of name. + */ + String initialName; + /** * User-provided, one-line summary of the bug; when set, will be used as the subject * of the {@link Intent#ACTION_SEND_MULTIPLE} intent. @@ -2062,12 +1805,6 @@ public class BugreportProgressService extends Service { */ boolean finished; - /** - * Whether this bugreport is using API workflow. - * TODO(b/136066578): Remove when legacy bugreport methods are removed - */ - boolean usingApi; - /** * Whether the details entries have been added to the bugreport yet. */ @@ -2091,43 +1828,19 @@ public class BugreportProgressService extends Service { */ int type; - /** - * Constructor for tracked bugreports - typically called upon receiving BUGREPORT_STARTED. - */ - BugreportInfo(Context context, int id, int pid, String name, int max) { - // bugreports triggered by STARTED broadcast do not use callback functions, - // onFinished() callback method is the only function where type is used. - // Set type to -1 as it is unused in this workflow. - // This constructor will soon be removed. - this(context, pid, "" /* dumpstate handles basename */, name, max, null, null, -1, - false); - this.id = id; - } - /** * Constructor for tracked bugreports - typically called upon receiving BUGREPORT_REQUESTED. */ - BugreportInfo(Context context, int pid, String baseName, String name, int max, + BugreportInfo(Context context, String baseName, String name, int max, @Nullable String shareTitle, @Nullable String shareDescription, - @BugreportParams.BugreportMode int type, boolean usingApi) { + @BugreportParams.BugreportMode int type) { this.context = context; - this.pid = pid; - this.name = name; + this.name = this.initialName = name; this.max = this.realMax = max; this.shareTitle = shareTitle == null ? "" : shareTitle; this.shareDescription = shareDescription == null ? "" : shareDescription; this.type = type; this.baseName = baseName; - this.usingApi = usingApi; - } - - /** - * Constructor for untracked bugreports - typically called upon receiving BUGREPORT_FINISHED - * without a previous call to BUGREPORT_STARTED. - */ - BugreportInfo(Context context, int id) { - this(context, id, id, null, 0); - this.finished = true; } ParcelFileDescriptor createBugreportFd() { @@ -2136,17 +1849,24 @@ public class BugreportProgressService extends Service { } ParcelFileDescriptor createScreenshotFd() { - File screenshotFile = new File(BUGREPORT_DIR, getFileName(this, ".png")); + File screenshotFile = new File(BUGREPORT_DIR, getScreenshotName("default")); addScreenshot(screenshotFile); return createReadWriteFile(screenshotFile); } /** - * Gets the name for next screenshot file. + * Gets the name for next user triggered screenshot file. */ String getPathNextScreenshot() { screenshotCounter ++; - return "screenshot-" + pid + "-" + screenshotCounter + ".png"; + return getScreenshotName(Integer.toString(screenshotCounter)); + } + + /** + * Gets the name for screenshot file based on the suffix that is passed. + */ + String getScreenshotName(String suffix) { + return "screenshot-" + initialName + "-" + suffix + ".png"; } /** @@ -2157,7 +1877,8 @@ public class BugreportProgressService extends Service { } /** - * Rename all screenshots files so that they contain the user-generated name instead of pid. + * Rename all screenshots files so that they contain the new {@code name} instead of the + * {@code initialName} if user has changed it. */ void renameScreenshots(File screenshotDir) { if (TextUtils.isEmpty(name)) { @@ -2166,22 +1887,21 @@ public class BugreportProgressService extends Service { final List renamedFiles = new ArrayList<>(screenshotFiles.size()); for (File oldFile : screenshotFiles) { final String oldName = oldFile.getName(); - final String newName; - if (usingApi) { - newName = getFileName(this, ".png"); - } else { - newName = oldName.replaceFirst(Integer.toString(pid), name); - } + final String newName = oldName.replaceFirst(initialName, name); final File newFile; if (!newName.equals(oldName)) { final File renamedFile = new File(screenshotDir, newName); Log.d(TAG, "Renaming screenshot file " + oldFile + " to " + renamedFile); newFile = oldFile.renameTo(renamedFile) ? renamedFile : oldFile; } else { - Log.w(TAG, "Name didn't change: " + oldName); // Shouldn't happen. + Log.w(TAG, "Name didn't change: " + oldName); newFile = oldFile; } - renamedFiles.add(newFile); + if (newFile.length() > 0) { + renamedFiles.add(newFile); + } else if (newFile.delete()) { + Log.d(TAG, "screenshot file: " + newFile + "deleted successfully."); + } } screenshotFiles = renamedFiles; } @@ -2215,11 +1935,10 @@ public class BugreportProgressService extends Service { final StringBuilder builder = new StringBuilder() .append("\tid: ").append(id) - .append(", pid: ").append(pid) .append(", baseName: ").append(baseName) .append(", name: ").append(name) + .append(", initialName: ").append(initialName) .append(", finished: ").append(finished) - .append(", usingApi: ").append(usingApi) .append("\n\ttitle: ").append(title) .append("\n\tdescription: "); if (description == null) { @@ -2250,9 +1969,9 @@ public class BugreportProgressService extends Service { protected BugreportInfo(Parcel in) { context = null; id = in.readInt(); - pid = in.readInt(); baseName = in.readString(); name = in.readString(); + initialName = in.readString(); title = in.readString(); description = in.readString(); max = in.readInt(); @@ -2269,7 +1988,6 @@ public class BugreportProgressService extends Service { } finished = in.readInt() == 1; - usingApi = in.readInt() == 1; screenshotCounter = in.readInt(); shareDescription = in.readString(); shareTitle = in.readString(); @@ -2278,9 +1996,9 @@ public class BugreportProgressService extends Service { @Override public void writeToParcel(Parcel dest, int flags) { dest.writeInt(id); - dest.writeInt(pid); dest.writeString(baseName); dest.writeString(name); + dest.writeString(initialName); dest.writeString(title); dest.writeString(description); dest.writeInt(max); @@ -2297,7 +2015,6 @@ public class BugreportProgressService extends Service { } dest.writeInt(finished ? 1 : 0); - dest.writeInt(usingApi ? 1 : 0); dest.writeInt(screenshotCounter); dest.writeString(shareDescription); dest.writeString(shareTitle); @@ -2333,80 +2050,6 @@ public class BugreportProgressService extends Service { } - private final class DumpstateListener extends IDumpstateListener.Stub - implements DeathRecipient { - - private final BugreportInfo info; - private IDumpstateToken token; - - DumpstateListener(BugreportInfo info) { - this.info = info; - } - - /** - * Connects to the {@code dumpstate} binder to receive updates. - */ - boolean connect() { - if (token != null) { - Log.d(TAG, "connect(): " + info.id + " already connected"); - return true; - } - final IBinder service = ServiceManager.getService("dumpstate"); - if (service == null) { - Log.d(TAG, "dumpstate service not bound yet"); - return true; - } - final IDumpstate dumpstate = IDumpstate.Stub.asInterface(service); - try { - token = dumpstate.setListener("Shell", this, /* perSectionDetails= */ false); - if (token != null) { - token.asBinder().linkToDeath(this, 0); - } - } catch (Exception e) { - Log.e(TAG, "Could not set dumpstate listener: " + e); - } - return token != null; - } - - @Override - public void binderDied() { - if (!info.finished) { - // TODO: linkToDeath() might be called BEFORE Shell received the - // BUGREPORT_FINISHED broadcast, in which case the statements below - // spam logcat (but are harmless). - // The right, long-term solution is to provide an onFinished() callback - // on IDumpstateListener and call it instead of using a broadcast. - Log.w(TAG, "Dumpstate process died:\n" + info); - synchronized (mLock) { - stopProgressLocked(info.id); - } - } - token.asBinder().unlinkToDeath(this, 0); - } - - @Override - public void onProgress(int progress) throws RemoteException { - synchronized (mLock) { - checkProgressUpdatedLocked(info, progress); - } - } - - @Override - public void onError(int errorCode) throws RemoteException { - // TODO(b/111441001): implement - } - - @Override - public void onFinished() throws RemoteException { - // TODO(b/111441001): implement - } - - public void dump(String prefix, PrintWriter pw) { - pw.print(prefix); pw.print("token: "); pw.println(token); - } - - } - @GuardedBy("mLock") private void checkProgressUpdatedLocked(BugreportInfo info, int progress) { if (progress > CAPPED_PROGRESS) { @@ -2418,11 +2061,11 @@ public class BugreportProgressService extends Service { private void updateProgressInfo(BugreportInfo info, int progress, int max) { if (DEBUG) { if (progress != info.progress) { - Log.v(TAG, "Updating progress for PID " + info.pid + "(id: " + info.id + Log.v(TAG, "Updating progress for name " + info.name + "(id: " + info.id + ") from " + info.progress + " to " + progress); } if (max != info.max) { - Log.v(TAG, "Updating max progress for PID " + info.pid + "(id: " + info.id + Log.v(TAG, "Updating max progress for name " + info.name + "(id: " + info.id + ") from " + info.max + " to " + max); } } diff --git a/packages/Shell/src/com/android/shell/BugreportReceiver.java b/packages/Shell/src/com/android/shell/BugreportReceiver.java deleted file mode 100644 index 15ce90fa6418c..0000000000000 --- a/packages/Shell/src/com/android/shell/BugreportReceiver.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * Copyright (C) 2013 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 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.getFileExtra; -import static com.android.shell.BugreportProgressService.dumpIntent; - -import java.io.File; - -import android.content.BroadcastReceiver; -import android.content.Context; -import android.content.Intent; -import android.os.AsyncTask; -import android.os.FileUtils; -import android.text.format.DateUtils; -import android.util.Log; - -/** - * Receiver that handles finished bugreports, usually by attaching them to an - * {@link Intent#ACTION_SEND_MULTIPLE}. - */ -public class BugreportReceiver extends BroadcastReceiver { - private static final String TAG = "BugreportReceiver"; - - /** - * Always keep the newest 8 bugreport files. - */ - private static final int MIN_KEEP_COUNT = 8; - - /** - * Always keep bugreports taken in the last week. - */ - private static final long MIN_KEEP_AGE = DateUtils.WEEK_IN_MILLIS; - - @Override - public void onReceive(Context context, Intent intent) { - Log.d(TAG, "onReceive(): " + dumpIntent(intent)); - // Clean up older bugreports in background - cleanupOldFiles(this, intent, INTENT_BUGREPORT_FINISHED, MIN_KEEP_COUNT, MIN_KEEP_AGE); - - // Delegate intent handling to service. - Intent serviceIntent = new Intent(context, BugreportProgressService.class); - serviceIntent.putExtra(EXTRA_ORIGINAL_INTENT, intent); - context.startService(serviceIntent); - } - - static void cleanupOldFiles(BroadcastReceiver br, Intent intent, String expectedAction, - final int minCount, final long minAge) { - if (!expectedAction.equals(intent.getAction())) { - return; - } - final File bugreportFile = getFileExtra(intent, EXTRA_BUGREPORT); - if (bugreportFile == null || !bugreportFile.exists()) { - Log.e(TAG, "Not deleting old files because file " + bugreportFile + " doesn't exist"); - return; - } - final PendingResult result = br.goAsync(); - new AsyncTask() { - @Override - protected Void doInBackground(Void... params) { - try { - FileUtils.deleteOlderFiles(bugreportFile.getParentFile(), minCount, minAge); - } catch (RuntimeException e) { - Log.e(TAG, "RuntimeException deleting old files", e); - } - result.finish(); - return null; - } - }.execute(); - } -} diff --git a/packages/Shell/src/com/android/shell/RemoteBugreportReceiver.java b/packages/Shell/src/com/android/shell/RemoteBugreportReceiver.java deleted file mode 100644 index 634c3b47c7876..0000000000000 --- a/packages/Shell/src/com/android/shell/RemoteBugreportReceiver.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * 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 static com.android.shell.BugreportProgressService.EXTRA_BUGREPORT; -import static com.android.shell.BugreportProgressService.INTENT_REMOTE_BUGREPORT_FINISHED; -import static com.android.shell.BugreportProgressService.getFileExtra; -import static com.android.shell.BugreportProgressService.getUri; -import static com.android.shell.BugreportReceiver.cleanupOldFiles; - -import java.io.File; - -import android.app.admin.DevicePolicyManager; -import android.content.BroadcastReceiver; -import android.content.Context; -import android.content.Intent; -import android.net.Uri; -import android.os.UserHandle; -import android.text.format.DateUtils; - -/** - * Receiver that handles finished remote bugreports, by re-sending - * the intent with appended bugreport zip file URI. - * - *

    Remote bugreport never contains a screenshot. - */ -public class RemoteBugreportReceiver extends BroadcastReceiver { - - private static final String BUGREPORT_MIMETYPE = "application/vnd.android.bugreport"; - - /** Always keep just the last remote bugreport's files around. */ - private static final int REMOTE_BUGREPORT_FILES_AMOUNT = 3; - - /** Always keep remote bugreport files created in the last day. */ - private static final long MIN_KEEP_AGE = DateUtils.DAY_IN_MILLIS; - - @Override - public void onReceive(Context context, Intent intent) { - cleanupOldFiles(this, intent, INTENT_REMOTE_BUGREPORT_FINISHED, - REMOTE_BUGREPORT_FILES_AMOUNT, MIN_KEEP_AGE); - - final File bugreportFile = getFileExtra(intent, EXTRA_BUGREPORT); - final Uri bugreportUri = getUri(context, bugreportFile); - final String bugreportHash = intent.getStringExtra( - DevicePolicyManager.EXTRA_REMOTE_BUGREPORT_HASH); - - final Intent newIntent = new Intent(DevicePolicyManager.ACTION_REMOTE_BUGREPORT_DISPATCH); - newIntent.setDataAndType(bugreportUri, BUGREPORT_MIMETYPE); - newIntent.putExtra(DevicePolicyManager.EXTRA_REMOTE_BUGREPORT_HASH, bugreportHash); - context.sendBroadcastAsUser(newIntent, UserHandle.SYSTEM, - android.Manifest.permission.DUMP); - } -} diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java index 3a71632cf1caf..bb298e937fbbd 100644 --- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java +++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java @@ -29,10 +29,8 @@ import static com.android.shell.BugreportProgressService.EXTRA_BUGREPORT; import static com.android.shell.BugreportProgressService.EXTRA_ID; import static com.android.shell.BugreportProgressService.EXTRA_MAX; import static com.android.shell.BugreportProgressService.EXTRA_NAME; -import static com.android.shell.BugreportProgressService.EXTRA_PID; import static com.android.shell.BugreportProgressService.EXTRA_SCREENSHOT; import static com.android.shell.BugreportProgressService.INTENT_BUGREPORT_FINISHED; -import static com.android.shell.BugreportProgressService.INTENT_BUGREPORT_STARTED; import static com.android.shell.BugreportProgressService.SCREENSHOT_DELAY_SECONDS; import static org.junit.Assert.assertEquals; @@ -145,6 +143,10 @@ public class BugreportReceiverTest { private static final String TITLE2 = "Master of the Universe"; private static final String DESCRIPTION = "One's description..."; private static final String DESCRIPTION2 = "...is another's treasure."; + // TODO(b/143130523): Fix (update) tests and add to presubmit + private static final String EXTRA_PID = "android.intent.extra.PID"; + private static final String INTENT_BUGREPORT_STARTED = + "com.android.internal.intent.action.BUGREPORT_STARTED"; private static final String NO_DESCRIPTION = null; private static final String NO_NAME = null;