Merge "Add progress API to ICarBugreport and CarBugReportManagerService." into qt-dev
diff --git a/car-lib/src/android/car/CarBugreportManager.java b/car-lib/src/android/car/CarBugreportManager.java
index 7fca8ba..b0f7a03 100644
--- a/car-lib/src/android/car/CarBugreportManager.java
+++ b/car-lib/src/android/car/CarBugreportManager.java
@@ -16,6 +16,7 @@
package android.car;
+import android.annotation.FloatRange;
import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.RequiresPermission;
@@ -74,6 +75,16 @@
public static final int CAR_BUGREPORT_SERVICE_NOT_AVAILABLE = 4;
/**
+ * Called when bugreport progress changes.
+ *
+ * <p>It's never called after {@link #onError} or {@link #onFinished}.
+ *
+ * @param progress - a number in [0.0, 100.0].
+ */
+ public void onProgress(@FloatRange(from = 0f, to = 100f) float progress) {
+ }
+
+ /**
* Called on an error condition with one of the error codes listed above.
*
* @param errorCode the error code that defines failure reason.
@@ -110,6 +121,15 @@
}
@Override
+ public void onProgress(@FloatRange(from = 0f, to = 100f) float progress) {
+ CarBugreportManagerCallback callback = mWeakCallback.get();
+ Handler handler = mWeakHandler.get();
+ if (handler != null && callback != null) {
+ handler.post(() -> callback.onProgress(progress));
+ }
+ }
+
+ @Override
public void onError(@CarBugreportManagerCallback.CarBugreportErrorCode int errorCode) {
CarBugreportManagerCallback callback = mWeakCallback.get();
Handler handler = mWeakHandler.get();
@@ -123,7 +143,7 @@
CarBugreportManagerCallback callback = mWeakCallback.get();
Handler handler = mWeakHandler.get();
if (handler != null && callback != null) {
- handler.post(() -> callback.onFinished());
+ handler.post(callback::onFinished);
}
}
}
@@ -139,37 +159,30 @@
}
/**
- * Request a bug report. An zipped bugreport is generated in the background.
- * The file descriptors are closed when bugreport is written or if an exception happens.
- * The progress protocol is described
- * <a href="https://android.googlesource.com/platform/frameworks/native/+/master/cmds/bugreportz/readme.md">
- * here</a>
+ * Request a bug report. A zipped bugreport is generated in the background.
+ *
+ * <p>The file descriptor is closed when bugreport is written or if an exception happens.
*
* @param output the zipped bugreport file
- * @param progress the progress information that includes failure/success status.
* @param callback the callback for reporting dump status
*/
@RequiresPermission(android.Manifest.permission.DUMP)
- public void requestZippedBugreport(@NonNull ParcelFileDescriptor output,
- @NonNull ParcelFileDescriptor progress,
- @NonNull CarBugreportManagerCallback callback) {
+ public void requestZippedBugreport(
+ @NonNull ParcelFileDescriptor output, @NonNull CarBugreportManagerCallback callback) {
Preconditions.checkNotNull(output);
- Preconditions.checkNotNull(progress);
Preconditions.checkNotNull(callback);
try {
CarBugreportManagerCallbackWrapper wrapper =
new CarBugreportManagerCallbackWrapper(callback, mHandler);
- mService.requestZippedBugreport(output, progress, wrapper);
+ mService.requestZippedBugreport(output, wrapper);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
} finally {
IoUtils.closeQuietly(output);
- IoUtils.closeQuietly(progress);
}
}
@Override
public void onCarDisconnected() {
}
-
}
diff --git a/car-lib/src/android/car/ICarBugreportCallback.aidl b/car-lib/src/android/car/ICarBugreportCallback.aidl
index e254d4c..0384d43 100644
--- a/car-lib/src/android/car/ICarBugreportCallback.aidl
+++ b/car-lib/src/android/car/ICarBugreportCallback.aidl
@@ -29,8 +29,15 @@
void onError(int errorCode);
/**
+ * Called when the bugreport progress changes. Progress value is a number between 0.0 and 100.0.
+ *
+ * <p>Never called after {@link #onError()} or {@link onFinished()}.
+ */
+ void onProgress(float progress);
+
+ /**
* Called when taking bugreport finishes successfully.
*/
void onFinished();
-}
\ No newline at end of file
+}
diff --git a/car-lib/src/android/car/ICarBugreportService.aidl b/car-lib/src/android/car/ICarBugreportService.aidl
index 2d713da..1ec4e91 100644
--- a/car-lib/src/android/car/ICarBugreportService.aidl
+++ b/car-lib/src/android/car/ICarBugreportService.aidl
@@ -28,14 +28,7 @@
/**
* Starts bugreport service to capture a zipped bugreport. The caller needs to provide
* two file descriptors. The "output" file descriptor will be used to provide the actual
- * zip file and the "progress" descriptor will be used to provide the progress information.
- * Both of these descriptors are written by the service and will be read by the client.
- *
- * The progress protocol is described
- * <a href="https://android.googlesource.com/platform/frameworks/native/+/master/cmds/bugreportz/readme.md">
- * here</a>
+ * zip file. The file descriptor is written by the service and will be read by the client.
*/
- void requestZippedBugreport(in ParcelFileDescriptor output, in ParcelFileDescriptor progress,
- ICarBugreportCallback callback) = 1;
+ void requestZippedBugreport(in ParcelFileDescriptor output, ICarBugreportCallback callback) = 1;
}
-
diff --git a/service/src/com/android/car/CarBugreportManagerService.java b/service/src/com/android/car/CarBugreportManagerService.java
index 338ca43..572f17d 100644
--- a/service/src/com/android/car/CarBugreportManagerService.java
+++ b/service/src/com/android/car/CarBugreportManagerService.java
@@ -16,6 +16,11 @@
package com.android.car;
+import static android.car.CarBugreportManager.CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED;
+import static android.car.CarBugreportManager.CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_FAILED;
+
+import android.annotation.NonNull;
+import android.annotation.Nullable;
import android.annotation.RequiresPermission;
import android.car.CarBugreportManager.CarBugreportManagerCallback;
import android.car.ICarBugreportCallback;
@@ -37,14 +42,15 @@
import com.android.internal.annotations.GuardedBy;
+import java.io.BufferedReader;
import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.PrintWriter;
-
/**
* Bugreport service for cars. Should *only* be used on userdebug or eng builds.
*/
@@ -52,17 +58,19 @@
CarServiceBase {
private static final String TAG = "CarBugreportMgrService";
- private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
- private final Context mContext;
- private final Object mLock = new Object();
+ /**
+ * {@code dumpstate} progress prefixes.
+ *
+ * <p>The protocol is described in {@code frameworks/native/cmds/bugreportz/readme.md}.
+ */
+ private static final String BEGIN_PREFIX = "BEGIN:";
+ private static final String PROGRESS_PREFIX = "PROGRESS:";
+ private static final String OK_PREFIX = "OK:";
+ private static final String FAIL_PREFIX = "FAIL:";
- private HandlerThread mHandlerThread = null;
- private Handler mHandler;
- private boolean mIsServiceRunning;
+ private static final String BUGREPORTD_SERVICE = "car-bugreportd";
- // The socket at /dev/socket/dumpstate to communicate with dumpstate.
- private static final String DUMPSTATE_SOCKET = "dumpstate";
// The socket definitions must match the actual socket names defined in car_bugreportd service
// definition.
private static final String BUGREPORT_PROGRESS_SOCKET = "car_br_progress_socket";
@@ -70,6 +78,13 @@
private static final int SOCKET_CONNECTION_MAX_RETRY = 10;
+ private final Context mContext;
+ private final Object mLock = new Object();
+
+ private HandlerThread mHandlerThread;
+ private Handler mHandler;
+ private boolean mIsServiceRunning;
+
/**
* Create a CarBugreportManagerService instance.
*
@@ -93,8 +108,7 @@
@Override
@RequiresPermission(android.Manifest.permission.DUMP)
- public void requestZippedBugreport(ParcelFileDescriptor data, ParcelFileDescriptor progress,
- ICarBugreportCallback callback) {
+ public void requestZippedBugreport(ParcelFileDescriptor data, ICarBugreportCallback callback) {
// Check the caller has proper permission
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP,
@@ -124,84 +138,134 @@
}
synchronized (mLock) {
- requestZippedBugReportLocked(data, progress, callback);
+ requestZippedBugReportLocked(data, callback);
}
}
@GuardedBy("mLock")
- private void requestZippedBugReportLocked(ParcelFileDescriptor data,
- ParcelFileDescriptor progress, ICarBugreportCallback callback) {
+ private void requestZippedBugReportLocked(
+ ParcelFileDescriptor data, ICarBugreportCallback callback) {
if (mIsServiceRunning) {
Slog.w(TAG, "Bugreport Service already running");
reportError(callback, CarBugreportManagerCallback.CAR_BUGREPORT_IN_PROGRESS);
return;
}
mIsServiceRunning = true;
- mHandler.post(() -> startBugreportd(data, progress, callback));
+ mHandler.post(() -> startBugreportd(data, callback));
}
- private void startBugreportd(ParcelFileDescriptor data, ParcelFileDescriptor progress,
- ICarBugreportCallback callback) {
- readBugreport(data, progress, callback);
+ private void startBugreportd(ParcelFileDescriptor data, ICarBugreportCallback callback) {
+ Slog.i(TAG, "Starting " + BUGREPORTD_SERVICE);
+ try {
+ SystemProperties.set("ctl.start", BUGREPORTD_SERVICE);
+ } catch (RuntimeException e) {
+ Slog.e(TAG, "Failed to start " + BUGREPORTD_SERVICE, e);
+ reportError(callback, CAR_BUGREPORT_DUMPSTATE_FAILED);
+ return;
+ }
+ processBugreportSockets(data, callback);
synchronized (mLock) {
mIsServiceRunning = false;
}
}
- private void readBugreport(ParcelFileDescriptor output, ParcelFileDescriptor progress,
- ICarBugreportCallback callback) {
- Slog.i(TAG, "Starting car-bugreportd");
+ private void handleProgress(String line, ICarBugreportCallback callback) {
+ String progressOverTotal = line.substring(PROGRESS_PREFIX.length());
+ String[] parts = progressOverTotal.split("/");
+ if (parts.length != 2) {
+ Slog.w(TAG, "Invalid progress line from bugreportz: " + line);
+ return;
+ }
+ float progress;
+ float total;
try {
- SystemProperties.set("ctl.start", "car-bugreportd");
- } catch (RuntimeException e) {
- Slog.e(TAG, "Failed to start car-bugreportd", e);
- reportError(callback, CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_FAILED);
+ progress = Float.parseFloat(parts[0]);
+ total = Float.parseFloat(parts[1]);
+ } catch (NumberFormatException e) {
+ Slog.w(TAG, "Invalid progress value: " + line, e);
return;
}
- // The native service first generates the progress data. Once it writes the progress
- // data fully, it closes the socket and writes the zip file. So we read both files
- // sequentially here.
- if (!readSocket(BUGREPORT_PROGRESS_SOCKET, progress, callback)) {
- Slog.e(TAG, "failed reading bugreport progress socket");
- reportError(callback, CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_FAILED);
+ if (total == 0) {
+ Slog.w(TAG, "Invalid progress total value: " + line);
return;
}
- if (!readSocket(BUGREPORT_OUTPUT_SOCKET, output, callback)) {
- Slog.i(TAG, "failed reading bugreport output socket");
- reportError(callback, CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_FAILED);
- return;
- }
- Slog.i(TAG, "finished reading bugreport");
try {
- callback.onFinished();
+ callback.onProgress(100f * progress / total);
} catch (RemoteException e) {
- Slog.e(TAG, "onFinished() failed: " + e.getMessage());
+ Slog.e(TAG, "Failed to call onProgress callback", e);
}
}
- private boolean readSocket(String name, ParcelFileDescriptor pfd,
- ICarBugreportCallback callback) {
- LocalSocket localSocket;
-
+ private void handleFinished(ParcelFileDescriptor output, ICarBugreportCallback callback) {
+ Slog.i(TAG, "Finished reading bugreport");
+ if (!copySocketToPfd(output, callback)) {
+ return;
+ }
try {
- localSocket = connectSocket(name);
- } catch (IOException e) {
- Slog.e(TAG, "Failed connecting to socket " + name, e);
- reportError(callback,
- CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED);
- // Early out if connection to socket fails.
+ callback.onFinished();
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Failed to call onFinished callback", e);
+ }
+ }
+
+ /**
+ * Reads from dumpstate progress and output sockets and invokes appropriate callbacks.
+ *
+ * <p>dumpstate prints {@code BEGIN:} right away, then prints {@code PROGRESS:} as it
+ * progresses. When it finishes or fails it prints {@code OK:pathToTheZipFile} or
+ * {@code FAIL:message} accordingly.
+ */
+ private void processBugreportSockets(
+ ParcelFileDescriptor output, ICarBugreportCallback callback) {
+ LocalSocket localSocket = connectSocket(BUGREPORT_PROGRESS_SOCKET);
+ if (localSocket == null) {
+ reportError(callback, CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED);
+ return;
+ }
+
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(localSocket.getInputStream()))) {
+ String line;
+ while ((line = reader.readLine()) != null) {
+ if (line.startsWith(PROGRESS_PREFIX)) {
+ handleProgress(line, callback);
+ } else if (line.startsWith(FAIL_PREFIX)) {
+ String errorMessage = line.substring(FAIL_PREFIX.length());
+ Slog.e(TAG, "Failed to dumpstate: " + errorMessage);
+ reportError(callback, CAR_BUGREPORT_DUMPSTATE_FAILED);
+ return;
+ } else if (line.startsWith(OK_PREFIX)) {
+ handleFinished(output, callback);
+ return;
+ } else if (!line.startsWith(BEGIN_PREFIX)) {
+ Slog.w(TAG, "Received unknown progress line from dumpstate: " + line);
+ }
+ }
+ Slog.e(TAG, "dumpstate progress unexpectedly ended");
+ reportError(callback, CAR_BUGREPORT_DUMPSTATE_FAILED);
+ } catch (IOException | RuntimeException e) {
+ Slog.i(TAG, "Failed to read from progress socket", e);
+ reportError(callback, CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED);
+ }
+ }
+
+ private boolean copySocketToPfd(
+ ParcelFileDescriptor pfd, ICarBugreportCallback callback) {
+ LocalSocket localSocket = connectSocket(BUGREPORT_OUTPUT_SOCKET);
+ if (localSocket == null) {
+ reportError(callback, CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED);
return false;
}
try (
DataInputStream in = new DataInputStream(localSocket.getInputStream());
DataOutputStream out =
- new DataOutputStream(new ParcelFileDescriptor.AutoCloseOutputStream(pfd));
+ new DataOutputStream(new ParcelFileDescriptor.AutoCloseOutputStream(pfd))
) {
rawCopyStream(out, in);
} catch (IOException | RuntimeException e) {
- Slog.e(TAG, "Failed to grab dump state " + name, e);
- reportError(callback, CarBugreportManagerCallback.CAR_BUGREPORT_DUMPSTATE_FAILED);
+ Slog.e(TAG, "Failed to grab dump state from " + BUGREPORT_OUTPUT_SOCKET, e);
+ reportError(callback, CAR_BUGREPORT_DUMPSTATE_FAILED);
return false;
}
return true;
@@ -220,15 +284,15 @@
// TODO(sgurun) implement
}
- private LocalSocket connectSocket(String socketName) throws IOException {
+ @Nullable
+ private LocalSocket connectSocket(@NonNull String socketName) {
LocalSocket socket = new LocalSocket();
// The dumpstate socket will be created by init upon receiving the
// service request. It may not be ready by this point. So we will
// keep retrying until success or reaching timeout.
int retryCount = 0;
while (true) {
- // First connection always fails so we just 1 second before trying to connect for the
- // first time too.
+ // First connection always fails, so we wait 1 second before trying to connect.
SystemClock.sleep(/* ms= */ 1000);
try {
socket.connect(new LocalSocketAddress(socketName,
@@ -236,9 +300,11 @@
return socket;
} catch (IOException e) {
if (++retryCount >= SOCKET_CONNECTION_MAX_RETRY) {
- throw e;
+ Slog.i(TAG, "Failed to connect to dumpstate socket " + socketName
+ + " after " + retryCount + " retries", e);
+ return null;
}
- Log.i(TAG, "Failed to connect to" + socketName + ". will try again"
+ Log.i(TAG, "Failed to connect to" + socketName + ". Will try again "
+ e.getMessage());
}
}
diff --git a/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java b/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java
index 1b1d030..b0c754a 100644
--- a/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java
+++ b/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java
@@ -89,7 +89,6 @@
// http://cs/android/frameworks/base/core/java/android/app/ActivityView.java
private static final String ACTIVITY_VIEW_VIRTUAL_DISPLAY = "ActivityViewVirtualDisplay";
private static final String OUTPUT_ZIP_FILE = "output_file.zip";
- private static final String PROGRESS_FILE = "progress.txt";
private static final String MESSAGE_FAILURE_DUMPSTATE = "Failed to grab dumpstate";
private static final String MESSAGE_FAILURE_ZIP = "Failed to zip files";
@@ -250,31 +249,19 @@
private void dumpStateToFile() {
Log.i(TAG, "Dumpstate to file");
File outputFile = FileUtils.getFile(this, mMetaBugReport.getTimestamp(), OUTPUT_ZIP_FILE);
- File progressFile = FileUtils.getFile(this, mMetaBugReport.getTimestamp(), PROGRESS_FILE);
- ParcelFileDescriptor outFd = null;
- ParcelFileDescriptor progressFd = null;
- try {
- outFd = ParcelFileDescriptor.open(outputFile,
- ParcelFileDescriptor.MODE_CREATE | ParcelFileDescriptor.MODE_READ_WRITE);
-
- progressFd = ParcelFileDescriptor.open(progressFile,
- ParcelFileDescriptor.MODE_CREATE | ParcelFileDescriptor.MODE_READ_WRITE);
-
- requestBugReport(outFd, progressFd);
+ try (ParcelFileDescriptor outFd = ParcelFileDescriptor.open(outputFile,
+ ParcelFileDescriptor.MODE_CREATE | ParcelFileDescriptor.MODE_READ_WRITE)) {
+ requestBugReport(outFd);
} catch (IOException | RuntimeException e) {
Log.e(TAG, "Failed to grab dump state", e);
BugStorageUtils.setBugReportStatus(this, mMetaBugReport, Status.STATUS_WRITE_FAILED,
MESSAGE_FAILURE_DUMPSTATE);
sendStatusInformation(R.string.toast_status_dump_state_failed);
- } finally {
- IoUtils.closeQuietly(outFd);
- IoUtils.closeQuietly(progressFd);
}
}
- // In Android Q and above, use the CarBugreportManager API
- private void requestBugReport(ParcelFileDescriptor outFd, ParcelFileDescriptor progressFd) {
+ private void requestBugReport(ParcelFileDescriptor outFd) {
if (DEBUG) {
Log.d(TAG, "Requesting a bug report from CarBugReportManager.");
}
@@ -288,12 +275,17 @@
}
@Override
+ public void onProgress(float progress) {
+ Log.d(TAG, "bugreport progress received " + progress);
+ }
+
+ @Override
public void onFinished() {
Log.i(TAG, "Bugreport finished");
scheduleZipTask();
}
};
- mBugreportManager.requestZippedBugreport(outFd, progressFd, mCallback);
+ mBugreportManager.requestZippedBugreport(outFd, mCallback);
}
private void scheduleZipTask() {
@@ -371,10 +363,6 @@
continue;
}
String filename = file.getName();
- if (filename.equals(PROGRESS_FILE)) {
- // Progress file is already part of zipped bugreport - skip it.
- continue;
- }
// only for the OUTPUT_FILE, we add invidiual entries to zip file
if (filename.equals(OUTPUT_ZIP_FILE)) {
extractZippedFileToOutputStream(file, zipStream);