Merge "Add screencapture feature to bugreporting" into qt-dev
am: 0021b71bbb

Change-Id: I58214a5b29fa0e71472669baf51aa100495ecd1d
diff --git a/car-bugreportd/Android.bp b/car-bugreportd/Android.bp
index e026d0b..36420f0 100644
--- a/car-bugreportd/Android.bp
+++ b/car-bugreportd/Android.bp
@@ -29,7 +29,11 @@
     ],
     shared_libs: [
         "libbase",
-        "liblog",
         "libcutils",
+        "libgui",
+        "libhwui",
+        "liblog",
+        "libui",
+        "libziparchive",
     ],
 }
diff --git a/car-bugreportd/car-bugreportd.rc b/car-bugreportd/car-bugreportd.rc
index 4c405e1..0935a60 100644
--- a/car-bugreportd/car-bugreportd.rc
+++ b/car-bugreportd/car-bugreportd.rc
@@ -1,6 +1,7 @@
 service car-bugreportd /system/bin/car-bugreportd
     socket car_br_progress_socket stream 0660 shell log
     socket car_br_output_socket stream 0660 shell log
+    socket car_br_extra_output_socket stream 0660 shell log
     class core
     user shell
     group log
diff --git a/car-bugreportd/main.cpp b/car-bugreportd/main.cpp
index 055d35a..f2ef87c 100644
--- a/car-bugreportd/main.cpp
+++ b/car-bugreportd/main.cpp
@@ -27,25 +27,34 @@
 #include <cutils/sockets.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <ftw.h>
+#include <gui/SurfaceComposerClient.h>
 #include <log/log_main.h>
 #include <private/android_filesystem_config.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/prctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <time.h>
 #include <unistd.h>
+#include <ziparchive/zip_writer.h>
 
 #include <chrono>
 #include <string>
 #include <vector>
 
 namespace {
+// Directory used for keeping temporary files
+constexpr const char* kTempDirectory = "/data/user_de/0/com.android.shell/temp_bugreport_files";
 // Socket to write the progress information.
 constexpr const char* kCarBrProgressSocket = "car_br_progress_socket";
 // Socket to write the zipped bugreport file.
 constexpr const char* kCarBrOutputSocket = "car_br_output_socket";
+// Socket to write the extra bugreport zip file. This zip file contains data that does not exist
+// in bugreport file generated by dumpstate.
+constexpr const char* kCarBrExtraOutputSocket = "car_br_extra_output_socket";
 // The prefix used by bugreportz protocol to indicate bugreport finished successfully.
 constexpr const char* kOkPrefix = "OK:";
 // Number of connect attempts to dumpstate socket
@@ -55,6 +64,13 @@
 // Wait time for dumpstate. No timeout in dumpstate is longer than 60 seconds. Choose
 // a value that is twice longer.
 constexpr const int kDumpstateTimeoutInSec = 120;
+// The prefix for screenshot filename in the generated zip file.
+constexpr const char* kScreenshotPrefix = "/screenshot";
+
+using android::OK;
+using android::PhysicalDisplayId;
+using android::status_t;
+using android::SurfaceComposerClient;
 
 // Returns a valid socket descriptor or -1 on failure.
 int openSocket(const char* service) {
@@ -96,6 +112,66 @@
     return;
 }
 
+// Sends the contents of the zip fileto |outfd|.
+// Returns true if success
+void zipFilesToFd(const std::vector<std::string>& extra_files, int outfd) {
+    // pass fclose as Deleter to close the file when unique_ptr is destroyed.
+    std::unique_ptr<FILE, decltype(fclose)*> outfile = {fdopen(outfd, "wb"), fclose};
+    if (outfile == nullptr) {
+        ALOGE("Failed to open output descriptor");
+        return;
+    }
+    auto writer = std::make_unique<ZipWriter>(outfile.get());
+
+    int error = 0;
+    for (const auto& filepath : extra_files) {
+        const auto name = android::base::Basename(filepath);
+
+        error = writer->StartEntry(name.c_str(), 0);
+        if (error) {
+            ALOGE("Failed to start entry %s", writer->ErrorCodeString(error));
+            return;
+        }
+        android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(filepath.c_str(), O_RDONLY)));
+        if (fd == -1) {
+            return;
+        }
+        while (1) {
+            char buffer[65536];
+
+            ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd, buffer, sizeof(buffer)));
+            if (bytes_read == 0) {
+                break;
+            }
+            if (bytes_read == -1) {
+                if (errno == EAGAIN) {
+                    ALOGE("timed out while reading %s", name.c_str());
+                } else {
+                    ALOGE("read terminated abnormally (%s)", strerror(errno));
+                }
+                // fail immediately
+                return;
+            }
+            error = writer->WriteBytes(buffer, bytes_read);
+            if (error) {
+                ALOGE("WriteBytes() failed %s", ZipWriter::ErrorCodeString(error));
+                // fail immediately
+                return;
+            }
+        }
+
+        error = writer->FinishEntry();
+        if (error) {
+            ALOGE("failed to finish entry %s", writer->ErrorCodeString(error));
+            continue;
+        }
+    }
+    error = writer->Finish();
+    if (error) {
+        ALOGE("failed to finish zip writer %s", writer->ErrorCodeString(error));
+    }
+}
+
 int copyTo(int fd_in, int fd_out, void* buffer, size_t buffer_len) {
     ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd_in, buffer, buffer_len));
     if (bytes_read == 0) {
@@ -182,6 +258,7 @@
                 line.append(1, c);
             }
         }
+        *out_bytes_written += bytes_read;
     }
     s.reset();
     // Process final line, in case it didn't finish with newline.
@@ -194,6 +271,168 @@
     return true;
 }
 
+bool waitpid_with_timeout(pid_t pid, int timeout_secs, int* status) {
+    sigset_t child_mask, old_mask;
+    sigemptyset(&child_mask);
+    sigaddset(&child_mask, SIGCHLD);
+
+    if (sigprocmask(SIG_BLOCK, &child_mask, &old_mask) == -1) {
+        ALOGE("*** sigprocmask failed: %s\n", strerror(errno));
+        return false;
+    }
+
+    timespec ts = {.tv_sec = timeout_secs, .tv_nsec = 0};
+    int ret = TEMP_FAILURE_RETRY(sigtimedwait(&child_mask, nullptr, &ts));
+    int saved_errno = errno;
+
+    // Set the signals back the way they were.
+    if (sigprocmask(SIG_SETMASK, &old_mask, nullptr) == -1) {
+        ALOGE("*** sigprocmask failed: %s\n", strerror(errno));
+        if (ret == 0) {
+            return false;
+        }
+    }
+    if (ret == -1) {
+        errno = saved_errno;
+        if (errno == EAGAIN) {
+            errno = ETIMEDOUT;
+        } else {
+            ALOGE("*** sigtimedwait failed: %s\n", strerror(errno));
+        }
+        return false;
+    }
+
+    pid_t child_pid = waitpid(pid, status, WNOHANG);
+    if (child_pid != pid) {
+        if (child_pid != -1) {
+            ALOGE("*** Waiting for pid %d, got pid %d instead\n", pid, child_pid);
+        } else {
+            ALOGE("*** waitpid failed: %s\n", strerror(errno));
+        }
+        return false;
+    }
+    return true;
+}
+
+// Runs the given command. Kills the command if it does not finish by timeout.
+int runCommand(int timeout_secs, const char* file, std::vector<const char*> args) {
+    pid_t pid = fork();
+
+    // handle error case
+    if (pid < 0) {
+        ALOGE("fork failed %s", strerror(errno));
+        return pid;
+    }
+
+    // handle child case
+    if (pid == 0) {
+        /* make sure the child dies when parent dies */
+        prctl(PR_SET_PDEATHSIG, SIGKILL);
+
+        /* just ignore SIGPIPE, will go down with parent's */
+        struct sigaction sigact;
+        memset(&sigact, 0, sizeof(sigact));
+        sigact.sa_handler = SIG_IGN;
+        sigaction(SIGPIPE, &sigact, nullptr);
+
+        execvp(file, (char**)args.data());
+        // execvp's result will be handled after waitpid_with_timeout() below, but
+        // if it failed, it's safer to exit dumpstate.
+        ALOGE("execvp on command %s failed (error: %s)", file, strerror(errno));
+        _exit(EXIT_FAILURE);
+    }
+
+    // handle parent case
+    int status;
+    bool ret = waitpid_with_timeout(pid, timeout_secs, &status);
+
+    if (!ret) {
+        if (errno == ETIMEDOUT) {
+            ALOGE("command %s timed out (killing pid %d)", file, pid);
+        } else {
+            ALOGE("command %s: Error (killing pid %d)\n", file, pid);
+        }
+        kill(pid, SIGTERM);
+        if (!waitpid_with_timeout(pid, 5, nullptr)) {
+            kill(pid, SIGKILL);
+            if (!waitpid_with_timeout(pid, 5, nullptr)) {
+                ALOGE("could not kill command '%s' (pid %d) even with SIGKILL.\n", file, pid);
+            }
+        }
+        return -1;
+    }
+
+    if (WIFSIGNALED(status)) {
+        ALOGE("command '%s' failed: killed by signal %d\n", file, WTERMSIG(status));
+    } else if (WIFEXITED(status) && WEXITSTATUS(status) > 0) {
+        status = WEXITSTATUS(status);
+        ALOGE("command '%s' failed: exit code %d\n", file, status);
+    }
+
+    return status;
+}
+
+void takeScreenshotForDisplayId(int id, const char* tmp_dir,
+        std::vector<std::string>* extra_files) {
+    std::string id_as_string = std::to_string(id);
+    std::string filename = std::string(tmp_dir) + kScreenshotPrefix + id_as_string + ".png";
+    std::vector<const char*> args { "-p", "-d", id_as_string.c_str(), filename.c_str(), nullptr };
+    ALOGI("capturing screen for display (%d) as %s", id, filename.c_str());
+    int status = runCommand(10, "/system/bin/screencap", args);
+    if (status == 0) {
+        LOG(INFO) << "Screenshot saved for display:" << id_as_string;
+    }
+    // add the file regardless of the exit status of the screencap util.
+    extra_files->push_back(filename);
+
+    LOG(ERROR) << "Failed to take screenshot for display:" << id_as_string;
+}
+
+void takeScreenshot(const char* tmp_dir, std::vector<std::string>* extra_files) {
+    // Now send the screencaptures
+    std::vector<PhysicalDisplayId> ids = SurfaceComposerClient::getPhysicalDisplayIds();
+
+    for (PhysicalDisplayId display_id : ids) {
+        takeScreenshotForDisplayId(display_id, tmp_dir, extra_files);
+    }
+}
+
+bool recursiveRemoveDir(const std::string& path) {
+    auto callback = [](const char* child, const struct stat*, int file_type, struct FTW*) -> int {
+        if (file_type == FTW_DP) {
+            if (rmdir(child) == -1) {
+                ALOGE("rmdir(%s): %s", child, strerror(errno));
+                return -1;
+            }
+        } else if (file_type == FTW_F) {
+            if (unlink(child) == -1) {
+                ALOGE("unlink(%s): %s", child, strerror(errno));
+                return -1;
+            }
+        }
+        return 0;
+    };
+    // do a file tree walk with a sufficiently large depth.
+    return nftw(path.c_str(), callback, 128, FTW_DEPTH) == 0;
+}
+
+status_t createTempDir(const char* dir) {
+    struct stat sb;
+    if (TEMP_FAILURE_RETRY(stat(dir, &sb)) == 0) {
+        if (!recursiveRemoveDir(dir)) {
+            return -errno;
+        }
+    } else if (errno != ENOENT) {
+        ALOGE("Failed to stat %s ", dir);
+        return -errno;
+    }
+    if (TEMP_FAILURE_RETRY(mkdir(dir, 0700)) == -1) {
+        ALOGE("Failed to mkdir %s", dir);
+        return -errno;
+    }
+    return OK;
+}
+
 // Removes bugreport
 void cleanupBugreportFile(const std::string& zip_path) {
     if (unlink(zip_path.c_str()) != 0) {
@@ -204,10 +443,16 @@
 }  // namespace
 
 int main(void) {
-    ALOGE("Starting bugreport collecting service");
+    ALOGI("Starting bugreport collecting service");
 
     auto t0 = std::chrono::steady_clock::now();
 
+    std::vector<std::string> extra_files;
+    if (createTempDir(kTempDirectory) == OK) {
+        // take screenshots of the physical displays as early as possible
+        takeScreenshot(kTempDirectory, &extra_files);
+    }
+
     // Start the dumpstatez service.
     android::base::SetProperty("ctl.start", "car-dumpstatez");
 
@@ -231,6 +476,14 @@
         close(output_socket);
     }
 
+    int extra_output_socket = openSocket(kCarBrExtraOutputSocket);
+    if (extra_output_socket != -1 && ret_val) {
+        zipFilesToFd(extra_files, extra_output_socket);
+    }
+    if (extra_output_socket != -1) {
+        close(extra_output_socket);
+    }
+
     auto delta = std::chrono::duration_cast<std::chrono::duration<double>>(
                      std::chrono::steady_clock::now() - t0)
                      .count();
@@ -239,6 +492,8 @@
     ALOGI("bugreport %s in %.02fs, %zu bytes written", result.c_str(), delta, bytes_written);
     cleanupBugreportFile(zip_path);
 
+    recursiveRemoveDir(kTempDirectory);
+
     // No matter how doBugreport() finished, let's try to explicitly stop
     // car-dumpstatez in case it stalled.
     android::base::SetProperty("ctl.stop", "car-dumpstatez");
diff --git a/car-lib/src/android/car/CarBugreportManager.java b/car-lib/src/android/car/CarBugreportManager.java
index b0f7a03..b74c446 100644
--- a/car-lib/src/android/car/CarBugreportManager.java
+++ b/car-lib/src/android/car/CarBugreportManager.java
@@ -159,26 +159,34 @@
     }
 
     /**
-     * Request a bug report. A zipped bugreport is generated in the background.
+     * Request a bug report. A zipped (i.e. legacy) bugreport is generated in the background
+     * using dumpstate. This API also generates extra files that does not exist in the legacy
+     * bugreport and makes them available through a extra output file. Currently the extra
+     * output contains the screenshots for all the physical displays.
      *
      * <p>The file descriptor is closed when bugreport is written or if an exception happens.
      *
      * @param output the zipped bugreport file
+     * @param extraOutput a zip file that contains extra files generated for automotive.
      * @param callback  the callback for reporting dump status
      */
     @RequiresPermission(android.Manifest.permission.DUMP)
-    public void requestZippedBugreport(
-            @NonNull ParcelFileDescriptor output, @NonNull CarBugreportManagerCallback callback) {
+    public void requestBugreport(
+            @NonNull ParcelFileDescriptor output,
+            @NonNull ParcelFileDescriptor extraOutput,
+            @NonNull CarBugreportManagerCallback callback) {
         Preconditions.checkNotNull(output);
+        Preconditions.checkNotNull(extraOutput);
         Preconditions.checkNotNull(callback);
         try {
             CarBugreportManagerCallbackWrapper wrapper =
                     new CarBugreportManagerCallbackWrapper(callback, mHandler);
-            mService.requestZippedBugreport(output, wrapper);
+            mService.requestBugreport(output, extraOutput, wrapper);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         } finally {
             IoUtils.closeQuietly(output);
+            IoUtils.closeQuietly(extraOutput);
         }
     }
 
diff --git a/car-lib/src/android/car/ICarBugreportService.aidl b/car-lib/src/android/car/ICarBugreportService.aidl
index 1ec4e91..2673c80 100644
--- a/car-lib/src/android/car/ICarBugreportService.aidl
+++ b/car-lib/src/android/car/ICarBugreportService.aidl
@@ -28,7 +28,10 @@
     /**
      * 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. The file descriptor is written by the service and will be read by the client.
+     * zip file. The "extra_output" file descriptor will be provided to add files that does not
+     * exist in the original file.
+     * The file descriptor is written by the service and will be read by the client.
      */
-    void requestZippedBugreport(in ParcelFileDescriptor output, ICarBugreportCallback callback) = 1;
+    void requestBugreport(in ParcelFileDescriptor output,
+            in ParcelFileDescriptor extraOutput, ICarBugreportCallback callback) = 1;
  }
diff --git a/car_product/sepolicy/private/file_contexts b/car_product/sepolicy/private/file_contexts
index 908c853..9666f70 100644
--- a/car_product/sepolicy/private/file_contexts
+++ b/car_product/sepolicy/private/file_contexts
@@ -6,3 +6,4 @@
 /system/bin/car-bugreportd  u:object_r:dumpstate_exec:s0
 /dev/socket/car_br_progress_socket  u:object_r:dumpstate_socket:s0
 /dev/socket/car_br_output_socket  u:object_r:dumpstate_socket:s0
+/dev/socket/car_br_extra_output_socket u:object_r:dumpstate_socket:s0
diff --git a/service/src/com/android/car/CarBugreportManagerService.java b/service/src/com/android/car/CarBugreportManagerService.java
index 572f17d..52d0d33 100644
--- a/service/src/com/android/car/CarBugreportManagerService.java
+++ b/service/src/com/android/car/CarBugreportManagerService.java
@@ -52,7 +52,7 @@
 import java.io.PrintWriter;
 
 /**
- * Bugreport service for cars. Should *only* be used on userdebug or eng builds.
+ * Bugreport service for cars.
  */
 public class CarBugreportManagerService extends ICarBugreportService.Stub implements
         CarServiceBase {
@@ -75,8 +75,10 @@
     // definition.
     private static final String BUGREPORT_PROGRESS_SOCKET = "car_br_progress_socket";
     private static final String BUGREPORT_OUTPUT_SOCKET = "car_br_output_socket";
+    private static final String BUGREPORT_EXTRA_OUTPUT_SOCKET = "car_br_extra_output_socket";
 
     private static final int SOCKET_CONNECTION_MAX_RETRY = 10;
+    private static final int SOCKET_CONNECTION_RETRY_DELAY_IN_MS = 5000;
 
     private final Context mContext;
     private final Object mLock = new Object();
@@ -108,7 +110,8 @@
 
     @Override
     @RequiresPermission(android.Manifest.permission.DUMP)
-    public void requestZippedBugreport(ParcelFileDescriptor data, ICarBugreportCallback callback) {
+    public void requestBugreport(ParcelFileDescriptor output, ParcelFileDescriptor extraOutput,
+            ICarBugreportCallback callback) {
 
         // Check the caller has proper permission
         mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP,
@@ -138,23 +141,24 @@
         }
 
         synchronized (mLock) {
-            requestZippedBugReportLocked(data, callback);
+            requestBugReportLocked(output, extraOutput, callback);
         }
     }
 
     @GuardedBy("mLock")
-    private void requestZippedBugReportLocked(
-            ParcelFileDescriptor data, ICarBugreportCallback callback) {
+    private void requestBugReportLocked(ParcelFileDescriptor output,
+            ParcelFileDescriptor extraOutput, 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, callback));
+        mHandler.post(() -> startBugreportd(output, extraOutput, callback));
     }
 
-    private void startBugreportd(ParcelFileDescriptor data, ICarBugreportCallback callback) {
+    private void startBugreportd(ParcelFileDescriptor output, ParcelFileDescriptor extraOutput,
+            ICarBugreportCallback callback) {
         Slog.i(TAG, "Starting " + BUGREPORTD_SERVICE);
         try {
             SystemProperties.set("ctl.start", BUGREPORTD_SERVICE);
@@ -163,7 +167,7 @@
             reportError(callback, CAR_BUGREPORT_DUMPSTATE_FAILED);
             return;
         }
-        processBugreportSockets(data, callback);
+        processBugreportSockets(output, extraOutput, callback);
         synchronized (mLock) {
             mIsServiceRunning = false;
         }
@@ -196,9 +200,14 @@
         }
     }
 
-    private void handleFinished(ParcelFileDescriptor output, ICarBugreportCallback callback) {
+    private void handleFinished(ParcelFileDescriptor output, ParcelFileDescriptor extraOutput,
+            ICarBugreportCallback callback) {
         Slog.i(TAG, "Finished reading bugreport");
-        if (!copySocketToPfd(output, callback)) {
+        // copysockettopfd calls callback.onError on error
+        if (!copySocketToPfd(output, BUGREPORT_OUTPUT_SOCKET, callback)) {
+            return;
+        }
+        if (!copySocketToPfd(extraOutput, BUGREPORT_EXTRA_OUTPUT_SOCKET, callback)) {
             return;
         }
         try {
@@ -216,7 +225,8 @@
      * {@code FAIL:message} accordingly.
      */
     private void processBugreportSockets(
-            ParcelFileDescriptor output, ICarBugreportCallback callback) {
+            ParcelFileDescriptor output, ParcelFileDescriptor extraOutput,
+            ICarBugreportCallback callback) {
         LocalSocket localSocket = connectSocket(BUGREPORT_PROGRESS_SOCKET);
         if (localSocket == null) {
             reportError(callback, CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED);
@@ -235,7 +245,7 @@
                     reportError(callback, CAR_BUGREPORT_DUMPSTATE_FAILED);
                     return;
                 } else if (line.startsWith(OK_PREFIX)) {
-                    handleFinished(output, callback);
+                    handleFinished(output, extraOutput, callback);
                     return;
                 } else if (!line.startsWith(BEGIN_PREFIX)) {
                     Slog.w(TAG, "Received unknown progress line from dumpstate: " + line);
@@ -250,8 +260,8 @@
     }
 
     private boolean copySocketToPfd(
-            ParcelFileDescriptor pfd, ICarBugreportCallback callback) {
-        LocalSocket localSocket = connectSocket(BUGREPORT_OUTPUT_SOCKET);
+            ParcelFileDescriptor pfd, String remoteSocket, ICarBugreportCallback callback) {
+        LocalSocket localSocket = connectSocket(remoteSocket);
         if (localSocket == null) {
             reportError(callback, CAR_BUGREPORT_DUMPSTATE_CONNECTION_FAILED);
             return false;
@@ -292,8 +302,15 @@
         // keep retrying until success or reaching timeout.
         int retryCount = 0;
         while (true) {
-            // First connection always fails, so we wait 1 second before trying to connect.
-            SystemClock.sleep(/* ms= */ 1000);
+            // There are a few factors impacting the socket delay:
+            // 1. potential system slowness
+            // 2. car-bugreportd takes the screenshots early (before starting dumpstate). This
+            //    should be taken into account as the socket opens after screenshots are
+            //    captured.
+            // Therefore we are generous in setting the timeout. Most cases should not even
+            // come close to the timeouts, but since bugreports are taken when there is a
+            // system issue, it is hard to guess.
+            SystemClock.sleep(SOCKET_CONNECTION_RETRY_DELAY_IN_MS);
             try {
                 socket.connect(new LocalSocketAddress(socketName,
                         LocalSocketAddress.Namespace.RESERVED));
@@ -304,7 +321,7 @@
                             + " 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 cce327d..981faab 100644
--- a/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java
+++ b/tests/BugReportApp/src/com/google/android/car/bugreport/BugReportService.java
@@ -18,7 +18,6 @@
 import static com.google.android.car.bugreport.PackageUtils.getPackageVersion;
 
 import android.annotation.FloatRange;
-import android.annotation.Nullable;
 import android.annotation.StringRes;
 import android.app.Notification;
 import android.app.NotificationChannel;
@@ -28,15 +27,14 @@
 import android.car.CarBugreportManager;
 import android.car.CarNotConnectedException;
 import android.content.Intent;
-import android.hardware.display.DisplayManager;
 import android.os.Binder;
+import android.os.Build;
 import android.os.Bundle;
 import android.os.Handler;
 import android.os.IBinder;
 import android.os.Message;
 import android.os.ParcelFileDescriptor;
 import android.util.Log;
-import android.view.Display;
 import android.widget.Toast;
 
 import com.google.common.util.concurrent.AtomicDouble;
@@ -52,9 +50,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.ArrayList;
 import java.util.Enumeration;
-import java.util.List;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -90,9 +86,8 @@
     private static final String NOTIFICATION_STATUS_CHANNEL_ID = "BUGREPORT_STATUS_CHANNEL_ID";
     private static final int BUGREPORT_IN_PROGRESS_NOTIF_ID = 1;
 
-    // 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 EXTRA_OUTPUT_ZIP_FILE = "extra_output_file.zip";
 
     private static final String MESSAGE_FAILURE_DUMPSTATE = "Failed to grab dumpstate";
     private static final String MESSAGE_FAILURE_ZIP = "Failed to zip files";
@@ -232,61 +227,12 @@
     }
 
     private void collectBugReport() {
-        // Order is important when capturing. Screenshot should be first
-        mSingleThreadExecutor.schedule(
-                this::takeAllScreenshots, ACTIVITY_FINISH_DELAY, TimeUnit.MILLISECONDS);
-        mSingleThreadExecutor.schedule(
-                this::grabBtSnoopLog, ACTIVITY_FINISH_DELAY, TimeUnit.MILLISECONDS);
-        mSingleThreadExecutor.schedule(
-                this::dumpStateToFile, ACTIVITY_FINISH_DELAY, TimeUnit.MILLISECONDS);
-    }
-
-    private void takeAllScreenshots() {
-        for (int displayId : getAvailableDisplayIds()) {
-            takeScreenshot(displayId);
+        if (Build.IS_USERDEBUG || Build.IS_ENG) {
+            mSingleThreadExecutor.schedule(
+                    this::grabBtSnoopLog, ACTIVITY_FINISH_DELAY, TimeUnit.MILLISECONDS);
         }
-    }
-
-    @Nullable
-    private File takeScreenshot(int displayId) {
-        Log.i(TAG, String.format("takeScreenshot displayId=%d", displayId));
-        File result = FileUtils.getFileWithSuffix(this, mMetaBugReport.getTimestamp(),
-                "-" + displayId + "-screenshot.png");
-        try {
-            if (DEBUG) {
-                Log.d(TAG, "Screen output: " + result.getName());
-            }
-
-            java.lang.Process process = Runtime.getRuntime()
-                    .exec("/system/bin/screencap -d " + displayId + " -p "
-                            + result.getAbsolutePath());
-
-            // Waits for the command to finish.
-            int err = process.waitFor();
-            if (DEBUG) {
-                Log.d(TAG, "screencap process finished: " + err);
-            }
-            return result;
-        } catch (IOException | InterruptedException e) {
-            Log.e(TAG, "screencap process failed: ", e);
-            showToast(R.string.toast_status_screencap_failed);
-        }
-        return null;
-    }
-
-    private List<Integer> getAvailableDisplayIds() {
-        DisplayManager displayManager = getSystemService(DisplayManager.class);
-        ArrayList<Integer> displayIds = new ArrayList<>();
-        for (Display d : displayManager.getDisplays()) {
-            Log.v(TAG,
-                    "getAvailableDisplayIds: d.Name=" + d.getName() + ", d.id=" + d.getDisplayId());
-            // We skip virtual displays as they are not captured by screencap.
-            if (d.getName().contains(ACTIVITY_VIEW_VIRTUAL_DISPLAY)) {
-                continue;
-            }
-            displayIds.add(d.getDisplayId());
-        }
-        return displayIds;
+        mSingleThreadExecutor.schedule(
+                this::saveBugReport, ACTIVITY_FINISH_DELAY, TimeUnit.MILLISECONDS);
     }
 
     private void grabBtSnoopLog() {
@@ -302,13 +248,16 @@
         }
     }
 
-    private void dumpStateToFile() {
+    private void saveBugReport() {
         Log.i(TAG, "Dumpstate to file");
         File outputFile = FileUtils.getFile(this, mMetaBugReport.getTimestamp(), OUTPUT_ZIP_FILE);
-
+        File extraOutputFile = FileUtils.getFile(this, mMetaBugReport.getTimestamp(),
+                EXTRA_OUTPUT_ZIP_FILE);
         try (ParcelFileDescriptor outFd = ParcelFileDescriptor.open(outputFile,
+                ParcelFileDescriptor.MODE_CREATE | ParcelFileDescriptor.MODE_READ_WRITE);
+             ParcelFileDescriptor extraOutFd = ParcelFileDescriptor.open(extraOutputFile,
                 ParcelFileDescriptor.MODE_CREATE | ParcelFileDescriptor.MODE_READ_WRITE)) {
-            requestBugReport(outFd);
+            requestBugReport(outFd, extraOutFd);
         } catch (IOException | RuntimeException e) {
             Log.e(TAG, "Failed to grab dump state", e);
             BugStorageUtils.setBugReportStatus(this, mMetaBugReport, Status.STATUS_WRITE_FAILED,
@@ -324,7 +273,7 @@
         mHandler.sendMessage(message);
     }
 
-    private void requestBugReport(ParcelFileDescriptor outFd) {
+    private void requestBugReport(ParcelFileDescriptor outFd, ParcelFileDescriptor extraOutFd) {
         if (DEBUG) {
             Log.d(TAG, "Requesting a bug report from CarBugReportManager.");
         }
@@ -355,7 +304,7 @@
                 sendProgressEventToHandler(MAX_PROGRESS_VALUE);
             }
         };
-        mBugreportManager.requestZippedBugreport(outFd, mCallback);
+        mBugreportManager.requestBugreport(outFd, extraOutFd, mCallback);
     }
 
     private void scheduleZipTask() {
@@ -433,8 +382,9 @@
                     continue;
                 }
                 String filename = file.getName();
-                // only for the OUTPUT_FILE, we add invidiual entries to zip file
-                if (filename.equals(OUTPUT_ZIP_FILE)) {
+
+                // only for the zipped output file, we add invidiual entries to zip file
+                if (filename.equals(OUTPUT_ZIP_FILE) || filename.equals(EXTRA_OUTPUT_ZIP_FILE)) {
                     extractZippedFileToOutputStream(file, zipStream);
                 } else {
                     FileInputStream reader = new FileInputStream(file);
diff --git a/tests/BugReportApp/src/com/google/android/car/bugreport/SystemUtils.java b/tests/BugReportApp/src/com/google/android/car/bugreport/SystemUtils.java
deleted file mode 100644
index 6a47298..0000000
--- a/tests/BugReportApp/src/com/google/android/car/bugreport/SystemUtils.java
+++ /dev/null
@@ -1,81 +0,0 @@
-/*
- * Copyright (C) 2019 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.google.android.car.bugreport;
-
-import android.os.Build;
-import android.util.Log;
-
-import java.io.BufferedReader;
-import java.io.InputStreamReader;
-
-/**
- * Android System utility class.
- */
-final class SystemUtils {
-    private static final String TAG = SystemUtils.class.getSimpleName();
-
-    private static final String ENFORCING = "Enforcing";
-    private static final String PERMISSIVE = "Permissive";
-
-    private static final String VOLVO_FINGERPRINT_PREFIX = "VolvoCars";
-
-    /**
-     * Returns {@code true} if SELinux is {@code Enforcing}. If it can't verify it returns {@code
-     * true} because most likely SELinux is not allowing running {@code getenforce}.
-     *
-     * <p>Returns {@code false} only when SELinux is {@code Permissive}.
-     */
-    static boolean isSeLinuxEnforcing() {
-        StringBuilder result = new StringBuilder();
-        Process p;
-        try {
-            p = Runtime.getRuntime().exec("getenforce");
-            p.waitFor();
-            BufferedReader reader = new BufferedReader(new InputStreamReader(p.getInputStream()));
-            String line;
-            while ((line = reader.readLine()) != null) {
-                result.append(line);
-            }
-        } catch (Exception e) {
-            Log.e(TAG, "OS does not support getenforce", e);
-            // If getenforce is not available to the device, assume the device is not enforcing
-            return false;
-        }
-        String response = result.toString();
-        if (ENFORCING.equals(response)) {
-            return true;
-        } else if (PERMISSIVE.equals(response)) {
-            return false;
-        } else {
-            Log.e(TAG, "getenforce returned unexpected value, assuming selinux is enforcing!");
-            // If getenforce doesn't return anything, most likely SELinux is in enforcing mode.
-            return true;
-        }
-    }
-
-    /** Returns {@code true} if the app is running on Volvo cars. */
-    static boolean isVolvo() {
-        return Build.FINGERPRINT.startsWith(VOLVO_FINGERPRINT_PREFIX);
-    }
-
-    /** Returns {@code true} if the app is running on Android P. */
-    static boolean isAndroidP() {
-        return Build.VERSION.SDK_INT == Build.VERSION_CODES.P;
-    }
-
-    private SystemUtils() {
-    }
-}