Merge "Log duration reporter to logcat"
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp
index bb089e6..768cb4f 100644
--- a/cmds/dumpstate/DumpstateService.cpp
+++ b/cmds/dumpstate/DumpstateService.cpp
@@ -44,17 +44,18 @@
     return binder::Status::fromExceptionCode(code, String8(msg.c_str()));
 }
 
-static binder::Status error(uint32_t code, const std::string& msg) {
-    MYLOGE("%s (%d) ", msg.c_str(), code);
-    return binder::Status::fromServiceSpecificError(code, String8(msg.c_str()));
-}
-
-// Takes ownership of data.
-static void* callAndNotify(void* data) {
+// Creates a bugreport and exits, thus preserving the oneshot nature of the service.
+// Note: takes ownership of data.
+[[noreturn]] static void* dumpstate_thread_main(void* data) {
     std::unique_ptr<DumpstateInfo> ds_info(static_cast<DumpstateInfo*>(data));
     ds_info->ds->Run(ds_info->calling_uid, ds_info->calling_package);
-    MYLOGD("Finished Run()\n");
-    return nullptr;
+    MYLOGD("Finished taking a bugreport. Exiting.\n");
+    exit(0);
+}
+
+[[noreturn]] static void signalErrorAndExit(sp<IDumpstateListener> listener, int error_code) {
+    listener->onError(error_code);
+    exit(0);
 }
 
 class DumpstateToken : public BnDumpstateToken {};
@@ -120,6 +121,25 @@
                                                 const sp<IDumpstateListener>& listener) {
     MYLOGI("startBugreport() with mode: %d\n", bugreport_mode);
 
+    // This is the bugreporting API flow, so ensure there is only one bugreport in progress at a
+    // time.
+    std::lock_guard<std::mutex> lock(lock_);
+    if (ds_ != nullptr) {
+        MYLOGE("Error! There is already a bugreport in progress. Returning.");
+        if (listener != nullptr) {
+            listener->onError(IDumpstateListener::BUGREPORT_ERROR_CONCURRENT_BUGREPORTS_FORBIDDEN);
+        }
+        return exception(binder::Status::EX_SERVICE_SPECIFIC,
+                         "There is already a bugreport in progress");
+    }
+
+    // From here on, all conditions that indicate we are done with this incoming request should
+    // result in exiting the service to free it up for next invocation.
+    if (listener == nullptr) {
+        MYLOGE("Invalid input: no listener");
+        exit(0);
+    }
+
     if (bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_FULL &&
         bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE &&
         bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_REMOTE &&
@@ -127,30 +147,23 @@
         bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_TELEPHONY &&
         bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_WIFI &&
         bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_DEFAULT) {
-        return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
-                         StringPrintf("Invalid bugreport mode: %d", bugreport_mode));
+        MYLOGE("Invalid input: bad bugreport mode: %d", bugreport_mode);
+        signalErrorAndExit(listener, IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);
     }
 
     if (bugreport_fd.get() == -1 || screenshot_fd.get() == -1) {
-        return exception(binder::Status::EX_ILLEGAL_ARGUMENT, "Invalid file descriptor");
+        // TODO(b/111441001): screenshot fd should be optional
+        MYLOGE("Invalid filedescriptor");
+        signalErrorAndExit(listener, IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);
     }
 
     std::unique_ptr<Dumpstate::DumpOptions> options = std::make_unique<Dumpstate::DumpOptions>();
     options->Initialize(static_cast<Dumpstate::BugreportMode>(bugreport_mode), bugreport_fd,
                         screenshot_fd);
 
-    // This is the bugreporting API flow, so ensure there is only one bugreport in progress at a
-    // time.
-    std::lock_guard<std::mutex> lock(lock_);
-    if (ds_ != nullptr) {
-        return exception(binder::Status::EX_SERVICE_SPECIFIC,
-                         "There is already a bugreport in progress");
-    }
     ds_ = &(Dumpstate::GetInstance());
     ds_->SetOptions(std::move(options));
-    if (listener != nullptr) {
-        ds_->listener_ = listener;
-    }
+    ds_->listener_ = listener;
 
     DumpstateInfo* ds_info = new DumpstateInfo();
     ds_info->ds = ds_;
@@ -158,11 +171,12 @@
     ds_info->calling_package = calling_package;
 
     pthread_t thread;
-    status_t err = pthread_create(&thread, nullptr, callAndNotify, ds_info);
+    status_t err = pthread_create(&thread, nullptr, dumpstate_thread_main, ds_info);
     if (err != 0) {
         delete ds_info;
         ds_info = nullptr;
-        return error(err, "Could not create a background thread.");
+        MYLOGE("Could not create a thread");
+        signalErrorAndExit(listener, IDumpstateListener::BUGREPORT_ERROR_RUNTIME_ERROR);
     }
     return binder::Status::ok();
 }
diff --git a/cmds/dumpstate/README.md b/cmds/dumpstate/README.md
index 1bf55e4..c818c05 100644
--- a/cmds/dumpstate/README.md
+++ b/cmds/dumpstate/README.md
@@ -14,7 +14,8 @@
 mmm -j frameworks/native/cmds/dumpstate
 ```
 
-If you're working on device-specific code, you might need to build them as well. Example:
+If you're working on device-specific code, you might need to build them as well.
+Example:
 
 ```
 mmm -j frameworks/native/cmds/dumpstate device/acme/secret_device/dumpstate/ hardware/interfaces/dumpstate
@@ -26,17 +27,20 @@
 mmm -j frameworks/native/cmds/dumpstate && adb push ${OUT}/system/bin/dumpstate system/bin && adb push ${OUT}/system/lib64/*dumpstate*.so /system/lib64/ && adb shell am bug-report
 ```
 
-Make sure that the device is remounted before running the above command.
-* If you're working with `userdebug` variant, you may need to run the following to remount your device:
+Make sure that the device is remounted before running the above command. * If
+you're working with `userdebug` variant, you may need to run the following to
+remount your device:
 
-  ```
+```
   adb root && adb remount -R && adb wait-for-device && adb root && adb remount
-  ```
-* If you're working with `eng` variant, you may need to run the following to remount your device:
+```
 
-  ```
-  adb root && adb remount
-  ```
+*   If you're working with `eng` variant, you may need to run the following to
+    remount your device:
+
+    ```
+    adb root && adb remount
+    ```
 
 ## To build, deploy, and run unit tests
 
@@ -82,7 +86,6 @@
 adb shell setprop dumpstate.version split-dumpsys && adb shell dumpstate -v
 ```
 
-
 Then to restore the default version:
 
 ```
@@ -91,8 +94,9 @@
 
 ## Code style and formatting
 
-Use the style defined at the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html)
-and make sure to run the following command prior to `repo upload`:
+Use the style defined at the
+[Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) and
+make sure to run the following command prior to `repo upload`:
 
 ```
 git clang-format --style=file HEAD~
diff --git a/cmds/dumpstate/binder/android/os/IDumpstate.aidl b/cmds/dumpstate/binder/android/os/IDumpstate.aidl
index b1005d3..37ff442 100644
--- a/cmds/dumpstate/binder/android/os/IDumpstate.aidl
+++ b/cmds/dumpstate/binder/android/os/IDumpstate.aidl
@@ -36,6 +36,9 @@
     IDumpstateToken setListener(@utf8InCpp String name, IDumpstateListener listener,
                                 boolean getSectionDetails);
 
+    // NOTE: If you add to or change these modes, please also change the corresponding enums
+    // in system server, in BugreportParams.java.
+
     // These modes encapsulate a set of run time options for generating bugreports.
     // Takes a bugreport without user interference.
     const int BUGREPORT_MODE_FULL = 0;
diff --git a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
index 907a67c..42858e0 100644
--- a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
+++ b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
@@ -19,6 +19,11 @@
 /**
   * Listener for dumpstate events.
   *
+  * <p>When bugreport creation is complete one of {@code onError} or {@code onFinished} is called.
+  *
+  * <p>These methods are synchronous by design in order to make dumpstate's lifecycle simpler
+  * to handle.
+  *
   * {@hide}
   */
 interface IDumpstateListener {
@@ -27,7 +32,10 @@
      *
      * @param progress the progress in [0, 100]
      */
-    oneway void onProgress(int progress);
+    void onProgress(int progress);
+
+    // NOTE: If you add to or change these error codes, please also change the corresponding enums
+    // in system server, in BugreportManager.java.
 
     /* Options specified are invalid or incompatible */
     const int BUGREPORT_ERROR_INVALID_INPUT = 1;
@@ -41,15 +49,18 @@
     /* The request to get user consent timed out */
     const int BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT = 4;
 
+    /* There is currently a bugreport running. The caller should try again later. */
+    const int BUGREPORT_ERROR_CONCURRENT_BUGREPORTS_FORBIDDEN = 5;
+
     /**
      * Called on an error condition with one of the error codes listed above.
      */
-    oneway void onError(int errorCode);
+    void onError(int errorCode);
 
     /**
      * Called when taking bugreport finishes successfully.
      */
-     oneway void onFinished();
+    void onFinished();
 
     // TODO(b/111441001): Remove old methods when not used anymore.
     void onProgressUpdated(int progress);
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 8bd834d..06dc58f 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -113,6 +113,7 @@
 #define LOGPERSIST_DATA_DIR "/data/misc/logd"
 #define PROFILE_DATA_DIR_CUR "/data/misc/profiles/cur"
 #define PROFILE_DATA_DIR_REF "/data/misc/profiles/ref"
+#define XFRM_STAT_PROC_FILE "/proc/net/xfrm_stat"
 #define WLUTIL "/vendor/xbin/wlutil"
 #define WMTRACE_DATA_DIR "/data/misc/wmtrace"
 
@@ -157,7 +158,7 @@
 }
 
 static bool CopyFileToFd(const std::string& input_file, int out_fd) {
-    MYLOGD("Going to copy bugreport file (%s) to %d\n", ds.path_.c_str(), out_fd);
+    MYLOGD("Going to copy file (%s) to %d\n", input_file.c_str(), out_fd);
 
     // Obtain a handle to the source file.
     android::base::unique_fd in_fd(OpenForRead(input_file));
@@ -165,14 +166,14 @@
         if (CopyFile(in_fd.get(), out_fd)) {
             return true;
         }
-        MYLOGE("Failed to copy zip file: %s\n", strerror(errno));
+        MYLOGE("Failed to copy file: %s\n", strerror(errno));
     }
     return false;
 }
 
 static bool UnlinkAndLogOnError(const std::string& file) {
-    if (unlink(file.c_str()) != -1) {
-        MYLOGE("Failed to remove file (%s): %s\n", file.c_str(), strerror(errno));
+    if (unlink(file.c_str())) {
+        MYLOGE("Failed to unlink file (%s): %s\n", file.c_str(), strerror(errno));
         return false;
     }
     return true;
@@ -460,9 +461,7 @@
             if (!ds.AddZipEntry("anrd_trace.txt", path)) {
                 MYLOGE("Unable to add anrd_trace file %s to zip file\n", path);
             } else {
-                if (remove(path)) {
-                    MYLOGE("Error removing anrd_trace file %s: %s", path, strerror(errno));
-                }
+                android::os::UnlinkAndLogOnError(path);
                 return true;
             }
         } else {
@@ -767,6 +766,17 @@
                ZipWriter::ErrorCodeString(err));
         return UNKNOWN_ERROR;
     }
+    bool finished_entry = false;
+    auto finish_entry = [this, &finished_entry] {
+        if (!finished_entry) {
+            // This should only be called when we're going to return an earlier error,
+            // which would've been logged. This may imply the file is already corrupt
+            // and any further logging from FinishEntry is more likely to mislead than
+            // not.
+            this->zip_writer_->FinishEntry();
+        }
+    };
+    auto scope_guard = android::base::make_scope_guard(finish_entry);
     auto start = std::chrono::steady_clock::now();
     auto end = start + timeout;
     struct pollfd pfd = {fd, POLLIN};
@@ -783,11 +793,11 @@
 
             int rc = TEMP_FAILURE_RETRY(poll(&pfd, 1, time_left_ms()));
             if (rc < 0) {
-                MYLOGE("Error in poll while adding from fd to zip entry %s:%s", entry_name.c_str(),
-                       strerror(errno));
+                MYLOGE("Error in poll while adding from fd to zip entry %s:%s\n",
+                       entry_name.c_str(), strerror(errno));
                 return -errno;
             } else if (rc == 0) {
-                MYLOGE("Timed out adding from fd to zip entry %s:%s Timeout:%lldms",
+                MYLOGE("Timed out adding from fd to zip entry %s:%s Timeout:%lldms\n",
                        entry_name.c_str(), strerror(errno), timeout.count());
                 return TIMED_OUT;
             }
@@ -808,6 +818,7 @@
     }
 
     err = zip_writer_->FinishEntry();
+    finished_entry = true;
     if (err != 0) {
         MYLOGE("zip_writer_->FinishEntry(): %s\n", ZipWriter::ErrorCodeString(err));
         return UNKNOWN_ERROR;
@@ -1330,6 +1341,7 @@
     DumpFile("BINDER STATS", "/sys/kernel/debug/binder/stats");
     DumpFile("BINDER STATE", "/sys/kernel/debug/binder/state");
 
+    RunDumpsys("WINSCOPE TRACE", {"window", "trace"});
     /* Add window and surface trace files. */
     if (!PropertiesHelper::IsUserBuild()) {
         ds.AddDir(WMTRACE_DATA_DIR, false);
@@ -1449,9 +1461,12 @@
     add_mountinfo();
     DumpIpTablesAsRoot();
 
-    // Capture any IPSec policies in play.  No keys are exposed here.
+    // Capture any IPSec policies in play. No keys are exposed here.
     RunCommand("IP XFRM POLICY", {"ip", "xfrm", "policy"}, CommandOptions::WithTimeout(10).Build());
 
+    // Dump IPsec stats. No keys are exposed here.
+    DumpFile("XFRM STATS", XFRM_STAT_PROC_FILE);
+
     // Run ss as root so we can see socket marks.
     RunCommand("DETAILED SOCKET STATE", {"ss", "-eionptu"}, CommandOptions::WithTimeout(10).Build());
 
@@ -1582,13 +1597,8 @@
     for (int i = 0; i < NUM_OF_DUMPS; i++) {
         paths.emplace_back(StringPrintf("%s/%s", ds.bugreport_internal_dir_.c_str(),
                                         kDumpstateBoardFiles[i].c_str()));
-        remover.emplace_back(android::base::make_scope_guard(std::bind(
-            [](std::string path) {
-                if (remove(path.c_str()) != 0 && errno != ENOENT) {
-                    MYLOGE("Could not remove(%s): %s\n", path.c_str(), strerror(errno));
-                }
-            },
-            paths[i])));
+        remover.emplace_back(android::base::make_scope_guard(
+            std::bind([](std::string path) { android::os::UnlinkAndLogOnError(path); }, paths[i])));
     }
 
     sp<IDumpstateDevice> dumpstate_device(IDumpstateDevice::getService());
@@ -1749,9 +1759,7 @@
     ds.zip_file.reset(nullptr);
 
     MYLOGD("Removing temporary file %s\n", tmp_path_.c_str())
-    if (remove(tmp_path_.c_str()) != 0) {
-        MYLOGE("Failed to remove temporary file (%s): %s\n", tmp_path_.c_str(), strerror(errno));
-    }
+    android::os::UnlinkAndLogOnError(tmp_path_);
 
     return true;
 }
@@ -2335,6 +2343,7 @@
 
     register_sig_handler();
 
+    // TODO(b/111441001): maybe skip if already started?
     if (options_->do_start_service) {
         MYLOGI("Starting 'dumpstate' service\n");
         android::status_t ret;
@@ -2481,15 +2490,24 @@
             // Do an early return if there were errors. We make an exception for consent
             // timing out because it's possible the user got distracted. In this case the
             // bugreport is not shared but made available for manual retrieval.
+            MYLOGI("User denied consent. Returning\n");
             return status;
         }
-        if (options_->screenshot_fd.get() != -1) {
+        if (options_->do_fb && options_->screenshot_fd.get() != -1) {
             bool copy_succeeded = android::os::CopyFileToFd(screenshot_path_,
                                                             options_->screenshot_fd.get());
             if (copy_succeeded) {
                 android::os::UnlinkAndLogOnError(screenshot_path_);
             }
         }
+        if (status == Dumpstate::RunStatus::USER_CONSENT_TIMED_OUT) {
+            MYLOGI(
+                "Did not receive user consent yet."
+                " Will not copy the bugreport artifacts to caller.\n");
+            // TODO(b/111441001):
+            // 1. cancel outstanding requests
+            // 2. check for result more frequently
+        }
     }
 
     /* vibrate a few but shortly times to let user know it's finished */
@@ -2592,21 +2610,26 @@
     return Dumpstate::RunStatus::ERROR;
 }
 
-/* Main entry point for dumpstate binary. */
-int run_main(int argc, char* argv[]) {
+Dumpstate::RunStatus Dumpstate::ParseCommandlineAndRun(int argc, char* argv[]) {
     std::unique_ptr<Dumpstate::DumpOptions> options = std::make_unique<Dumpstate::DumpOptions>();
     Dumpstate::RunStatus status = options->Initialize(argc, argv);
     if (status == Dumpstate::RunStatus::OK) {
-        ds.SetOptions(std::move(options));
+        SetOptions(std::move(options));
         // When directly running dumpstate binary, the output is not expected to be written
         // to any external file descriptor.
-        assert(ds.options_->bugreport_fd.get() == -1);
+        assert(options_->bugreport_fd.get() == -1);
 
         // calling_uid and calling_package are for user consent to share the bugreport with
         // an app; they are irrelvant here because bugreport is only written to a local
         // directory, and not shared.
-        status = ds.Run(-1 /* calling_uid */, "" /* calling_package */);
+        status = Run(-1 /* calling_uid */, "" /* calling_package */);
     }
+    return status;
+}
+
+/* Main entry point for dumpstate binary. */
+int run_main(int argc, char* argv[]) {
+    Dumpstate::RunStatus status = ds.ParseCommandlineAndRun(argc, argv);
 
     switch (status) {
         case Dumpstate::RunStatus::OK:
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 7fb2f3b..9803f00 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -322,6 +322,8 @@
     /* Main entry point for running a complete bugreport. */
     RunStatus Run(int32_t calling_uid, const std::string& calling_package);
 
+    RunStatus ParseCommandlineAndRun(int argc, char* argv[]);
+
     /* Sets runtime options. */
     void SetOptions(std::unique_ptr<DumpOptions> options);
 
diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
index 570c6c9..555badd 100644
--- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
@@ -21,6 +21,10 @@
 #include <libgen.h>
 
 #include <android-base/file.h>
+#include <android/os/BnDumpstate.h>
+#include <android/os/BnDumpstateListener.h>
+#include <binder/IServiceManager.h>
+#include <binder/ProcessState.h>
 #include <cutils/properties.h>
 #include <ziparchive/zip_archive.h>
 
@@ -34,6 +38,24 @@
 
 using ::testing::Test;
 using ::std::literals::chrono_literals::operator""s;
+using android::base::unique_fd;
+
+class DumpstateListener;
+
+namespace {
+
+sp<IDumpstate> GetDumpstateService() {
+    return android::interface_cast<IDumpstate>(
+        android::defaultServiceManager()->getService(String16("dumpstate")));
+}
+
+int OpenForWrite(const std::string& filename) {
+    return TEMP_FAILURE_RETRY(open(filename.c_str(),
+                                   O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
+                                   S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
+}
+
+}  // namespace
 
 struct SectionInfo {
     std::string name;
@@ -46,41 +68,71 @@
  * Listens to bugreport progress and updates the user by writing the progress to STDOUT. All the
  * section details generated by dumpstate are added to a vector to be used by Tests later.
  */
-class DumpstateListener : public IDumpstateListener {
+class DumpstateListener : public BnDumpstateListener {
   public:
-    int outFd_, max_progress_;
-    std::shared_ptr<std::vector<SectionInfo>> sections_;
     DumpstateListener(int fd, std::shared_ptr<std::vector<SectionInfo>> sections)
-        : outFd_(fd), max_progress_(5000), sections_(sections) {
+        : out_fd_(fd), sections_(sections) {
     }
+
+    DumpstateListener(int fd) : out_fd_(fd) {
+    }
+
     binder::Status onProgress(int32_t progress) override {
-        dprintf(outFd_, "\rIn progress %d", progress);
+        dprintf(out_fd_, "\rIn progress %d", progress);
         return binder::Status::ok();
     }
+
     binder::Status onError(int32_t error_code) override {
-        dprintf(outFd_, "\rError %d", error_code);
+        std::lock_guard<std::mutex> lock(lock_);
+        error_code_ = error_code;
+        dprintf(out_fd_, "\rError code %d", error_code);
         return binder::Status::ok();
     }
+
     binder::Status onFinished() override {
-        dprintf(outFd_, "\rFinished");
+        std::lock_guard<std::mutex> lock(lock_);
+        is_finished_ = true;
+        dprintf(out_fd_, "\rFinished");
         return binder::Status::ok();
     }
+
     binder::Status onProgressUpdated(int32_t progress) override {
-        dprintf(outFd_, "\rIn progress %d/%d", progress, max_progress_);
+        dprintf(out_fd_, "\rIn progress %d/%d", progress, max_progress_);
         return binder::Status::ok();
     }
+
     binder::Status onMaxProgressUpdated(int32_t max_progress) override {
+        std::lock_guard<std::mutex> lock(lock_);
         max_progress_ = max_progress;
         return binder::Status::ok();
     }
+
     binder::Status onSectionComplete(const ::std::string& name, int32_t status, int32_t size_bytes,
                                      int32_t duration_ms) override {
-        sections_->push_back({name, status, size_bytes, duration_ms});
+        std::lock_guard<std::mutex> lock(lock_);
+        if (sections_.get() != nullptr) {
+            sections_->push_back({name, status, size_bytes, duration_ms});
+        }
         return binder::Status::ok();
     }
-    IBinder* onAsBinder() override {
-        return nullptr;
+
+    bool getIsFinished() {
+        std::lock_guard<std::mutex> lock(lock_);
+        return is_finished_;
     }
+
+    int getErrorCode() {
+        std::lock_guard<std::mutex> lock(lock_);
+        return error_code_;
+    }
+
+  private:
+    int out_fd_;
+    int max_progress_ = 5000;
+    int error_code_ = -1;
+    bool is_finished_ = false;
+    std::shared_ptr<std::vector<SectionInfo>> sections_;
+    std::mutex lock_;
 };
 
 /**
@@ -109,7 +161,7 @@
         ds.listener_name_ = "Smokey";
         ds.report_section_ = true;
         auto start = std::chrono::steady_clock::now();
-        run_main(ARRAY_SIZE(argv), argv);
+        ds.ParseCommandlineAndRun(ARRAY_SIZE(argv), argv);
         auto end = std::chrono::steady_clock::now();
         duration = std::chrono::duration_cast<std::chrono::milliseconds>(end - start);
     }
@@ -293,6 +345,148 @@
     SectionExists("DUMPSYS - wifi", /* bytes= */ 100000);
 }
 
+class DumpstateBinderTest : public Test {
+  protected:
+    void SetUp() override {
+        // In case there is a stray service, stop it first.
+        property_set("ctl.stop", "bugreportd");
+        // dry_run results in a faster bugreport.
+        property_set("dumpstate.dry_run", "true");
+        // We need to receive some async calls later. Ensure we have binder threads.
+        ProcessState::self()->startThreadPool();
+    }
+
+    void TearDown() override {
+        property_set("ctl.stop", "bugreportd");
+        property_set("dumpstate.dry_run", "");
+
+        unlink("/data/local/tmp/tmp.zip");
+        unlink("/data/local/tmp/tmp.png");
+    }
+
+    // Waits until listener gets the callbacks.
+    void WaitTillExecutionComplete(DumpstateListener* listener) {
+        // Wait till one of finished, error or timeout.
+        static const int kBugreportTimeoutSeconds = 120;
+        int i = 0;
+        while (!listener->getIsFinished() && listener->getErrorCode() == -1 &&
+               i < kBugreportTimeoutSeconds) {
+            sleep(1);
+            i++;
+        }
+    }
+};
+
+TEST_F(DumpstateBinderTest, Baseline) {
+    // In the beginning dumpstate binder service is not running.
+    sp<android::os::IDumpstate> ds_binder(GetDumpstateService());
+    EXPECT_EQ(ds_binder, nullptr);
+
+    // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service
+    // and makes it wait.
+    property_set("dumpstate.dry_run", "true");
+    property_set("ctl.start", "bugreportd");
+
+    // Now we are able to retrieve dumpstate binder service.
+    ds_binder = GetDumpstateService();
+    EXPECT_NE(ds_binder, nullptr);
+
+    // Prepare arguments
+    unique_fd bugreport_fd(OpenForWrite("/bugreports/tmp.zip"));
+    unique_fd screenshot_fd(OpenForWrite("/bugreports/tmp.png"));
+
+    EXPECT_NE(bugreport_fd.get(), -1);
+    EXPECT_NE(screenshot_fd.get(), -1);
+
+    sp<DumpstateListener> listener(new DumpstateListener(dup(fileno(stdout))));
+    android::binder::Status status =
+        ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,
+                                  Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener);
+    // startBugreport is an async call. Verify binder call succeeded first, then wait till listener
+    // gets expected callbacks.
+    EXPECT_TRUE(status.isOk());
+    WaitTillExecutionComplete(listener.get());
+
+    // Bugreport generation requires user consent, which we cannot get in a test set up,
+    // so instead of getting is_finished_, we are more likely to get a consent error.
+    EXPECT_TRUE(
+        listener->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_DENIED_CONSENT ||
+        listener->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT);
+
+    // The service should have died on its own, freeing itself up for a new invocation.
+    sleep(2);
+    ds_binder = GetDumpstateService();
+    EXPECT_EQ(ds_binder, nullptr);
+}
+
+TEST_F(DumpstateBinderTest, ServiceDies_OnInvalidInput) {
+    // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service
+    // and makes it wait.
+    property_set("ctl.start", "bugreportd");
+    sp<android::os::IDumpstate> ds_binder(GetDumpstateService());
+    EXPECT_NE(ds_binder, nullptr);
+
+    // Prepare arguments
+    unique_fd bugreport_fd(OpenForWrite("/data/local/tmp/tmp.zip"));
+    unique_fd screenshot_fd(OpenForWrite("/data/local/tmp/tmp.png"));
+
+    EXPECT_NE(bugreport_fd.get(), -1);
+    EXPECT_NE(screenshot_fd.get(), -1);
+
+    // Call startBugreport with bad arguments.
+    sp<DumpstateListener> listener(new DumpstateListener(dup(fileno(stdout))));
+    android::binder::Status status =
+        ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,
+                                  2000,  // invalid bugreport mode
+                                  listener);
+    EXPECT_EQ(listener->getErrorCode(), IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);
+
+    // The service should have died, freeing itself up for a new invocation.
+    sleep(2);
+    ds_binder = GetDumpstateService();
+    EXPECT_EQ(ds_binder, nullptr);
+}
+
+TEST_F(DumpstateBinderTest, SimultaneousBugreportsNotAllowed) {
+    // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service
+    // and makes it wait.
+    property_set("dumpstate.dry_run", "true");
+    property_set("ctl.start", "bugreportd");
+    sp<android::os::IDumpstate> ds_binder(GetDumpstateService());
+    EXPECT_NE(ds_binder, nullptr);
+
+    // Prepare arguments
+    unique_fd bugreport_fd(OpenForWrite("/data/local/tmp/tmp.zip"));
+    unique_fd screenshot_fd(OpenForWrite("/data/local/tmp/tmp.png"));
+
+    EXPECT_NE(bugreport_fd.get(), -1);
+    EXPECT_NE(screenshot_fd.get(), -1);
+
+    sp<DumpstateListener> listener1(new DumpstateListener(dup(fileno(stdout))));
+    android::binder::Status status =
+        ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,
+                                  Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener1);
+    EXPECT_TRUE(status.isOk());
+
+    // try to make another call to startBugreport. This should fail.
+    sp<DumpstateListener> listener2(new DumpstateListener(dup(fileno(stdout))));
+    status = ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,
+                                       Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener2);
+    EXPECT_FALSE(status.isOk());
+    WaitTillExecutionComplete(listener2.get());
+    EXPECT_EQ(listener2->getErrorCode(),
+              IDumpstateListener::BUGREPORT_ERROR_CONCURRENT_BUGREPORTS_FORBIDDEN);
+
+    // Meanwhile the first call works as expected. Service should not die in this case.
+    WaitTillExecutionComplete(listener1.get());
+
+    // Bugreport generation requires user consent, which we cannot get in a test set up,
+    // so instead of getting is_finished_, we are more likely to get a consent error.
+    EXPECT_TRUE(
+        listener1->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_DENIED_CONSENT ||
+        listener1->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT);
+}
+
 }  // namespace dumpstate
 }  // namespace os
 }  // namespace android
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index ccffd29..a51946d 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -39,6 +39,7 @@
 #include <sys/xattr.h>
 #include <unistd.h>
 
+#include <android-base/file.h>
 #include <android-base/logging.h>
 #include <android-base/properties.h>
 #include <android-base/scopeguard.h>
@@ -80,6 +81,8 @@
 // An uuid used in unit tests.
 static constexpr const char* kTestUuid = "TEST";
 
+static constexpr const mode_t kRollbackFolderMode = 0700;
+
 static constexpr const char* kCpPath = "/system/bin/cp";
 static constexpr const char* kXattrDefault = "user.default";
 
@@ -784,8 +787,8 @@
 
 binder::Status InstalldNativeService::snapshotAppData(
         const std::unique_ptr<std::string>& volumeUuid,
-        const std::string& packageName, int32_t user, int32_t storageFlags,
-        int64_t* _aidl_return) {
+        const std::string& packageName, int32_t user, int32_t snapshotId,
+        int32_t storageFlags, int64_t* _aidl_return) {
     ENFORCE_UID(AID_SYSTEM);
     CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
     CHECK_ARGUMENT_PACKAGE_NAME(packageName);
@@ -802,16 +805,19 @@
     bool clear_ce_on_exit = false;
     bool clear_de_on_exit = false;
 
-    auto deleter = [&clear_ce_on_exit, &clear_de_on_exit, &volume_uuid, &user, &package_name] {
+    auto deleter = [&clear_ce_on_exit, &clear_de_on_exit, &volume_uuid, &user, &package_name,
+            &snapshotId] {
         if (clear_de_on_exit) {
-            auto to = create_data_misc_de_rollback_package_path(volume_uuid, user, package_name);
+            auto to = create_data_misc_de_rollback_package_path(volume_uuid, user, snapshotId,
+                    package_name);
             if (delete_dir_contents(to.c_str(), 1, nullptr) != 0) {
                 LOG(WARNING) << "Failed to delete app data snapshot: " << to;
             }
         }
 
         if (clear_ce_on_exit) {
-            auto to = create_data_misc_ce_rollback_package_path(volume_uuid, user, package_name);
+            auto to = create_data_misc_ce_rollback_package_path(volume_uuid, user, snapshotId,
+                    package_name);
             if (delete_dir_contents(to.c_str(), 1, nullptr) != 0) {
                 LOG(WARNING) << "Failed to delete app data snapshot: " << to;
             }
@@ -847,15 +853,21 @@
 
     if (storageFlags & FLAG_STORAGE_DE) {
         auto from = create_data_user_de_package_path(volume_uuid, user, package_name);
-        auto to = create_data_misc_de_rollback_path(volume_uuid, user);
+        auto to = create_data_misc_de_rollback_path(volume_uuid, user, snapshotId);
+        auto rollback_package_path = create_data_misc_de_rollback_package_path(volume_uuid, user,
+            snapshotId, package_name);
 
-        int rd = delete_dir_contents(to, true /* ignore_if_missing */);
-        if (rd != 0) {
-            res = error(rd, "Failed clearing existing snapshot " + to);
-            return res;
+        int rc = create_dir_if_needed(to.c_str(), kRollbackFolderMode);
+        if (rc != 0) {
+            return error(rc, "Failed to create folder " + to);
         }
 
-        int rc = copy_directory_recursive(from.c_str(), to.c_str());
+        rc = delete_dir_contents(rollback_package_path, true /* ignore_if_missing */);
+        if (rc != 0) {
+            return error(rc, "Failed clearing existing snapshot " + rollback_package_path);
+        }
+
+        rc = copy_directory_recursive(from.c_str(), to.c_str());
         if (rc != 0) {
             res = error(rc, "Failed copying " + from + " to " + to);
             clear_de_on_exit = true;
@@ -865,15 +877,21 @@
 
     if (storageFlags & FLAG_STORAGE_CE) {
         auto from = create_data_user_ce_package_path(volume_uuid, user, package_name);
-        auto to = create_data_misc_ce_rollback_path(volume_uuid, user);
+        auto to = create_data_misc_ce_rollback_path(volume_uuid, user, snapshotId);
+        auto rollback_package_path = create_data_misc_ce_rollback_package_path(volume_uuid, user,
+            snapshotId, package_name);
 
-        int rd = delete_dir_contents(to, true /* ignore_if_missing */);
-        if (rd != 0) {
-            res = error(rd, "Failed clearing existing snapshot " + to);
-            return res;
+        int rc = create_dir_if_needed(to.c_str(), kRollbackFolderMode);
+        if (rc != 0) {
+            return error(rc, "Failed to create folder " + to);
         }
 
-        int rc = copy_directory_recursive(from.c_str(), to.c_str());
+        rc = delete_dir_contents(rollback_package_path, true /* ignore_if_missing */);
+        if (rc != 0) {
+            return error(rc, "Failed clearing existing snapshot " + rollback_package_path);
+        }
+
+        rc = copy_directory_recursive(from.c_str(), to.c_str());
         if (rc != 0) {
             res = error(rc, "Failed copying " + from + " to " + to);
             clear_ce_on_exit = true;
@@ -881,7 +899,7 @@
         }
         if (_aidl_return != nullptr) {
             auto ce_snapshot_path = create_data_misc_ce_rollback_package_path(volume_uuid, user,
-                    package_name);
+                    snapshotId, package_name);
             rc = get_path_inode(ce_snapshot_path, reinterpret_cast<ino_t*>(_aidl_return));
             if (rc != 0) {
                 res = error(rc, "Failed to get_path_inode for " + ce_snapshot_path);
@@ -896,8 +914,8 @@
 
 binder::Status InstalldNativeService::restoreAppDataSnapshot(
         const std::unique_ptr<std::string>& volumeUuid, const std::string& packageName,
-        const int32_t appId, const int64_t ceDataInode, const std::string& seInfo,
-        const int32_t user, int32_t storageFlags) {
+        const int32_t appId, const std::string& seInfo, const int32_t user,
+        const int32_t snapshotId, int32_t storageFlags) {
     ENFORCE_UID(AID_SYSTEM);
     CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
     CHECK_ARGUMENT_PACKAGE_NAME(packageName);
@@ -907,9 +925,9 @@
     const char* package_name = packageName.c_str();
 
     auto from_ce = create_data_misc_ce_rollback_package_path(volume_uuid,
-            user, package_name);
+            user, snapshotId, package_name);
     auto from_de = create_data_misc_de_rollback_package_path(volume_uuid,
-            user, package_name);
+            user, snapshotId, package_name);
 
     const bool needs_ce_rollback = (storageFlags & FLAG_STORAGE_CE) &&
         (access(from_ce.c_str(), F_OK) == 0);
@@ -926,7 +944,11 @@
     // app with no data in those cases is arguably better than leaving the app
     // with mismatched / stale data.
     LOG(INFO) << "Clearing app data for " << packageName << " to restore snapshot.";
-    binder::Status res = clearAppData(volumeUuid, packageName, user, storageFlags, ceDataInode);
+    // It's fine to pass 0 as ceDataInode here, because restoreAppDataSnapshot
+    // can only be called when user unlocks the phone, meaning that CE user data
+    // is decrypted.
+    binder::Status res = clearAppData(volumeUuid, packageName, user, storageFlags,
+            0 /* ceDataInode */);
     if (!res.isOk()) {
         return res;
     }
@@ -962,7 +984,8 @@
 
 binder::Status InstalldNativeService::destroyAppDataSnapshot(
         const std::unique_ptr<std::string> &volumeUuid, const std::string& packageName,
-        const int32_t user, const int64_t ceSnapshotInode, int32_t storageFlags) {
+        const int32_t user, const int64_t ceSnapshotInode, const int32_t snapshotId,
+        int32_t storageFlags) {
     ENFORCE_UID(AID_SYSTEM);
     CHECK_ARGUMENT_UUID_IS_TEST_OR_NULL(volumeUuid);
     CHECK_ARGUMENT_PACKAGE_NAME(packageName);
@@ -973,7 +996,7 @@
 
     if (storageFlags & FLAG_STORAGE_DE) {
         auto de_snapshot_path = create_data_misc_de_rollback_package_path(volume_uuid,
-                user, package_name);
+                user, snapshotId, package_name);
 
         int res = delete_dir_contents_and_dir(de_snapshot_path, true /* ignore_if_missing */);
         if (res != 0) {
@@ -983,7 +1006,7 @@
 
     if (storageFlags & FLAG_STORAGE_CE) {
         auto ce_snapshot_path = create_data_misc_ce_rollback_package_path(volume_uuid,
-                user, package_name, ceSnapshotInode);
+                user, snapshotId, package_name, ceSnapshotInode);
         int res = delete_dir_contents_and_dir(ce_snapshot_path, true /* ignore_if_missing */);
         if (res != 0) {
             return error(res, "Failed clearing snapshot " + ce_snapshot_path);
diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h
index 578132d..0e91cb2 100644
--- a/cmds/installd/InstalldNativeService.h
+++ b/cmds/installd/InstalldNativeService.h
@@ -61,14 +61,14 @@
     binder::Status fixupAppData(const std::unique_ptr<std::string>& uuid, int32_t flags);
 
     binder::Status snapshotAppData(const std::unique_ptr<std::string>& volumeUuid,
-            const std::string& packageName, const int32_t user, int32_t storageFlags,
-            int64_t* _aidl_return);
+            const std::string& packageName, const int32_t user, const int32_t snapshotId,
+            int32_t storageFlags, int64_t* _aidl_return);
     binder::Status restoreAppDataSnapshot(const std::unique_ptr<std::string>& volumeUuid,
-            const std::string& packageName, const int32_t appId, const int64_t ceDataInode,
-            const std::string& seInfo, const int32_t user, int32_t storageFlags);
+            const std::string& packageName, const int32_t appId, const std::string& seInfo,
+            const int32_t user, const int32_t snapshotId, int32_t storageFlags);
     binder::Status destroyAppDataSnapshot(const std::unique_ptr<std::string> &volumeUuid,
             const std::string& packageName, const int32_t user, const int64_t ceSnapshotInode,
-            int32_t storageFlags);
+            const int32_t snapshotId, int32_t storageFlags);
 
     binder::Status getAppSize(const std::unique_ptr<std::string>& uuid,
             const std::vector<std::string>& packageNames, int32_t userId, int32_t flags,
diff --git a/cmds/installd/TEST_MAPPING b/cmds/installd/TEST_MAPPING
index 3de5c79..287f2d9 100644
--- a/cmds/installd/TEST_MAPPING
+++ b/cmds/installd/TEST_MAPPING
@@ -14,6 +14,15 @@
     },
     {
       "name": "installd_utils_test"
+    },
+    {
+      "name": "CtsUsesLibraryHostTestCases"
+    },
+    {
+      "name": "CtsClassloaderSplitsHostTestCases"
+    },
+    {
+      "name": "CtsCompilationTestCases"
     }
   ]
 }
diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl
index 377074a..51fe66b 100644
--- a/cmds/installd/binder/android/os/IInstalld.aidl
+++ b/cmds/installd/binder/android/os/IInstalld.aidl
@@ -106,11 +106,11 @@
         @nullable @utf8InCpp String dexMetadata);
 
     long snapshotAppData(@nullable @utf8InCpp String uuid, in @utf8InCpp String packageName,
-            int userId, int storageFlags);
+            int userId, int snapshotId, int storageFlags);
     void restoreAppDataSnapshot(@nullable @utf8InCpp String uuid, in @utf8InCpp String packageName,
-            int appId, long ceDataInode, @utf8InCpp String seInfo, int user, int storageflags);
+            int appId, @utf8InCpp String seInfo, int user, int snapshotId, int storageflags);
     void destroyAppDataSnapshot(@nullable @utf8InCpp String uuid, @utf8InCpp String packageName,
-            int userId, long ceSnapshotInode, int storageFlags);
+            int userId, long ceSnapshotInode, int snapshotId, int storageFlags);
 
     const int FLAG_STORAGE_DE = 0x1;
     const int FLAG_STORAGE_CE = 0x2;
diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp
index 852aa79..acc0647 100644
--- a/cmds/installd/dexopt.cpp
+++ b/cmds/installd/dexopt.cpp
@@ -352,6 +352,8 @@
         bool generate_minidebug_info = kEnableMinidebugInfo &&
                 GetBoolProperty(kMinidebugInfoSystemProperty, kMinidebugInfoSystemPropertyDefault);
 
+        std::string boot_image = MapPropertyToArg("dalvik.vm.boot-image", "-Ximage:%s");
+
         // clang FORTIFY doesn't let us use strlen in constant array bounds, so we
         // use arraysize instead.
         std::string zip_fd_arg = StringPrintf("--zip-fd=%d", zip_fd);
@@ -366,7 +368,7 @@
         std::string dex2oat_image_fd;
         std::string target_sdk_version_arg;
         if (target_sdk_version != 0) {
-            StringPrintf("-Xtarget-sdk-version:%d", target_sdk_version);
+            target_sdk_version_arg = StringPrintf("-Xtarget-sdk-version:%d", target_sdk_version);
         }
         std::string class_loader_context_arg;
         if (class_loader_context != nullptr) {
@@ -437,6 +439,7 @@
         AddArg(instruction_set_variant_arg);
         AddArg(instruction_set_features_arg);
 
+        AddRuntimeArg(boot_image);
         AddRuntimeArg(dex2oat_Xms_arg);
         AddRuntimeArg(dex2oat_Xmx_arg);
 
@@ -468,7 +471,7 @@
         if (disable_cdex) {
             AddArg(kDisableCompactDexFlag);
         }
-        AddArg(target_sdk_version_arg);
+        AddRuntimeArg(target_sdk_version_arg);
         if (enable_hidden_api_checks) {
             AddRuntimeArg("-Xhidden-api-checks");
         }
diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp
index ed1a0f4..cd63931 100644
--- a/cmds/installd/tests/installd_service_test.cpp
+++ b/cmds/installd/tests/installd_service_test.cpp
@@ -259,32 +259,59 @@
         return false;
     }
 
-    return (::mkdir(path.c_str(), mode) != -1);
+    if (::mkdir(path.c_str(), mode) != 0) {
+        PLOG(DEBUG) << "Failed to create folder " << path;
+        return false;
+    }
+    return true;
 }
 
-TEST_F(ServiceTest, CreateAppDataSnapshot) {
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+class AppDataSnapshotTest : public testing::Test {
+private:
+    std::string rollback_ce_base_dir;
+    std::string rollback_de_base_dir;
 
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
+protected:
+    InstalldNativeService* service;
 
-  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
-  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
+    std::string fake_package_ce_path;
+    std::string fake_package_de_path;
 
-  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
+    virtual void SetUp() {
+        setenv("ANDROID_LOG_TAGS", "*:v", 1);
+        android::base::InitLogging(nullptr);
 
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
-          &fake_package_ce_path, &fake_package_de_path]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      delete_dir_contents(fake_package_ce_path, true);
-      delete_dir_contents(fake_package_de_path, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+        service = new InstalldNativeService();
+        ASSERT_TRUE(mkdirs("/data/local/tmp/user/0", 0700));
+
+        init_globals_from_data_and_root();
+
+        rollback_ce_base_dir = create_data_misc_ce_rollback_base_path("TEST", 0);
+        rollback_de_base_dir = create_data_misc_de_rollback_base_path("TEST", 0);
+
+        fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
+        fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
+
+        ASSERT_TRUE(mkdirs(rollback_ce_base_dir, 0700));
+        ASSERT_TRUE(mkdirs(rollback_de_base_dir, 0700));
+        ASSERT_TRUE(mkdirs(fake_package_ce_path, 0700));
+        ASSERT_TRUE(mkdirs(fake_package_de_path, 0700));
+    }
+
+    virtual void TearDown() {
+        ASSERT_EQ(0, delete_dir_contents_and_dir(rollback_ce_base_dir, true));
+        ASSERT_EQ(0, delete_dir_contents_and_dir(rollback_de_base_dir, true));
+        ASSERT_EQ(0, delete_dir_contents(fake_package_ce_path, true));
+        ASSERT_EQ(0, delete_dir_contents(fake_package_de_path, true));
+
+        delete service;
+        ASSERT_EQ(0, delete_dir_contents_and_dir("/data/local/tmp/user/0", true));
+    }
+};
+
+TEST_F(AppDataSnapshotTest, CreateAppDataSnapshot) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 37);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 37);
 
   ASSERT_TRUE(android::base::WriteStringToFile(
           "TEST_CONTENT_CE", fake_package_ce_path + "/file1",
@@ -296,7 +323,7 @@
   // Request a snapshot of the CE content but not the DE content.
   int64_t ce_snapshot_inode;
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_CE, &ce_snapshot_inode));
+          "com.foo", 0, 37, FLAG_STORAGE_CE, &ce_snapshot_inode));
   struct stat buf;
   memset(&buf, 0, sizeof(buf));
   ASSERT_EQ(0, stat((rollback_ce_dir + "/com.foo").c_str(), &buf));
@@ -318,7 +345,7 @@
 
   // Request a snapshot of the DE content but not the CE content.
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE, &ce_snapshot_inode));
+          "com.foo", 0, 37, FLAG_STORAGE_DE, &ce_snapshot_inode));
   // Only DE content snapshot was requested.
   ASSERT_EQ(ce_snapshot_inode, 0);
 
@@ -339,7 +366,7 @@
 
   // Request a snapshot of both the CE as well as the DE content.
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE, nullptr));
+          "com.foo", 0, 37, FLAG_STORAGE_DE | FLAG_STORAGE_CE, nullptr));
 
   ASSERT_TRUE(android::base::ReadFileToString(
       rollback_ce_dir + "/com.foo/file1", &ce_content, false /* follow_symlinks */));
@@ -349,27 +376,73 @@
   ASSERT_EQ("TEST_CONTENT_DE_MODIFIED", de_content);
 }
 
-TEST_F(ServiceTest, CreateAppDataSnapshot_AppDataAbsent) {
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+TEST_F(AppDataSnapshotTest, CreateAppDataSnapshot_TwoSnapshotsWithTheSameId) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 67);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 67);
 
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
+  auto another_fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.bar");
+  auto another_fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.bar");
 
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
+  // Since this test sets up data for another package, some bookkeeping is required.
+  auto deleter = [&]() {
+      ASSERT_EQ(0, delete_dir_contents_and_dir(another_fake_package_ce_path, true));
+      ASSERT_EQ(0, delete_dir_contents_and_dir(another_fake_package_de_path, true));
   };
-
   auto scope_guard = android::base::make_scope_guard(deleter);
 
+  ASSERT_TRUE(mkdirs(another_fake_package_ce_path, 0700));
+  ASSERT_TRUE(mkdirs(another_fake_package_de_path, 0700));
+
+  ASSERT_TRUE(android::base::WriteStringToFile(
+          "TEST_CONTENT_CE", fake_package_ce_path + "/file1",
+          0700, 10000, 20000, false /* follow_symlinks */));
+  ASSERT_TRUE(android::base::WriteStringToFile(
+          "TEST_CONTENT_DE", fake_package_de_path + "/file1",
+          0700, 10000, 20000, false /* follow_symlinks */));
+  ASSERT_TRUE(android::base::WriteStringToFile(
+          "ANOTHER_TEST_CONTENT_CE", another_fake_package_ce_path + "/file1",
+          0700, 10000, 20000, false /* follow_symlinks */));
+  ASSERT_TRUE(android::base::WriteStringToFile(
+          "ANOTHER_TEST_CONTENT_DE", another_fake_package_de_path + "/file1",
+          0700, 10000, 20000, false /* follow_symlinks */));
+
+  // Request snapshot for the package com.foo.
+  ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
+          "com.foo", 0, 67, FLAG_STORAGE_DE | FLAG_STORAGE_CE, nullptr));
+  // Now request snapshot with the same id for the package com.bar
+  ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
+          "com.bar", 0, 67, FLAG_STORAGE_DE | FLAG_STORAGE_CE, nullptr));
+
+  // Check that both snapshots have correct data in them.
+  std::string com_foo_ce_content, com_foo_de_content;
+  std::string com_bar_ce_content, com_bar_de_content;
+  ASSERT_TRUE(android::base::ReadFileToString(
+      rollback_ce_dir + "/com.foo/file1", &com_foo_ce_content, false /* follow_symlinks */));
+  ASSERT_TRUE(android::base::ReadFileToString(
+      rollback_de_dir + "/com.foo/file1", &com_foo_de_content, false /* follow_symlinks */));
+  ASSERT_TRUE(android::base::ReadFileToString(
+      rollback_ce_dir + "/com.bar/file1", &com_bar_ce_content, false /* follow_symlinks */));
+  ASSERT_TRUE(android::base::ReadFileToString(
+      rollback_de_dir + "/com.bar/file1", &com_bar_de_content, false /* follow_symlinks */));
+  ASSERT_EQ("TEST_CONTENT_CE", com_foo_ce_content);
+  ASSERT_EQ("TEST_CONTENT_DE", com_foo_de_content);
+  ASSERT_EQ("ANOTHER_TEST_CONTENT_CE", com_bar_ce_content);
+  ASSERT_EQ("ANOTHER_TEST_CONTENT_DE", com_bar_de_content);
+}
+
+TEST_F(AppDataSnapshotTest, CreateAppDataSnapshot_AppDataAbsent) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 73);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 73);
+
+  // Similuating app data absence.
+  ASSERT_EQ(0, delete_dir_contents_and_dir(fake_package_ce_path, true));
+  ASSERT_EQ(0, delete_dir_contents_and_dir(fake_package_de_path, true));
+
   int64_t ce_snapshot_inode;
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_CE, &ce_snapshot_inode));
+          "com.foo", 0, 73, FLAG_STORAGE_CE, &ce_snapshot_inode));
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE, nullptr));
+          "com.foo", 0, 73, FLAG_STORAGE_DE, nullptr));
   // No CE content snapshot was performed.
   ASSERT_EQ(ce_snapshot_inode, 0);
 
@@ -380,29 +453,12 @@
   ASSERT_EQ(-1, stat((rollback_de_dir + "/com.foo").c_str(), &sb));
 }
 
-TEST_F(ServiceTest, CreateAppDataSnapshot_ClearsExistingSnapshot) {
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+TEST_F(AppDataSnapshotTest, CreateAppDataSnapshot_ClearsExistingSnapshot) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_package_path("TEST", 0, 13, "com.foo");
+  auto rollback_de_dir = create_data_misc_de_rollback_package_path("TEST", 0, 13, "com.foo");
 
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
-
-  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
-  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
-
-  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
-
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
-          &fake_package_ce_path, &fake_package_de_path]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      delete_dir_contents(fake_package_ce_path, true);
-      delete_dir_contents(fake_package_de_path, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+  ASSERT_TRUE(mkdirs(rollback_ce_dir, 0700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir, 0700));
 
   // Simulate presence of an existing snapshot
   ASSERT_TRUE(android::base::WriteStringToFile(
@@ -421,62 +477,40 @@
           0700, 10000, 20000, false /* follow_symlinks */));
 
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE, nullptr));
+          "com.foo", 0, 13, FLAG_STORAGE_DE | FLAG_STORAGE_CE, nullptr));
 
   // Previous snapshot (with data for file1) must be cleared.
   struct stat sb;
-  ASSERT_EQ(-1, stat((rollback_ce_dir + "/com.foo/file1").c_str(), &sb));
-  ASSERT_EQ(-1, stat((rollback_de_dir + "/com.foo/file1").c_str(), &sb));
+  ASSERT_EQ(-1, stat((rollback_ce_dir + "/file1").c_str(), &sb));
+  ASSERT_EQ(-1, stat((rollback_de_dir + "/file1").c_str(), &sb));
+  // New snapshot (with data for file2) must be present.
+  ASSERT_NE(-1, stat((rollback_ce_dir + "/file2").c_str(), &sb));
+  ASSERT_NE(-1, stat((rollback_de_dir + "/file2").c_str(), &sb));
 }
 
-TEST_F(ServiceTest, SnapshotAppData_WrongVolumeUuid) {
-  // Setup app data to make sure that fails due to wrong volumeUuid being
+TEST_F(AppDataSnapshotTest, SnapshotAppData_WrongVolumeUuid) {
+  // Setup rollback folders to make sure that fails due to wrong volumeUuid being
   // passed, not because of some other reason.
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 17);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 17);
 
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
-
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+  ASSERT_TRUE(mkdirs(rollback_ce_dir, 0700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir, 0700));
 
   EXPECT_BINDER_FAIL(service->snapshotAppData(std::make_unique<std::string>("FOO"),
-          "com.foo", 0, FLAG_STORAGE_DE, nullptr));
+          "com.foo", 0, 17, FLAG_STORAGE_DE, nullptr));
 }
 
-TEST_F(ServiceTest, CreateAppDataSnapshot_ClearsCache) {
-  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
-  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
+TEST_F(AppDataSnapshotTest, CreateAppDataSnapshot_ClearsCache) {
   auto fake_package_ce_cache_path = fake_package_ce_path + "/cache";
   auto fake_package_ce_code_cache_path = fake_package_ce_path + "/code_cache";
   auto fake_package_de_cache_path = fake_package_de_path + "/cache";
   auto fake_package_de_code_cache_path = fake_package_de_path + "/code_cache";
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
 
-  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_ce_cache_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_ce_code_cache_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_cache_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_code_cache_path, 700));
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
-
-  auto deleter = [&fake_package_ce_path, &fake_package_de_path,
-          &rollback_ce_dir, &rollback_de_dir]() {
-      delete_dir_contents(fake_package_ce_path, true);
-      delete_dir_contents(fake_package_de_path, true);
-      delete_dir_contents_and_dir(rollback_ce_dir, true);
-      delete_dir_contents_and_dir(rollback_de_dir, true);
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+  ASSERT_TRUE(mkdirs(fake_package_ce_cache_path, 0700));
+  ASSERT_TRUE(mkdirs(fake_package_ce_code_cache_path, 0700));
+  ASSERT_TRUE(mkdirs(fake_package_de_cache_path, 0700));
+  ASSERT_TRUE(mkdirs(fake_package_de_code_cache_path, 0700));
 
   ASSERT_TRUE(android::base::WriteStringToFile(
           "TEST_CONTENT_CE", fake_package_ce_cache_path + "/file1",
@@ -491,7 +525,7 @@
           "TEST_CONTENT_DE", fake_package_de_code_cache_path + "/file1",
           0700, 10000, 20000, false /* follow_symlinks */));
   ASSERT_BINDER_SUCCESS(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_CE | FLAG_STORAGE_DE, nullptr));
+          "com.foo", 0, 23, FLAG_STORAGE_CE | FLAG_STORAGE_DE, nullptr));
   // The snapshot call must clear cache.
   struct stat sb;
   ASSERT_EQ(-1, stat((fake_package_ce_cache_path + "/file1").c_str(), &sb));
@@ -500,34 +534,17 @@
   ASSERT_EQ(-1, stat((fake_package_de_code_cache_path + "/file1").c_str(), &sb));
 }
 
-TEST_F(ServiceTest, RestoreAppDataSnapshot) {
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+TEST_F(AppDataSnapshotTest, RestoreAppDataSnapshot) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 239);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 239);
 
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
-
-  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
-  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
-
-  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
-
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
-          &fake_package_ce_path, &fake_package_de_path]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      delete_dir_contents(fake_package_ce_path, true);
-      delete_dir_contents(fake_package_de_path, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+  ASSERT_TRUE(mkdirs(rollback_ce_dir, 0700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir, 0700));
 
   // Write contents to the rollback location. We'll write the same files to the
   // app data location and make sure the restore has overwritten them.
-  ASSERT_TRUE(mkdirs(rollback_ce_dir + "/com.foo/", 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir + "/com.foo/", 700));
+  ASSERT_TRUE(mkdirs(rollback_ce_dir + "/com.foo/", 0700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir + "/com.foo/", 0700));
   ASSERT_TRUE(android::base::WriteStringToFile(
           "CE_RESTORE_CONTENT", rollback_ce_dir + "/com.foo/file1",
           0700, 10000, 20000, false /* follow_symlinks */));
@@ -542,7 +559,7 @@
           0700, 10000, 20000, false /* follow_symlinks */));
 
   ASSERT_BINDER_SUCCESS(service->restoreAppDataSnapshot(std::make_unique<std::string>("TEST"),
-          "com.foo", 10000, -1, "", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE));
+          "com.foo", 10000, "", 0, 239, FLAG_STORAGE_DE | FLAG_STORAGE_CE));
 
   std::string ce_content, de_content;
   ASSERT_TRUE(android::base::ReadFileToString(
@@ -553,29 +570,9 @@
   ASSERT_EQ("DE_RESTORE_CONTENT", de_content);
 }
 
-TEST_F(ServiceTest, CreateSnapshotThenDestroyIt) {
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
-
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
-
-  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
-  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
-
-  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
-
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
-          &fake_package_ce_path, &fake_package_de_path]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      delete_dir_contents(fake_package_ce_path, true);
-      delete_dir_contents(fake_package_de_path, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+TEST_F(AppDataSnapshotTest, CreateSnapshotThenDestroyIt) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 57);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 57);
 
   // Prepare data for snapshot.
   ASSERT_TRUE(android::base::WriteStringToFile(
@@ -588,7 +585,7 @@
   int64_t ce_snapshot_inode;
   // Request a snapshot of both the CE as well as the DE content.
   ASSERT_TRUE(service->snapshotAppData(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE, &ce_snapshot_inode).isOk());
+          "com.foo", 0, 57, FLAG_STORAGE_DE | FLAG_STORAGE_CE, &ce_snapshot_inode).isOk());
   // Because CE data snapshot was requested, ce_snapshot_inode can't be null.
   ASSERT_NE(0, ce_snapshot_inode);
   // Check snapshot is there.
@@ -598,39 +595,19 @@
 
 
   ASSERT_TRUE(service->destroyAppDataSnapshot(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, ce_snapshot_inode, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
+          "com.foo", 0, ce_snapshot_inode, 57, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
   // Check snapshot is deleted.
   ASSERT_EQ(-1, stat((rollback_ce_dir + "/com.foo").c_str(), &sb));
   ASSERT_EQ(-1, stat((rollback_de_dir + "/com.foo").c_str(), &sb));
 }
 
-TEST_F(ServiceTest, DestroyAppDataSnapshot_CeSnapshotInodeIsZero) {
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
-
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
-
-  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
-  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
-
-  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
-
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
-          &fake_package_ce_path, &fake_package_de_path]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      delete_dir_contents(fake_package_ce_path, true);
-      delete_dir_contents(fake_package_de_path, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+TEST_F(AppDataSnapshotTest, DestroyAppDataSnapshot_CeSnapshotInodeIsZero) {
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 1543);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 1543);
 
   // Create a snapshot
-  ASSERT_TRUE(mkdirs(rollback_ce_dir + "/com.foo/", 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir + "/com.foo/", 700));
+  ASSERT_TRUE(mkdirs(rollback_ce_dir + "/com.foo/", 0700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir + "/com.foo/", 0700));
   ASSERT_TRUE(android::base::WriteStringToFile(
           "CE_RESTORE_CONTENT", rollback_ce_dir + "/com.foo/file1",
           0700, 10000, 20000, false /* follow_symlinks */));
@@ -639,7 +616,7 @@
           0700, 10000, 20000, false /* follow_symlinks */));
 
   ASSERT_TRUE(service->destroyAppDataSnapshot(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
+          "com.foo", 0, 0, 1543, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
 
   // Check snapshot is deleted.
   struct stat sb;
@@ -648,67 +625,33 @@
 
   // Check that deleting already deleted snapshot is no-op.
   ASSERT_TRUE(service->destroyAppDataSnapshot(std::make_unique<std::string>("TEST"),
-          "com.foo", 0, 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
+          "com.foo", 0, 0, 1543, FLAG_STORAGE_DE | FLAG_STORAGE_CE).isOk());
 }
 
-TEST_F(ServiceTest, DestroyAppDataSnapshot_WrongVolumeUuid) {
+TEST_F(AppDataSnapshotTest, DestroyAppDataSnapshot_WrongVolumeUuid) {
   // Setup rollback data to make sure that test fails due to wrong volumeUuid
   // being passed, not because of some other reason.
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 43);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 43);
 
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
-
-  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
-  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
-
-  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
-
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
-          &fake_package_ce_path, &fake_package_de_path]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      delete_dir_contents(fake_package_ce_path, true);
-      delete_dir_contents(fake_package_de_path, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+  ASSERT_TRUE(mkdirs(rollback_ce_dir, 0700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir, 0700));
 
   ASSERT_FALSE(service->destroyAppDataSnapshot(std::make_unique<std::string>("BAR"),
-          "com.foo", 0, 0, FLAG_STORAGE_DE).isOk());
+          "com.foo", 0, 0, 43, FLAG_STORAGE_DE).isOk());
 }
 
-TEST_F(ServiceTest, RestoreAppDataSnapshot_WrongVolumeUuid) {
+TEST_F(AppDataSnapshotTest, RestoreAppDataSnapshot_WrongVolumeUuid) {
   // Setup rollback data to make sure that fails due to wrong volumeUuid being
   // passed, not because of some other reason.
-  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0);
-  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0);
+  auto rollback_ce_dir = create_data_misc_ce_rollback_path("TEST", 0, 41);
+  auto rollback_de_dir = create_data_misc_de_rollback_path("TEST", 0, 41);
 
-  ASSERT_TRUE(mkdirs(rollback_ce_dir, 700));
-  ASSERT_TRUE(mkdirs(rollback_de_dir, 700));
-
-  auto fake_package_ce_path = create_data_user_ce_package_path("TEST", 0, "com.foo");
-  auto fake_package_de_path = create_data_user_de_package_path("TEST", 0, "com.foo");
-
-  ASSERT_TRUE(mkdirs(fake_package_ce_path, 700));
-  ASSERT_TRUE(mkdirs(fake_package_de_path, 700));
-
-  auto deleter = [&rollback_ce_dir, &rollback_de_dir,
-          &fake_package_ce_path, &fake_package_de_path]() {
-      delete_dir_contents(rollback_ce_dir, true);
-      delete_dir_contents(rollback_de_dir, true);
-      delete_dir_contents(fake_package_ce_path, true);
-      delete_dir_contents(fake_package_de_path, true);
-      rmdir(rollback_ce_dir.c_str());
-      rmdir(rollback_de_dir.c_str());
-  };
-  auto scope_guard = android::base::make_scope_guard(deleter);
+  ASSERT_TRUE(mkdirs(rollback_ce_dir, 0700));
+  ASSERT_TRUE(mkdirs(rollback_de_dir, 0700));
 
   EXPECT_BINDER_FAIL(service->restoreAppDataSnapshot(std::make_unique<std::string>("BAR"),
-          "com.foo", 10000, -1, "", 0, FLAG_STORAGE_DE));
+          "com.foo", 10000, "", 0, 41, FLAG_STORAGE_DE));
 }
 
 }  // namespace installd
diff --git a/cmds/installd/tests/installd_utils_test.cpp b/cmds/installd/tests/installd_utils_test.cpp
index 1782aa2..e61eb6e 100644
--- a/cmds/installd/tests/installd_utils_test.cpp
+++ b/cmds/installd/tests/installd_utils_test.cpp
@@ -546,56 +546,86 @@
 }
 
 TEST_F(UtilsTest, TestRollbackPaths) {
-    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
-            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo"));
-    EXPECT_EQ("/data/misc_ce/10/rollback/com.foo",
-            create_data_misc_ce_rollback_package_path(nullptr, 10, "com.foo"));
+    EXPECT_EQ("/data/misc_ce/0/rollback/239/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, 239, "com.foo"));
+    EXPECT_EQ("/data/misc_ce/10/rollback/37/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 10, 37, "com.foo"));
 
-    EXPECT_EQ("/data/misc_de/0/rollback/com.foo",
-            create_data_misc_de_rollback_package_path(nullptr, 0, "com.foo"));
-    EXPECT_EQ("/data/misc_de/10/rollback/com.foo",
-            create_data_misc_de_rollback_package_path(nullptr, 10, "com.foo"));
+    EXPECT_EQ("/data/misc_de/0/rollback/73/com.foo",
+            create_data_misc_de_rollback_package_path(nullptr, 0, 73, "com.foo"));
+    EXPECT_EQ("/data/misc_de/10/rollback/13/com.foo",
+            create_data_misc_de_rollback_package_path(nullptr, 10, 13, "com.foo"));
 
-    EXPECT_EQ("/data/misc_ce/0/rollback",
-            create_data_misc_ce_rollback_path(nullptr, 0));
-    EXPECT_EQ("/data/misc_ce/10/rollback",
-            create_data_misc_ce_rollback_path(nullptr, 10));
+    EXPECT_EQ("/data/misc_ce/0/rollback/57",
+            create_data_misc_ce_rollback_path(nullptr, 0, 57));
+    EXPECT_EQ("/data/misc_ce/10/rollback/1543",
+            create_data_misc_ce_rollback_path(nullptr, 10, 1543));
 
-    EXPECT_EQ("/data/misc_de/0/rollback",
-            create_data_misc_de_rollback_path(nullptr, 0));
-    EXPECT_EQ("/data/misc_de/10/rollback",
-            create_data_misc_de_rollback_path(nullptr, 10));
+    EXPECT_EQ("/data/misc_de/0/rollback/43",
+            create_data_misc_de_rollback_path(nullptr, 0, 43));
+    EXPECT_EQ("/data/misc_de/10/rollback/41",
+            create_data_misc_de_rollback_path(nullptr, 10, 41));
 
-    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
-            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo", 0));
-    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
-            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo", 239));
+    EXPECT_EQ("/data/misc_ce/0/rollback/17/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, 17, "com.foo", 0));
+    EXPECT_EQ("/data/misc_ce/0/rollback/19/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, 19, "com.foo", 239));
 
-    auto rollback_ce_package_path = create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo");
-    auto deleter = [&rollback_ce_package_path]() {
-        delete_dir_contents_and_dir(rollback_ce_package_path, true /* ignore_if_missing */);
+    auto rollback_ce_path = create_data_misc_ce_rollback_path(nullptr, 0, 53);
+    auto rollback_ce_package_path = create_data_misc_ce_rollback_package_path(nullptr, 0, 53,
+            "com.foo");
+    auto deleter = [&rollback_ce_path]() {
+        delete_dir_contents_and_dir(rollback_ce_path, true /* ignore_if_missing */);
     };
     auto scope_guard = android::base::make_scope_guard(deleter);
 
-    ASSERT_NE(-1, mkdir(rollback_ce_package_path.c_str(), 700));
+    EXPECT_NE(-1, mkdir(rollback_ce_path.c_str(), 700));
+    EXPECT_NE(-1, mkdir(rollback_ce_package_path.c_str(), 700));
 
     ino_t ce_data_inode;
-    ASSERT_EQ(0, get_path_inode(rollback_ce_package_path, &ce_data_inode));
+    EXPECT_EQ(0, get_path_inode(rollback_ce_package_path, &ce_data_inode));
 
-    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
-            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.foo", ce_data_inode));
+    EXPECT_EQ("/data/misc_ce/0/rollback/53/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, 53, "com.foo", ce_data_inode));
     // Check that path defined by inode is picked even if it's not the same as
     // the fallback one.
-    EXPECT_EQ("/data/misc_ce/0/rollback/com.foo",
-            create_data_misc_ce_rollback_package_path(nullptr, 0, "com.bar", ce_data_inode));
+    EXPECT_EQ("/data/misc_ce/0/rollback/53/com.foo",
+            create_data_misc_ce_rollback_package_path(nullptr, 0, 53, "com.bar", ce_data_inode));
 
     // These last couple of cases are never exercised in production because we
     // only snapshot apps in the primary data partition. Exercise them here for
     // the sake of completeness.
-    EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b/misc_ce/0/rollback/com.example",
-            create_data_misc_ce_rollback_package_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b", 0, "com.example"));
-    EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b/misc_de/0/rollback/com.example",
-            create_data_misc_de_rollback_package_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b", 0, "com.example"));
+    EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b/misc_ce/0/rollback/7/com.example",
+            create_data_misc_ce_rollback_package_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b", 0, 7,
+                    "com.example"));
+    EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b/misc_de/0/rollback/11/com.example",
+            create_data_misc_de_rollback_package_path("57f8f4bc-abf4-655f-bf67-946fc0f9f25b", 0, 11,
+                    "com.example"));
+}
+
+TEST_F(UtilsTest, TestCreateDirIfNeeded) {
+    system("mkdir -p /data/local/tmp/user/0");
+
+    auto deleter = [&]() {
+        delete_dir_contents_and_dir("/data/local/tmp/user/0", true /* ignore_if_missing */);
+    };
+    auto scope_guard = android::base::make_scope_guard(deleter);
+
+    // Create folder and check it's permissions.
+    ASSERT_EQ(0, create_dir_if_needed("/data/local/tmp/user/0/foo", 0700));
+    struct stat st;
+    ASSERT_EQ(0, stat("/data/local/tmp/user/0/foo", &st));
+    ASSERT_EQ(0700, st.st_mode & ALLPERMS);
+
+    // Check that create_dir_if_needed is no-op if folder already exists with
+    // correct permissions.
+    ASSERT_EQ(0, create_dir_if_needed("/data/local/tmp/user/0/foo", 0700));
+
+    // Check -1 is returned if folder exists but with different permissions.
+    ASSERT_EQ(-1, create_dir_if_needed("/data/local/tmp/user/0/foo", 0750));
+
+    // Check that call fails if parent doesn't exist.
+    ASSERT_NE(0, create_dir_if_needed("/data/local/tmp/user/0/bar/baz", 0700));
 }
 
 }  // namespace installd
diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp
index 5b487bb..52ca0df 100644
--- a/cmds/installd/utils.cpp
+++ b/cmds/installd/utils.cpp
@@ -197,32 +197,44 @@
     return StringPrintf("%s/user_de/%u", data.c_str(), userid);
 }
 
-
-std::string create_data_misc_ce_rollback_path(const char* volume_uuid, userid_t user) {
+std::string create_data_misc_ce_rollback_base_path(const char* volume_uuid, userid_t user) {
     return StringPrintf("%s/misc_ce/%u/rollback", create_data_path(volume_uuid).c_str(), user);
 }
 
-std::string create_data_misc_de_rollback_path(const char* volume_uuid, userid_t user) {
+std::string create_data_misc_de_rollback_base_path(const char* volume_uuid, userid_t user) {
     return StringPrintf("%s/misc_de/%u/rollback", create_data_path(volume_uuid).c_str(), user);
 }
 
-std::string create_data_misc_ce_rollback_package_path(const char* volume_uuid,
-        userid_t user, const char* package_name) {
-    return StringPrintf("%s/%s",
-           create_data_misc_ce_rollback_path(volume_uuid, user).c_str(), package_name);
+std::string create_data_misc_ce_rollback_path(const char* volume_uuid, userid_t user,
+        int32_t snapshot_id) {
+    return StringPrintf("%s/%d", create_data_misc_ce_rollback_base_path(volume_uuid, user).c_str(),
+          snapshot_id);
+}
+
+std::string create_data_misc_de_rollback_path(const char* volume_uuid, userid_t user,
+        int32_t snapshot_id) {
+    return StringPrintf("%s/%d", create_data_misc_de_rollback_base_path(volume_uuid, user).c_str(),
+          snapshot_id);
 }
 
 std::string create_data_misc_ce_rollback_package_path(const char* volume_uuid,
-        userid_t user, const char* package_name, ino_t ce_rollback_inode) {
-    auto fallback = create_data_misc_ce_rollback_package_path(volume_uuid, user, package_name);
-    auto user_path = create_data_misc_ce_rollback_path(volume_uuid, user);
+        userid_t user, int32_t snapshot_id, const char* package_name) {
+    return StringPrintf("%s/%s",
+           create_data_misc_ce_rollback_path(volume_uuid, user, snapshot_id).c_str(), package_name);
+}
+
+std::string create_data_misc_ce_rollback_package_path(const char* volume_uuid,
+        userid_t user, int32_t snapshot_id, const char* package_name, ino_t ce_rollback_inode) {
+    auto fallback = create_data_misc_ce_rollback_package_path(volume_uuid, user, snapshot_id,
+            package_name);
+    auto user_path = create_data_misc_ce_rollback_path(volume_uuid, user, snapshot_id);
     return resolve_ce_path_by_inode_or_fallback(user_path, ce_rollback_inode, fallback);
 }
 
 std::string create_data_misc_de_rollback_package_path(const char* volume_uuid,
-        userid_t user, const char* package_name) {
+        userid_t user, int32_t snapshot_id, const char* package_name) {
     return StringPrintf("%s/%s",
-           create_data_misc_de_rollback_path(volume_uuid, user).c_str(), package_name);
+           create_data_misc_de_rollback_path(volume_uuid, user, snapshot_id).c_str(), package_name);
 }
 
 /**
@@ -528,6 +540,30 @@
     return result;
 }
 
+int create_dir_if_needed(const std::string& pathname, mode_t perms) {
+    struct stat st;
+
+    int rc;
+    if ((rc = stat(pathname.c_str(), &st)) != 0) {
+        if (errno == ENOENT) {
+            return mkdir(pathname.c_str(), perms);
+        } else {
+            return rc;
+        }
+    } else if (!S_ISDIR(st.st_mode)) {
+        LOG(DEBUG) << pathname << " is not a folder";
+        return -1;
+    }
+
+    mode_t actual_perms = st.st_mode & ALLPERMS;
+    if (actual_perms != perms) {
+        LOG(WARNING) << pathname << " permissions " << actual_perms << " expected " << perms;
+        return -1;
+    }
+
+    return 0;
+}
+
 int delete_dir_contents(const std::string& pathname, bool ignore_if_missing) {
     return delete_dir_contents(pathname.c_str(), 0, nullptr, ignore_if_missing);
 }
diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h
index 5d3a97d..91d9e13 100644
--- a/cmds/installd/utils.h
+++ b/cmds/installd/utils.h
@@ -61,14 +61,18 @@
 std::string create_data_user_ce_package_path_as_user_link(
         const char* volume_uuid, userid_t userid, const char* package_name);
 
-std::string create_data_misc_ce_rollback_path(const char* volume_uuid, userid_t user);
-std::string create_data_misc_de_rollback_path(const char* volume_uuid, userid_t user);
+std::string create_data_misc_ce_rollback_base_path(const char* volume_uuid, userid_t user);
+std::string create_data_misc_de_rollback_base_path(const char* volume_uuid, userid_t user);
+std::string create_data_misc_ce_rollback_path(const char* volume_uuid, userid_t user,
+        int32_t snapshot_id);
+std::string create_data_misc_de_rollback_path(const char* volume_uuid, userid_t user,
+        int32_t snapshot_id);
 std::string create_data_misc_ce_rollback_package_path(const char* volume_uuid,
-        userid_t user, const char* package_name);
+        userid_t user, int32_t snapshot_id, const char* package_name);
 std::string create_data_misc_ce_rollback_package_path(const char* volume_uuid,
-        userid_t user, const char* package_name, ino_t ce_rollback_inode);
+        userid_t user, int32_t snapshot_id, const char* package_name, ino_t ce_rollback_inode);
 std::string create_data_misc_de_rollback_package_path(const char* volume_uuid,
-        userid_t user, const char* package_name);
+        userid_t user, int32_t snapshot_id, const char* package_name);
 
 std::string create_data_media_path(const char* volume_uuid, userid_t userid);
 std::string create_data_media_obb_path(const char* volume_uuid, const char* package_name);
@@ -109,6 +113,8 @@
 bool is_valid_filename(const std::string& name);
 bool is_valid_package_name(const std::string& packageName);
 
+int create_dir_if_needed(const std::string& pathname, mode_t mode);
+
 int delete_dir_contents(const std::string& pathname, bool ignore_if_missing = false);
 int delete_dir_contents_and_dir(const std::string& pathname, bool ignore_if_missing = false);
 
diff --git a/libs/binder/ndk/include_apex/android/binder_manager.h b/libs/binder/ndk/include_apex/android/binder_manager.h
index 80b6c07..055c79b 100644
--- a/libs/binder/ndk/include_apex/android/binder_manager.h
+++ b/libs/binder/ndk/include_apex/android/binder_manager.h
@@ -33,6 +33,15 @@
 binder_status_t AServiceManager_addService(AIBinder* binder, const char* instance);
 
 /**
+ * Gets a binder object with this specific instance name. Will return nullptr immediately if the
+ * service is not available This also implicitly calls AIBinder_incStrong (so the caller of this
+ * function is responsible for calling AIBinder_decStrong).
+ *
+ * \param instance identifier of the service used to lookup the service.
+ */
+__attribute__((warn_unused_result)) AIBinder* AServiceManager_checkService(const char* instance);
+
+/**
  * Gets a binder object with this specific instance name. Blocks for a couple of seconds waiting on
  * it. This also implicitly calls AIBinder_incStrong (so the caller of this function is responsible
  * for calling AIBinder_decStrong).
diff --git a/libs/binder/ndk/libbinder_ndk.map.txt b/libs/binder/ndk/libbinder_ndk.map.txt
index f0d25f7..655f4d5 100644
--- a/libs/binder/ndk/libbinder_ndk.map.txt
+++ b/libs/binder/ndk/libbinder_ndk.map.txt
@@ -91,6 +91,7 @@
     ABinderProcess_setThreadPoolMaxThreadCount; # apex
     ABinderProcess_startThreadPool; # apex
     AServiceManager_addService; # apex
+    AServiceManager_checkService; # apex
     AServiceManager_getService; # apex
   local:
     *;
diff --git a/libs/binder/ndk/service_manager.cpp b/libs/binder/ndk/service_manager.cpp
index 9ddc555..d0b166d 100644
--- a/libs/binder/ndk/service_manager.cpp
+++ b/libs/binder/ndk/service_manager.cpp
@@ -37,6 +37,18 @@
     status_t status = sm->addService(String16(instance), binder->getBinder());
     return PruneStatusT(status);
 }
+AIBinder* AServiceManager_checkService(const char* instance) {
+    if (instance == nullptr) {
+        return nullptr;
+    }
+
+    sp<IServiceManager> sm = defaultServiceManager();
+    sp<IBinder> binder = sm->checkService(String16(instance));
+
+    sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(binder);
+    AIBinder_incStrong(ret.get());
+    return ret.get();
+}
 AIBinder* AServiceManager_getService(const char* instance) {
     if (instance == nullptr) {
         return nullptr;
diff --git a/libs/binder/ndk/test/main_client.cpp b/libs/binder/ndk/test/main_client.cpp
index c159d71..bff601e 100644
--- a/libs/binder/ndk/test/main_client.cpp
+++ b/libs/binder/ndk/test/main_client.cpp
@@ -35,6 +35,19 @@
 //     EXPECT_EQ(nullptr, foo.get());
 // }
 
+TEST(NdkBinder, CheckServiceThatDoesntExist) {
+    AIBinder* binder = AServiceManager_checkService("asdfghkl;");
+    ASSERT_EQ(nullptr, binder);
+}
+
+TEST(NdkBinder, CheckServiceThatDoesExist) {
+    AIBinder* binder = AServiceManager_checkService(kExistingNonNdkService);
+    EXPECT_NE(nullptr, binder);
+    EXPECT_EQ(STATUS_OK, AIBinder_ping(binder));
+
+    AIBinder_decStrong(binder);
+}
+
 TEST(NdkBinder, DoubleNumber) {
     sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName);
     ASSERT_NE(foo, nullptr);
diff --git a/opengl/libs/EGL/eglApi.cpp b/opengl/libs/EGL/eglApi.cpp
index e824238..36deedc 100644
--- a/opengl/libs/EGL/eglApi.cpp
+++ b/opengl/libs/EGL/eglApi.cpp
@@ -552,6 +552,15 @@
                     break;
                 }
             }
+
+            // If the driver doesn't understand it, we should map sRGB-encoded P3 to
+            // sRGB rather than just dropping the colorspace on the floor.
+            // For this format, the driver is expected to apply the sRGB
+            // transfer function during framebuffer operations.
+            if (!copyAttribute && attr[1] == EGL_GL_COLORSPACE_DISPLAY_P3_EXT) {
+                strippedAttribList->push_back(attr[0]);
+                strippedAttribList->push_back(EGL_GL_COLORSPACE_SRGB_KHR);
+            }
         }
         if (copyAttribute) {
             strippedAttribList->push_back(attr[0]);
diff --git a/services/surfaceflinger/ContainerLayer.cpp b/services/surfaceflinger/ContainerLayer.cpp
index 6f5bd0e..f259d93 100644
--- a/services/surfaceflinger/ContainerLayer.cpp
+++ b/services/surfaceflinger/ContainerLayer.cpp
@@ -30,6 +30,10 @@
 
 void ContainerLayer::onDraw(const RenderArea&, const Region& /* clip */, bool) const {}
 
+bool ContainerLayer::isVisible() const {
+    return !isHiddenByPolicy();
+}
+
 void ContainerLayer::setPerFrameData(const sp<const DisplayDevice>&) {}
 
 } // namespace android
diff --git a/services/surfaceflinger/ContainerLayer.h b/services/surfaceflinger/ContainerLayer.h
index 38b1882..b352b96 100644
--- a/services/surfaceflinger/ContainerLayer.h
+++ b/services/surfaceflinger/ContainerLayer.h
@@ -32,7 +32,7 @@
     const char* getTypeId() const override { return "ContainerLayer"; }
     void onDraw(const RenderArea& renderArea, const Region& clip,
                 bool useIdentityTransform) const override;
-    bool isVisible() const override { return false; }
+    bool isVisible() const override;
 
     void setPerFrameData(const sp<const DisplayDevice>& displayDevice) override;
 
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 286a133..4ec1e7f 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4427,6 +4427,12 @@
     result.append("\n");
 
     /*
+     * Tracing state
+     */
+    mTracing.dump(result);
+    result.append("\n");
+
+    /*
      * HWC layer minidump
      */
     for (size_t d = 0; d < mDisplays.size(); d++) {
@@ -4755,12 +4761,12 @@
             case 1025: { // Set layer tracing
                 n = data.readInt32();
                 if (n) {
-                    ALOGV("LayerTracing enabled");
+                    ALOGD("LayerTracing enabled");
                     mTracing.enable();
                     doTracing("tracing.enable");
                     reply->writeInt32(NO_ERROR);
                 } else {
-                    ALOGV("LayerTracing disabled");
+                    ALOGD("LayerTracing disabled");
                     status_t err = mTracing.disable();
                     reply->writeInt32(err);
                 }
diff --git a/services/surfaceflinger/SurfaceTracing.cpp b/services/surfaceflinger/SurfaceTracing.cpp
index f8c466e..67dcd06 100644
--- a/services/surfaceflinger/SurfaceTracing.cpp
+++ b/services/surfaceflinger/SurfaceTracing.cpp
@@ -27,52 +27,67 @@
 namespace android {
 
 void SurfaceTracing::enable() {
+    ATRACE_CALL();
+    std::lock_guard<std::mutex> protoGuard(mTraceMutex);
+
     if (mEnabled) {
         return;
     }
-    ATRACE_CALL();
     mEnabled = true;
-    std::lock_guard<std::mutex> protoGuard(mTraceMutex);
 
-    mTrace.set_magic_number(uint64_t(LayersTraceFileProto_MagicNumber_MAGIC_NUMBER_H) << 32 |
-                            LayersTraceFileProto_MagicNumber_MAGIC_NUMBER_L);
+    mTrace = std::make_unique<LayersTraceFileProto>();
+    mTrace->set_magic_number(uint64_t(LayersTraceFileProto_MagicNumber_MAGIC_NUMBER_H) << 32 |
+                             LayersTraceFileProto_MagicNumber_MAGIC_NUMBER_L);
 }
 
 status_t SurfaceTracing::disable() {
+    ATRACE_CALL();
+    std::lock_guard<std::mutex> protoGuard(mTraceMutex);
+
     if (!mEnabled) {
         return NO_ERROR;
     }
-    ATRACE_CALL();
-    std::lock_guard<std::mutex> protoGuard(mTraceMutex);
     mEnabled = false;
     status_t err(writeProtoFileLocked());
     ALOGE_IF(err == PERMISSION_DENIED, "Could not save the proto file! Permission denied");
     ALOGE_IF(err == NOT_ENOUGH_DATA, "Could not save the proto file! There are missing fields");
-    mTrace.Clear();
+    mTrace.reset();
     return err;
 }
 
-bool SurfaceTracing::isEnabled() {
+bool SurfaceTracing::isEnabled() const {
+    std::lock_guard<std::mutex> protoGuard(mTraceMutex);
     return mEnabled;
 }
 
 void SurfaceTracing::traceLayers(const char* where, LayersProto layers) {
     std::lock_guard<std::mutex> protoGuard(mTraceMutex);
-
-    LayersTraceProto* entry = mTrace.add_entry();
+    if (!mEnabled) {
+        return;
+    }
+    LayersTraceProto* entry = mTrace->add_entry();
     entry->set_elapsed_realtime_nanos(elapsedRealtimeNano());
     entry->set_where(where);
     entry->mutable_layers()->Swap(&layers);
+
+    constexpr int maxBufferedEntryCount = 3600;
+    if (mTrace->entry_size() >= maxBufferedEntryCount) {
+        // TODO: flush buffered entries without disabling tracing
+        ALOGE("too many buffered frames; force disable tracing");
+        mEnabled = false;
+        writeProtoFileLocked();
+        mTrace.reset();
+    }
 }
 
 status_t SurfaceTracing::writeProtoFileLocked() {
     ATRACE_CALL();
 
-    if (!mTrace.IsInitialized()) {
+    if (!mTrace->IsInitialized()) {
         return NOT_ENOUGH_DATA;
     }
     std::string output;
-    if (!mTrace.SerializeToString(&output)) {
+    if (!mTrace->SerializeToString(&output)) {
         return PERMISSION_DENIED;
     }
     if (!android::base::WriteStringToFile(output, mOutputFileName, true)) {
@@ -82,4 +97,11 @@
     return NO_ERROR;
 }
 
+void SurfaceTracing::dump(String8& result) const {
+    std::lock_guard<std::mutex> protoGuard(mTraceMutex);
+
+    result.appendFormat("Tracing state: %s\n", mEnabled ? "enabled" : "disabled");
+    result.appendFormat("  number of entries: %d\n", mTrace ? mTrace->entry_size() : 0);
+}
+
 } // namespace android
diff --git a/services/surfaceflinger/SurfaceTracing.h b/services/surfaceflinger/SurfaceTracing.h
index 590ab96..fd8cb82 100644
--- a/services/surfaceflinger/SurfaceTracing.h
+++ b/services/surfaceflinger/SurfaceTracing.h
@@ -18,7 +18,9 @@
 
 #include <layerproto/LayerProtoHeader.h>
 #include <utils/Errors.h>
+#include <utils/String8.h>
 
+#include <memory>
 #include <mutex>
 
 using namespace android::surfaceflinger;
@@ -32,9 +34,10 @@
 public:
     void enable();
     status_t disable();
-    bool isEnabled();
+    bool isEnabled() const;
 
     void traceLayers(const char* where, LayersProto);
+    void dump(String8& result) const;
 
 private:
     static constexpr auto DEFAULT_FILENAME = "/data/misc/wmtrace/layers_trace.pb";
@@ -43,8 +46,8 @@
 
     bool mEnabled = false;
     std::string mOutputFileName = DEFAULT_FILENAME;
-    std::mutex mTraceMutex;
-    LayersTraceFileProto mTrace;
+    mutable std::mutex mTraceMutex;
+    std::unique_ptr<LayersTraceFileProto> mTrace;
 };
 
 } // namespace android