Use the new LargeParcelable API.

Bug: 210063973
Test: atest DefaultVehicleHalTest
Change-Id: Id1805638918acdf5bc79d2591d26829289ba56e8

automotive/vehicle/aidl/impl/vhal/include/ParcelableUtils.h

@@ -31,19 +31,18 @@ namespace vehicle {
 
 template <class T1, class T2>
 ::ndk::ScopedAStatus vectorToStableLargeParcelable(std::vector<T1>&& values, T2* output) {
+    output->payloads = std::move(values);
     auto result = ::android::automotive::car_binder_lib::LargeParcelableBase::
-            parcelableVectorToStableLargeParcelable(values);
+            parcelableToStableLargeParcelable(*output);
     if (!result.ok()) {
         return toScopedAStatus(
                 result, ::aidl::android::hardware::automotive::vehicle::StatusCode::INTERNAL_ERROR);
     }
     auto& fd = result.value();
-    if (fd == nullptr) {
-        // If we no longer needs values, move it inside the payloads to avoid copying.
-        output->payloads = std::move(values);
-    } else {
+    if (fd != nullptr) {
         // Move the returned ScopedFileDescriptor pointer to ScopedFileDescriptor value in
         // 'sharedMemoryFd' field.
+        output->payloads.clear();
         output->sharedMemoryFd = std::move(*fd);
     }
     return ::ndk::ScopedAStatus::ok();
@@ -57,12 +56,13 @@ template <class T1, class T2>
     return vectorToStableLargeParcelable(std::move(valuesCopy), output);
 }
 
-template <class T1, class T2>
-::android::base::expected<std::vector<T1>, ::ndk::ScopedAStatus> stableLargeParcelableToVector(
-        const T2& largeParcelable) {
-    ::android::base::Result<std::optional<std::vector<T1>>> result =
-            ::android::automotive::car_binder_lib::LargeParcelableBase::
-                    stableLargeParcelableToParcelableVector<T1>(largeParcelable.sharedMemoryFd);
+template <class T>
+::android::base::expected<
+        ::android::automotive::car_binder_lib::LargeParcelableBase::BorrowedOwnedObject<T>,
+        ::ndk::ScopedAStatus>
+fromStableLargeParcelable(const T& largeParcelable) {
+    auto result = ::android::automotive::car_binder_lib::LargeParcelableBase::
+            stableLargeParcelableToParcelable(largeParcelable);
 
     if (!result.ok()) {
         return ::android::base::unexpected(toScopedAStatus(
@@ -70,15 +70,7 @@ template <class T1, class T2>
                 "failed to parse large parcelable"));
     }
 
-    if (!result.value().has_value()) {
-        return ::android::base::unexpected(
-                ::ndk::ScopedAStatus::fromServiceSpecificErrorWithMessage(
-                        toInt(::aidl::android::hardware::automotive::vehicle::StatusCode::
-                                      INVALID_ARG),
-                        "empty request"));
-    }
-
-    return std::move(result.value().value());
+    return std::move(result.value());
 }
 
 }  // namespace vehicle

automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp

@@ -57,7 +57,9 @@ DefaultVehicleHal::DefaultVehicleHal(std::unique_ptr<IVehicleHardware> hardware)
     for (auto& config : configs) {
         mConfigsByPropId[config.prop] = config;
     }
-    auto result = LargeParcelableBase::parcelableVectorToStableLargeParcelable(configs);
+    VehiclePropConfigs vehiclePropConfigs;
+    vehiclePropConfigs.payloads = std::move(configs);
+    auto result = LargeParcelableBase::parcelableToStableLargeParcelable(vehiclePropConfigs);
     if (!result.ok()) {
         ALOGE("failed to convert configs to shared memory file, error: %s, code: %d",
               getErrorMsg(result).c_str(), getIntErrorCode(result));
@@ -71,6 +73,7 @@ DefaultVehicleHal::DefaultVehicleHal(std::unique_ptr<IVehicleHardware> hardware)
 
 ScopedAStatus DefaultVehicleHal::getAllPropConfigs(VehiclePropConfigs* output) {
     if (mConfigFile != nullptr) {
+        output->payloads.clear();
         output->sharedMemoryFd.set(dup(mConfigFile->get()));
         return ScopedAStatus::ok();
     }
@@ -131,20 +134,14 @@ ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,
                                            const GetValueRequests& requests) {
     // TODO(b/203713317): check for duplicate properties and duplicate request IDs.
 
-    const std::vector<GetValueRequest>* getValueRequests;
-    // Define deserializedResults here because we need it to have the same lifetime as
-    // getValueRequests.
-    expected<std::vector<GetValueRequest>, ScopedAStatus> deserializedResults;
-    if (!requests.payloads.empty()) {
-        getValueRequests = &requests.payloads;
-    } else {
-        deserializedResults = stableLargeParcelableToVector<GetValueRequest>(requests);
-        if (!deserializedResults.ok()) {
-            ALOGE("failed to parse getValues requests");
-            return std::move(deserializedResults.error());
-        }
-        getValueRequests = &deserializedResults.value();
+    expected<LargeParcelableBase::BorrowedOwnedObject<GetValueRequests>, ScopedAStatus>
+            deserializedResults = fromStableLargeParcelable(requests);
+    if (!deserializedResults.ok()) {
+        ALOGE("getValues: failed to parse getValues requests");
+        return std::move(deserializedResults.error());
     }
+    const std::vector<GetValueRequest>& getValueRequests =
+            deserializedResults.value().getObject()->payloads;
 
     std::shared_ptr<GetValuesClient> client;
     {
@@ -153,7 +150,7 @@ ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,
     }
 
     if (StatusCode status =
-                mVehicleHardware->getValues(client->getResultCallback(), *getValueRequests);
+                mVehicleHardware->getValues(client->getResultCallback(), getValueRequests);
         status != StatusCode::OK) {
         return ScopedAStatus::fromServiceSpecificErrorWithMessage(
                 toInt(status), "failed to get value from VehicleHardware");
@@ -166,27 +163,21 @@ ScopedAStatus DefaultVehicleHal::setValues(const CallbackType& callback,
                                            const SetValueRequests& requests) {
     // TODO(b/203713317): check for duplicate properties and duplicate request IDs.
 
-    const std::vector<SetValueRequest>* setValueRequests;
-    // Define deserializedResults here because we need it to have the same lifetime as
-    // setValueRequests.
-    expected<std::vector<SetValueRequest>, ScopedAStatus> deserializedResults;
-    if (!requests.payloads.empty()) {
-        setValueRequests = &requests.payloads;
-    } else {
-        deserializedResults = stableLargeParcelableToVector<SetValueRequest>(requests);
-        if (!deserializedResults.ok()) {
-            ALOGE("failed to parse setValues requests");
-            return std::move(deserializedResults.error());
-        }
-        setValueRequests = &deserializedResults.value();
+    expected<LargeParcelableBase::BorrowedOwnedObject<SetValueRequests>, ScopedAStatus>
+            deserializedResults = fromStableLargeParcelable(requests);
+    if (!deserializedResults.ok()) {
+        ALOGE("setValues: failed to parse setValues requests");
+        return std::move(deserializedResults.error());
     }
+    const std::vector<SetValueRequest>& setValueRequests =
+            deserializedResults.value().getObject()->payloads;
 
     // A list of failed result we already know before sending to hardware.
     std::vector<SetValueResult> failedResults;
     // The list of requests that we would send to hardware.
     std::vector<SetValueRequest> hardwareRequests;
 
-    for (auto& request : *setValueRequests) {
+    for (auto& request : setValueRequests) {
         int64_t requestId = request.requestId;
         if (auto result = checkProperty(request.value); !result.ok()) {
             ALOGW("property not valid: %s", result.error().message().c_str());

automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp

@@ -389,15 +389,14 @@ class DefaultVehicleHalTest : public ::testing::Test {
             });
         }
 
-        auto result = LargeParcelableBase::parcelableVectorToStableLargeParcelable(
-                expectedHardwareRequests);
+        requests.payloads = expectedHardwareRequests;
+        auto result = LargeParcelableBase::parcelableToStableLargeParcelable(requests);
         if (!result.ok()) {
             return result.error();
         }
-        if (result.value() == nullptr) {
-            requests.payloads = expectedHardwareRequests;
-        } else {
+        if (result.value() != nullptr) {
             requests.sharedMemoryFd = std::move(*result.value());
+            requests.payloads.clear();
         }
         return {};
     }
@@ -423,14 +422,13 @@ class DefaultVehicleHalTest : public ::testing::Test {
             });
         }
 
-        auto result = LargeParcelableBase::parcelableVectorToStableLargeParcelable(
-                expectedHardwareRequests);
+        requests.payloads = expectedHardwareRequests;
+        auto result = LargeParcelableBase::parcelableToStableLargeParcelable(requests);
         if (!result.ok()) {
             return result.error();
         }
-        if (result.value() == nullptr) {
-            requests.payloads = expectedHardwareRequests;
-        } else {
+        if (result.value() != nullptr) {
+            requests.payloads.clear();
             requests.sharedMemoryFd = std::move(*result.value());
         }
         return {};
@@ -490,13 +488,10 @@ TEST_F(DefaultVehicleHalTest, testGetAllPropConfigsLarge) {
 
     ASSERT_TRUE(status.isOk()) << "getAllPropConfigs failed: " << status.getMessage();
     ASSERT_TRUE(output.payloads.empty());
-    Result<std::optional<std::vector<VehiclePropConfig>>> result =
-            LargeParcelableBase::stableLargeParcelableToParcelableVector<VehiclePropConfig>(
-                    output.sharedMemoryFd);
+    auto result = LargeParcelableBase::stableLargeParcelableToParcelable(output);
     ASSERT_TRUE(result.ok()) << "failed to parse result shared memory file: "
                              << result.error().message();
-    ASSERT_TRUE(result.value().has_value()) << "empty parsed value";
-    ASSERT_EQ(result.value().value(), testConfigs);
+    ASSERT_EQ(result.value().getObject()->payloads, testConfigs);
 }
 
 TEST_F(DefaultVehicleHalTest, testGetValuesSmall) {
@@ -544,11 +539,9 @@ TEST_F(DefaultVehicleHalTest, testGetValuesLarge) {
     ASSERT_TRUE(getValueResults.payloads.empty())
             << "payload should be empty, shared memory file should be used";
 
-    auto result = LargeParcelableBase::stableLargeParcelableToParcelableVector<GetValueResult>(
-            getValueResults.sharedMemoryFd);
+    auto result = LargeParcelableBase::stableLargeParcelableToParcelable(getValueResults);
     ASSERT_TRUE(result.ok()) << "failed to parse shared memory file";
-    ASSERT_TRUE(result.value().has_value()) << "no parsed value";
-    ASSERT_EQ(result.value().value(), expectedResults) << "results mismatch";
+    ASSERT_EQ(result.value().getObject()->payloads, expectedResults) << "results mismatch";
     EXPECT_EQ(countClients(), static_cast<size_t>(1));
 }
 
@@ -621,11 +614,9 @@ TEST_F(DefaultVehicleHalTest, testSetValuesLarge) {
     ASSERT_TRUE(setValueResults.payloads.empty())
             << "payload should be empty, shared memory file should be used";
 
-    auto result = LargeParcelableBase::stableLargeParcelableToParcelableVector<SetValueResult>(
-            setValueResults.sharedMemoryFd);
+    auto result = LargeParcelableBase::stableLargeParcelableToParcelable(setValueResults);
     ASSERT_TRUE(result.ok()) << "failed to parse shared memory file";
-    ASSERT_TRUE(result.value().has_value()) << "no parsed value";
-    ASSERT_EQ(result.value().value(), expectedResults) << "results mismatch";
+    ASSERT_EQ(result.value().getObject()->payloads, expectedResults) << "results mismatch";
     EXPECT_EQ(countClients(), static_cast<size_t>(1));
 }