BootParameters: Use proto for disk io.

Use proto for disk usage.

Bug: 79932027
Test: Ran through the following scenarios-
- RebootActivity parses previous boot parameters; sets next as expected.
- Push old parameters to device and ensured bootanimations reads it and
saves new format (serialized proto) that can be read in userspace
(RebootActivity).
- Unit tests pass.

Change-Id: I98aefb7a832c985d5061ee80ab85a4523f067641

diff --git a/cmds/bootanimation/Android.mk b/cmds/bootanimation/Android.mk
index 453fb72..6943dab 100644 (file)
--- a/cmds/bootanimation/Android.mk
+++ b/cmds/bootanimation/Android.mk
@@ -27,7 +27,9 @@ ifeq ($(PRODUCT_IOT),true)
 
 LOCAL_SHARED_LIBRARIES += \
     libandroidthings \
+    libandroidthings_protos \
     libchrome \
+    libprotobuf-cpp-lite \
 
 LOCAL_STATIC_LIBRARIES += \
     libjsoncpp

diff --git a/cmds/bootanimation/iot/Android.mk b/cmds/bootanimation/iot/Android.mk
index 8b475d3..3d288e4 100644 (file)
--- a/cmds/bootanimation/iot/Android.mk
+++ b/cmds/bootanimation/iot/Android.mk
@@ -25,9 +25,11 @@ LOCAL_CFLAGS := -Wall -Werror -Wunused -Wunreachable-code
 
 LOCAL_SHARED_LIBRARIES := \
     libandroidthings \
+    libandroidthings_protos \
     libbase \
     libchrome \
     liblog \
+    libprotobuf-cpp-lite \
 
 LOCAL_STATIC_LIBRARIES += \
     libjsoncpp

diff --git a/cmds/bootanimation/iot/BootParameters.cpp b/cmds/bootanimation/iot/BootParameters.cpp
index 2cf1c19..30a9b28 100644 (file)
--- a/cmds/bootanimation/iot/BootParameters.cpp
+++ b/cmds/bootanimation/iot/BootParameters.cpp
@@ -22,10 +22,13 @@
 #include <fcntl.h>
 
 #include <android-base/file.h>
+#include <json/json.h>
 #include <utils/Log.h>
 
 using android::base::ReadFileToString;
 using android::base::RemoveFileIfExists;
+using android::base::WriteStringToFile;
+using Json::ArrayIndex;
 using Json::Reader;
 using Json::Value;
 
@@ -33,23 +36,34 @@ namespace android {
 
 namespace {
 
-// Keys for supporting a silent boot and user-defined BootAction parameters.
-constexpr const char *kKeySilentBoot = "silent_boot";
-constexpr const char* kKeyParams = "params";
+// Keys for deprecated parameters. Devices that OTA from N to O and that used
+// the hidden BootParameters API will store these in the JSON blob. To support
+// the transition from N to O, these keys are mapped to the new parameters.
+constexpr const char *kKeyLegacyVolume = "volume";
+constexpr const char *kKeyLegacyAnimationsDisabled = "boot_animation_disabled";
+constexpr const char *kKeyLegacyParamNames = "param_names";
+constexpr const char *kKeyLegacyParamValues = "param_values";
 
-constexpr const char* kNextBootFile = "/data/misc/bootanimation/next_boot.json";
-constexpr const char* kLastBootFile = "/data/misc/bootanimation/last_boot.json";
+constexpr const char *kNextBootFile = "/data/misc/bootanimation/next_boot.proto";
+constexpr const char *kLastBootFile = "/data/misc/bootanimation/last_boot.proto";
 
-void swapBootConfigs() {
-    // rename() will fail if next_boot.json doesn't exist, so delete
-    // last_boot.json manually first.
+constexpr const char *kLegacyNextBootFile = "/data/misc/bootanimation/next_boot.json";
+constexpr const char *kLegacyLastBootFile = "/data/misc/bootanimation/last_boot.json";
+
+void removeLegacyFiles() {
     std::string err;
-    if (!RemoveFileIfExists(kLastBootFile, &err))
-        ALOGE("Unable to delete last boot file: %s", err.c_str());
+    if (!RemoveFileIfExists(kLegacyLastBootFile, &err)) {
+        ALOGW("Unable to delete %s: %s", kLegacyLastBootFile, err.c_str());
+    }
 
-    if (rename(kNextBootFile, kLastBootFile) && errno != ENOENT)
-        ALOGE("Unable to swap boot files: %s", strerror(errno));
+    err.clear();
+    if (!RemoveFileIfExists(kLegacyNextBootFile, &err)) {
+        ALOGW("Unable to delete %s: %s", kLegacyNextBootFile, err.c_str());
+    }
+}
 
+void createNextBootFile() {
+    errno = 0;
     int fd = open(kNextBootFile, O_CREAT, DEFFILEMODE);
     if (fd == -1) {
         ALOGE("Unable to create next boot file: %s", strerror(errno));
@@ -64,54 +78,120 @@ void swapBootConfigs() {
 
 }  // namespace
 
+// Renames the 'next' boot file to the 'last' file and reads its contents.
+bool BootParameters::swapAndLoadBootConfigContents(const char *lastBootFile,
+                                                   const char *nextBootFile,
+                                                   std::string *contents) {
+    if (!ReadFileToString(nextBootFile, contents)) {
+        RemoveFileIfExists(lastBootFile);
+        return false;
+    }
+
+    errno = 0;
+    if (rename(nextBootFile, lastBootFile) && errno != ENOENT)
+        ALOGE("Unable to swap boot files: %s", strerror(errno));
+
+    return true;
+}
+
 BootParameters::BootParameters() {
-    swapBootConfigs();
     loadParameters();
 }
 
+// Saves the boot parameters state to disk so the framework can read it.
+void BootParameters::storeParameters() {
+    errno = 0;
+    if (!WriteStringToFile(mProto.SerializeAsString(), kLastBootFile)) {
+        ALOGE("Failed to write boot parameters to %s: %s", kLastBootFile, strerror(errno));
+    }
+
+    // WriteStringToFile sets the file permissions to 0666, but these are not
+    // honored by the system.
+    errno = 0;
+    if (chmod(kLastBootFile, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)) {
+        ALOGE("Failed to set permissions for %s: %s", kLastBootFile, strerror(errno));
+    }
+}
+
+// Load the boot parameters from disk, try the old location and format if the
+// file does not exist. Note:
+// - Parse errors result in defaults being used (a normal boot).
+// - Legacy boot parameters default to a silent boot.
 void BootParameters::loadParameters() {
+    // Precedence is given to the new file format (.proto).
     std::string contents;
-    if (!ReadFileToString(kLastBootFile, &contents)) {
-        if (errno != ENOENT)
-            ALOGE("Unable to read from %s: %s", kLastBootFile, strerror(errno));
+    if (swapAndLoadBootConfigContents(kLastBootFile, kNextBootFile, &contents)) {
+        parseBootParameters(contents);
+    } else if (swapAndLoadBootConfigContents(kLegacyLastBootFile, kLegacyNextBootFile, &contents)) {
+        parseLegacyBootParameters(contents);
+        storeParameters();
+        removeLegacyFiles();
+    }
+
+    createNextBootFile();
+}
 
+void BootParameters::parseBootParameters(const std::string &contents) {
+    if (!mProto.ParseFromString(contents)) {
+        ALOGW("Failed to parse parameters from %s", kLastBootFile);
         return;
     }
 
-    loadParameters(contents);
+    loadStateFromProto();
 }
 
-// If the boot parameters -
-// - File is missing, we assume a normal, non-silent boot.
-// - Are well-formed, initially assume a normal, non-silent boot and parse.
-void BootParameters::loadParameters(const std::string& raw_json) {
-  if (!Reader().parse(raw_json, mJson)) {
-    return;
-  }
+// Parses the JSON in the proto.
+void BootParameters::parseLegacyBootParameters(const std::string &contents) {
+    Value json;
+    if (!Reader().parse(contents, json)) {
+        ALOGW("Failed to parse parameters from %s", kLegacyLastBootFile);
+        return;
+    }
 
-  parseBootParameters();
-}
+    int volume = 0;
+    bool bootAnimationDisabled = true;
 
-void BootParameters::parseBootParameters() {
-    // A missing key returns a safe, missing value.
-    // Ignore invalid or missing JSON parameters.
-    Value &jsonValue = mJson[kKeySilentBoot];
-    if (jsonValue.isBool()) {
-        mIsSilentBoot = jsonValue.asBool();
+    Value &jsonValue = json[kKeyLegacyVolume];
+    if (jsonValue.isIntegral()) {
+        volume = jsonValue.asInt();
     }
 
-    jsonValue = mJson[kKeyParams];
-    if (jsonValue.isObject()) {
-        // getMemberNames returns a copy of the keys which must be stored.
-        mKeys = jsonValue.getMemberNames();
-        for (auto &key : mKeys) {
-            Value &value = jsonValue[key];
-            if (value.isString()) {
-                mParameters.push_back(
-                    {.key = key.c_str(), .value = value.asCString()});
+    jsonValue = json[kKeyLegacyAnimationsDisabled];
+    if (jsonValue.isIntegral()) {
+        bootAnimationDisabled = jsonValue.asInt() == 1;
+    }
+
+    // Assume a silent boot unless all of the following are true -
+    // 1. The volume is neither 0 nor -1000 (the legacy default value).
+    // 2. The boot animations are explicitly enabled.
+    // Note: brightness was never used.
+    mProto.set_silent_boot((volume == 0) || (volume == -1000) || bootAnimationDisabled);
+
+    Value &keys = json[kKeyLegacyParamNames];
+    Value &values = json[kKeyLegacyParamValues];
+    if (keys.isArray() && values.isArray() && (keys.size() == values.size())) {
+        for (ArrayIndex i = 0; i < keys.size(); ++i) {
+            auto &key = keys[i];
+            auto &value = values[i];
+            if (key.isString() && value.isString()) {
+                auto userParameter = mProto.add_user_parameter();
+                userParameter->set_key(key.asString());
+                userParameter->set_value(value.asString());
             }
         }
     }
+
+    loadStateFromProto();
+}
+
+void BootParameters::loadStateFromProto() {
+    // A missing key returns a safe, default value.
+    // Ignore invalid or missing parameters.
+    mIsSilentBoot = mProto.silent_boot();
+
+    for (const auto &param : mProto.user_parameter()) {
+        mParameters.push_back({.key = param.key().c_str(), .value = param.value().c_str()});
+    }
 }
 
 }  // namespace android

diff --git a/cmds/bootanimation/iot/BootParameters.h b/cmds/bootanimation/iot/BootParameters.h
index 49c34dd..cbd1ca6 100644 (file)
--- a/cmds/bootanimation/iot/BootParameters.h
+++ b/cmds/bootanimation/iot/BootParameters.h
@@ -22,7 +22,7 @@
 #include <vector>
 
 #include <boot_action/boot_action.h>  // libandroidthings native API.
-#include <json/json.h>
+#include <boot_parameters.pb.h>
 
 namespace android {
 
@@ -39,22 +39,33 @@ public:
     // Returns the additional boot parameters that were set on reboot.
     const std::vector<ABootActionParameter>& getParameters() const { return mParameters; }
 
-    // Exposed for testing. Updates the parameters with new JSON values.
-    void loadParameters(const std::string& raw_json);
-private:
+    // Exposed for testing. Sets the parameters to the serialized proto.
+    void parseBootParameters(const std::string &contents);
+
+    // For devices that OTA from N to O.
+    // Exposed for testing. Sets the parameters to the raw JSON.
+    void parseLegacyBootParameters(const std::string &contents);
+
+    // Exposed for testing. Loads the contents from |nextBootFile| and replaces
+    // |lastBootFile| with |nextBootFile|.
+    static bool swapAndLoadBootConfigContents(const char *lastBootFile, const char *nextBootFile,
+                                              std::string *contents);
+
+  private:
     void loadParameters();
 
-    void parseBootParameters();
+    // Replaces the legacy JSON blob with the updated version, allowing the
+    // framework to read it.
+    void storeParameters();
+
+    void loadStateFromProto();
 
     bool mIsSilentBoot = false;
 
     std::vector<ABootActionParameter> mParameters;
 
-    // Store parsed JSON because mParameters makes a shallow copy.
-    Json::Value mJson;
-
-    // Store parameter keys because mParameters makes a shallow copy.
-    Json::Value::Members mKeys;
+    // Store the proto because mParameters makes a shallow copy.
+    android::things::proto::BootParameters mProto;
 };
 
 }  // namespace android

diff --git a/cmds/bootanimation/iot/BootParameters_test.cpp b/cmds/bootanimation/iot/BootParameters_test.cpp
index 27d7d6f..d55bce6 100644 (file)
--- a/cmds/bootanimation/iot/BootParameters_test.cpp
+++ b/cmds/bootanimation/iot/BootParameters_test.cpp
@@ -16,6 +16,13 @@
 
 #include "BootParameters.h"
 
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <android-base/file.h>
+#include <android-base/test_utils.h>
+#include <boot_parameters.pb.h>
 #include <gtest/gtest.h>
 
 namespace android {
@@ -23,50 +30,131 @@ namespace android {
 namespace {
 
 TEST(BootParametersTest, TestNoBootParametersIsNotSilent) {
-    BootParameters boot_parameters = BootParameters();
-    boot_parameters.loadParameters("");
+    android::things::proto::BootParameters proto;
+
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseBootParameters(proto.SerializeAsString());
 
-    ASSERT_FALSE(boot_parameters.isSilentBoot());
-    ASSERT_EQ(0u, boot_parameters.getParameters().size());
+    ASSERT_FALSE(bootParameters.isSilentBoot());
+    ASSERT_EQ(0u, bootParameters.getParameters().size());
 }
 
 TEST(BootParametersTest, TestParseIsSilent) {
-    BootParameters boot_parameters = BootParameters();
-    boot_parameters.loadParameters(R"(
+    android::things::proto::BootParameters proto;
+    proto.set_silent_boot(true);
+
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseBootParameters(proto.SerializeAsString());
+
+    ASSERT_TRUE(bootParameters.isSilentBoot());
+}
+
+TEST(BootParametersTest, TestParseIsNotSilent) {
+    android::things::proto::BootParameters proto;
+    proto.set_silent_boot(false);
+
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseBootParameters(proto.SerializeAsString());
+
+    ASSERT_FALSE(bootParameters.isSilentBoot());
+}
+
+TEST(BootParametersTest, TestParseBootParameters) {
+    android::things::proto::BootParameters proto;
+    proto.set_silent_boot(false);
+
+    auto userParameter = proto.add_user_parameter();
+    userParameter->set_key("key1");
+    userParameter->set_value("value1");
+
+    userParameter = proto.add_user_parameter();
+    userParameter->set_key("key2");
+    userParameter->set_value("value2");
+
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseBootParameters(proto.SerializeAsString());
+
+    auto &parameters = bootParameters.getParameters();
+    ASSERT_EQ(2u, parameters.size());
+    ASSERT_STREQ(parameters[0].key, "key1");
+    ASSERT_STREQ(parameters[0].value, "value1");
+    ASSERT_STREQ(parameters[1].key, "key2");
+    ASSERT_STREQ(parameters[1].value, "value2");
+}
+
+TEST(BootParametersTest, TestParseLegacyDisableBootAnimationIsSilent) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
     {
-      "silent_boot":true,
-      "params":{}
+      "brightness":200,
+      "volume":100,
+      "boot_animation_disabled":1,
+      "param_names":[],
+      "param_values":[]
     }
     )");
 
-    ASSERT_TRUE(boot_parameters.isSilentBoot());
+    ASSERT_TRUE(bootParameters.isSilentBoot());
 }
 
-TEST(BootParametersTest, TestParseIsNotSilent) {
-    BootParameters boot_parameters = BootParameters();
-    boot_parameters.loadParameters(R"(
+TEST(BootParametersTest, TestParseLegacyZeroVolumeIsSilent) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
     {
-      "silent_boot":false,
-      "params":{}
+      "brightness":200,
+      "volume":0,
+      "boot_animation_disabled":0,
+      "param_names":[],
+      "param_values":[]
     }
     )");
 
-    ASSERT_FALSE(boot_parameters.isSilentBoot());
+    ASSERT_TRUE(bootParameters.isSilentBoot());
 }
 
-TEST(BootParametersTest, TestParseBootParameters) {
-    BootParameters boot_parameters = BootParameters();
-    boot_parameters.loadParameters(R"(
+TEST(BootParametersTest, TestParseLegacyDefaultVolumeIsSilent) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
     {
-      "silent_boot":false,
-      "params":{
-        "key1":"value1",
-        "key2":"value2"
-      }
+      "brightness":200,
+      "volume":-1000,
+      "boot_animation_disabled":0,
+      "param_names":[],
+      "param_values":[]
     }
     )");
 
-    auto &parameters = boot_parameters.getParameters();
+    ASSERT_TRUE(bootParameters.isSilentBoot());
+}
+
+TEST(BootParametersTest, TestParseLegacyNotSilent) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
+    {
+      "brightness":200,
+      "volume":500,
+      "boot_animation_disabled":0,
+      "param_names":[],
+      "param_values":[]
+    }
+    )");
+
+    ASSERT_FALSE(bootParameters.isSilentBoot());
+}
+
+TEST(BootParametersTest, TestParseLegacyParameters) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
+    {
+      "brightness":200,
+      "volume":100,
+      "boot_animation_disabled":1,
+      "param_names":["key1", "key2"],
+      "param_values":["value1", "value2"]
+    }
+    )");
+
+    auto parameters = bootParameters.getParameters();
     ASSERT_EQ(2u, parameters.size());
     ASSERT_STREQ(parameters[0].key, "key1");
     ASSERT_STREQ(parameters[0].value, "value1");
@@ -74,35 +162,102 @@ TEST(BootParametersTest, TestParseBootParameters) {
     ASSERT_STREQ(parameters[1].value, "value2");
 }
 
-TEST(BootParametersTest, TestParseMissingParametersIsNotSilent) {
-    BootParameters boot_parameters = BootParameters();
-    boot_parameters.loadParameters(R"(
+TEST(BootParametersTest, TestParseLegacyZeroParameters) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
     {
-      "params":{}
+      "brightness":200,
+      "volume":100,
+      "boot_animation_disabled":1,
+      "param_names":[],
+      "param_values":[]
     }
     )");
 
-    ASSERT_FALSE(boot_parameters.isSilentBoot());
+    ASSERT_EQ(0u, bootParameters.getParameters().size());
 }
 
-TEST(BootParametersTest, TestParseMalformedParametersAreSkipped) {
-    BootParameters boot_parameters = BootParameters();
-    boot_parameters.loadParameters(R"(
+TEST(BootParametersTest, TestMalformedLegacyParametersAreSkipped) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
     {
-      "silent_boot":false,
-      "params":{
-        "key1":123,
-        "key2":"value2"
-      }
+      "brightness":500,
+      "volume":500,
+      "boot_animation_disabled":0,
+      "param_names":["key1", "key2"],
+      "param_values":[1, "value2"]
     }
     )");
 
-    auto &parameters = boot_parameters.getParameters();
+    auto parameters = bootParameters.getParameters();
     ASSERT_EQ(1u, parameters.size());
     ASSERT_STREQ(parameters[0].key, "key2");
     ASSERT_STREQ(parameters[0].value, "value2");
 }
 
+TEST(BootParametersTest, TestLegacyUnequalParameterSizesAreSkipped) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
+    {
+      "brightness":500,
+      "volume":500,
+      "boot_animation_disabled":0,
+      "param_names":["key1", "key2"],
+      "param_values":["value1"]
+    }
+    )");
+
+    ASSERT_EQ(0u, bootParameters.getParameters().size());
+}
+
+TEST(BootParametersTest, TestMissingLegacyBootParametersIsSilent) {
+    BootParameters bootParameters = BootParameters();
+    bootParameters.parseLegacyBootParameters(R"(
+    {
+      "brightness":500
+    }
+    )");
+
+    EXPECT_TRUE(bootParameters.isSilentBoot());
+    ASSERT_EQ(0u, bootParameters.getParameters().size());
+}
+
+TEST(BootParametersTest, TestLastFileIsRemovedOnError) {
+    TemporaryFile lastFile;
+    TemporaryDir tempDir;
+    std::string nonExistentFilePath(std::string(tempDir.path) + "/nonexistent");
+    std::string contents;
+
+    BootParameters::swapAndLoadBootConfigContents(lastFile.path, nonExistentFilePath.c_str(),
+                                                  &contents);
+
+    struct stat buf;
+    ASSERT_EQ(-1, lstat(lastFile.path, &buf));
+    ASSERT_TRUE(contents.empty());
+}
+
+TEST(BootParametersTest, TestNextFileIsRemovedLastFileExistsOnSuccess) {
+    TemporaryFile lastFile;
+    TemporaryFile nextFile;
+
+    base::WriteStringToFile("foo", nextFile.path);
+
+    std::string contents;
+    // Expected side effects:
+    // - |next_file| is moved to |last_file|
+    // - |contents| is the contents of |next_file| before being moved.
+    BootParameters::swapAndLoadBootConfigContents(lastFile.path, nextFile.path, &contents);
+
+    struct stat buf;
+    ASSERT_EQ(0, lstat(lastFile.path, &buf));
+    ASSERT_EQ(-1, lstat(nextFile.path, &buf));
+    ASSERT_EQ(contents, "foo");
+
+    contents.clear();
+    ASSERT_TRUE(base::ReadFileToString(lastFile.path, &contents));
+    ASSERT_EQ(contents, "foo");
+}
+
 }  // namespace
 
 }  // namespace android