From 28d8dd7890ef6622e770c79653f883368ef7178b Mon Sep 17 00:00:00 2001 From: Nandana Dutt Date: Fri, 25 Jan 2019 14:31:05 +0000 Subject: [PATCH] Follow API guidelines in BugreportManager * Add an executor for callback * Rename *listener to *callback * Accept ParcelFileDescriptor. Not changing the binder interface to accept ParcelFileDescriptor because there seem to a bug in generated java code for "out" ParcelFileDescriptors causing compilation errors. BUG: 111441001 Test: Builds Change-Id: I9caf91b504eacc3ab6ff23620f1d6ded51caee1a --- core/java/android/os/BugreportManager.java | 123 +++++++++++++-------- 1 file changed, 74 insertions(+), 49 deletions(-) diff --git a/core/java/android/os/BugreportManager.java b/core/java/android/os/BugreportManager.java index d463b44228471..6f5f8072f471b 100644 --- a/core/java/android/os/BugreportManager.java +++ b/core/java/android/os/BugreportManager.java @@ -16,6 +16,7 @@ package android.os; +import android.annotation.CallbackExecutor; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; @@ -27,6 +28,7 @@ import android.os.IBinder.DeathRecipient; import java.io.FileDescriptor; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.concurrent.Executor; /** * Class that provides a privileged API to capture and consume bugreports. @@ -34,7 +36,7 @@ import java.lang.annotation.RetentionPolicy; * @hide */ // TODO: Expose API when the implementation is more complete. -// @SystemApi +//@SystemApi @SystemService(Context.BUGREPORT_SERVICE) public class BugreportManager { private final Context mContext; @@ -47,47 +49,46 @@ public class BugreportManager { } /** - * An interface describing the listener for bugreport progress and status. + * An interface describing the callback for bugreport progress and status. */ - public interface BugreportListener { + public abstract static class BugreportCallback { + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef(prefix = { "BUGREPORT_ERROR_" }, value = { + BUGREPORT_ERROR_INVALID_INPUT, + BUGREPORT_ERROR_RUNTIME, + BUGREPORT_ERROR_USER_DENIED_CONSENT, + BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT + }) + + /** Possible error codes taking a bugreport can encounter */ + public @interface BugreportErrorCode {} + + /** The input options were invalid */ + public static final int BUGREPORT_ERROR_INVALID_INPUT = + IDumpstateListener.BUGREPORT_ERROR_INVALID_INPUT; + + /** A runtime error occured */ + public static final int BUGREPORT_ERROR_RUNTIME = + IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR; + + /** User denied consent to share the bugreport */ + public static final int BUGREPORT_ERROR_USER_DENIED_CONSENT = + IDumpstateListener.BUGREPORT_ERROR_USER_DENIED_CONSENT; + + /** The request to get user consent timed out. */ + public static final int BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT = + IDumpstateListener.BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT; + /** * Called when there is a progress update. * @param progress the progress in [0.0, 100.0] */ - void onProgress(float progress); - - @Retention(RetentionPolicy.SOURCE) - @IntDef(prefix = { "BUGREPORT_ERROR_" }, value = { - BUGREPORT_ERROR_INVALID_INPUT, - BUGREPORT_ERROR_RUNTIME - }) - - /** Possible error codes taking a bugreport can encounter */ - @interface BugreportErrorCode {} - - /** The input options were invalid */ - int BUGREPORT_ERROR_INVALID_INPUT = IDumpstateListener.BUGREPORT_ERROR_INVALID_INPUT; - - /** A runtime error occured */ - int BUGREPORT_ERROR_RUNTIME = IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR; - - /** User denied consent to share the bugreport */ - int BUGREPORT_ERROR_USER_DENIED_CONSENT = - IDumpstateListener.BUGREPORT_ERROR_USER_DENIED_CONSENT; - - /** The request to get user consent timed out. */ - int BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT = - IDumpstateListener.BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT; + public void onProgress(float progress) {} /** * Called when taking bugreport resulted in an error. * - * @param errorCode the error that occurred. Possible values are - * {@code BUGREPORT_ERROR_INVALID_INPUT}, - * {@code BUGREPORT_ERROR_RUNTIME}, - * {@code BUGREPORT_ERROR_USER_DENIED_CONSENT}, - * {@code BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT}. - * *

If {@code BUGREPORT_ERROR_USER_DENIED_CONSENT} is passed, then the user did not * consent to sharing the bugreport with the calling app. * @@ -95,19 +96,19 @@ public class BugreportManager { * out, but the bugreport could be available in the internal directory of dumpstate for * manual retrieval. */ - void onError(@BugreportErrorCode int errorCode); + public void onError(@BugreportErrorCode int errorCode) {} /** * Called when taking bugreport finishes successfully. */ - void onFinished(); + public void onFinished() {} } /** * Starts a bugreport. * *

This starts a bugreport in the background. However the call itself can take several - * seconds to return in the worst case. {@code listener} will receive progress and status + * seconds to return in the worst case. {@code callback} will receive progress and status * updates. * *

The bugreport artifacts will be copied over to the given file descriptors only if the @@ -118,19 +119,23 @@ public class BugreportManager { * @param screenshotFd file to write the screenshot, if necessary. This should be opened * in write-only, append mode. * @param params options that specify what kind of a bugreport should be taken - * @param listener callback for progress and status updates + * @param callback callback for progress and status updates */ @RequiresPermission(android.Manifest.permission.DUMP) - public void startBugreport(@NonNull FileDescriptor bugreportFd, - @Nullable FileDescriptor screenshotFd, - @NonNull BugreportParams params, @NonNull BugreportListener listener) { + public void startBugreport(@NonNull ParcelFileDescriptor bugreportFd, + @Nullable ParcelFileDescriptor screenshotFd, + @NonNull BugreportParams params, + @NonNull @CallbackExecutor Executor executor, + @NonNull BugreportCallback callback) { // TODO(b/111441001): Enforce android.Manifest.permission.DUMP if necessary. - DumpstateListener dsListener = new DumpstateListener(listener); - + DumpstateListener dsListener = new DumpstateListener(executor, callback); try { // Note: mBinder can get callingUid from the binder transaction. mBinder.startBugreport(-1 /* callingUid */, - mContext.getOpPackageName(), bugreportFd, screenshotFd, + mContext.getOpPackageName(), + (bugreportFd != null ? bugreportFd.getFileDescriptor() : new FileDescriptor()), + (screenshotFd != null + ? screenshotFd.getFileDescriptor() : new FileDescriptor()), params.getMode(), dsListener); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); @@ -151,10 +156,12 @@ public class BugreportManager { private final class DumpstateListener extends IDumpstateListener.Stub implements DeathRecipient { - private final BugreportListener mListener; + private final Executor mExecutor; + private final BugreportCallback mCallback; - DumpstateListener(@Nullable BugreportListener listener) { - mListener = listener; + DumpstateListener(Executor executor, @Nullable BugreportCallback callback) { + mExecutor = executor; + mCallback = callback; } @Override @@ -164,19 +171,37 @@ public class BugreportManager { @Override public void onProgress(int progress) throws RemoteException { - mListener.onProgress(progress); + final long identity = Binder.clearCallingIdentity(); + try { + mExecutor.execute(() -> { + mCallback.onProgress(progress); + }); + } finally { + Binder.restoreCallingIdentity(identity); + } } @Override public void onError(int errorCode) throws RemoteException { - mListener.onError(errorCode); + final long identity = Binder.clearCallingIdentity(); + try { + mExecutor.execute(() -> { + mCallback.onError(errorCode); + }); + } finally { + Binder.restoreCallingIdentity(identity); + } } @Override public void onFinished() throws RemoteException { + final long identity = Binder.clearCallingIdentity(); try { - mListener.onFinished(); + mExecutor.execute(() -> { + mCallback.onFinished(); + }); } finally { + Binder.restoreCallingIdentity(identity); // The bugreport has finished. Let's shutdown the service to minimize its footprint. cancelBugreport(); }