From c1359ac51c970666691ef4d07a6969c434062037 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Fri, 1 Jul 2022 02:42:10 +0100 Subject: Reconcile native and Java parcel code for WorkSource. Prior to this change, native work sources didn't read or write any information concerning WorkChains, but Java ones did. This lead to a mismatch when Java code, such as PowerManagerService, unparceled the WorkSource as it read the whatever the next 4 bytes happened to be as the WorkChain count, but it was actually reading whatever the next item in the transaction happened to have written. Note that this change does _not_ attempt to add a WorkChain implementation to native. It instead makes it so that WorkSources written from native to Java, or Java to native with no WorkChains, are correctly and symmetrically parceled. In addition, if a Java WorkSource is sent to native with a non-zero amount of WorkChains associated, then it will return an error when unparceling rather than continuing but leaving in the Parcel in an undefined state. Bug: 234429395 Test: atest WorkSourceTest.cpp Test: Write sample native app that grabs wakelock, validate display ID Change-Id: Id1a5f29f4ccf2996e37ec99014ce392599b6b725 --- services/powermanager/WorkSource.cpp | 12 +++++-- services/powermanager/tests/Android.bp | 1 + services/powermanager/tests/WorkSourceTest.cpp | 46 ++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 services/powermanager/tests/WorkSourceTest.cpp diff --git a/services/powermanager/WorkSource.cpp b/services/powermanager/WorkSource.cpp index 1006a0666f..64a5499084 100644 --- a/services/powermanager/WorkSource.cpp +++ b/services/powermanager/WorkSource.cpp @@ -28,9 +28,16 @@ status_t WorkSource::readFromParcel(const android::Parcel *parcel) { return BAD_VALUE; } int32_t num; + int32_t workChainCount; status_t ret = parcel->readInt32(&num) ?: parcel->readInt32Vector(&mUids) - ?: parcel->readString16Vector(&mNames); + ?: parcel->readString16Vector(&mNames) + ?: parcel->readInt32(&workChainCount); + + if (ret == OK && workChainCount > 0) { + // We don't yet support WorkChains in native WorkSources. + return BAD_VALUE; + } return ret; } @@ -43,7 +50,8 @@ status_t WorkSource::writeToParcel(android::Parcel *parcel) const { return parcel->writeInt32(mUids.size()) ?: parcel->writeInt32Vector(mUids) - ?: parcel->writeString16Vector(mNames); + ?: parcel->writeString16Vector(mNames) + ?: parcel->writeInt32(-1); } } // namespace android::os diff --git a/services/powermanager/tests/Android.bp b/services/powermanager/tests/Android.bp index 2d1558a33f..962784cbae 100644 --- a/services/powermanager/tests/Android.bp +++ b/services/powermanager/tests/Android.bp @@ -31,6 +31,7 @@ cc_test { "PowerHalWrapperAidlTest.cpp", "PowerHalWrapperHidlV1_0Test.cpp", "PowerHalWrapperHidlV1_1Test.cpp", + "WorkSourceTest.cpp", ], cflags: [ "-Wall", diff --git a/services/powermanager/tests/WorkSourceTest.cpp b/services/powermanager/tests/WorkSourceTest.cpp new file mode 100644 index 0000000000..bb9164ac57 --- /dev/null +++ b/services/powermanager/tests/WorkSourceTest.cpp @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#define LOG_TAG "PowerHalLoaderTest" + +#include +#include +#include +#include + +#include + +using namespace android; +using namespace testing; + +TEST(WorkSourceTest, Parcel) { + std::vector uids = {1, 2}; + using Names = std::vector>; + std::optional names = std::make_optional({std::make_optional(String16("name"))}); + os::WorkSource ws{uids, names}; + + Parcel p; + ws.writeToParcel(&p); + p.setDataPosition(0); + + os::WorkSource otherWs; + otherWs.readFromParcel(&p); + + EXPECT_EQ(ws, otherWs); + EXPECT_EQ(uids, otherWs.getUids()); + EXPECT_EQ(names, otherWs.getNames()); +} -- cgit v1.2.3