diff options
-rw-r--r-- | client/wake_lock_unittest.cc | 12 | ||||
-rw-r--r-- | daemon/Android.mk | 15 | ||||
-rw-r--r-- | daemon/power_manager.cc | 9 | ||||
-rw-r--r-- | daemon/power_manager_stub.cc | 56 | ||||
-rw-r--r-- | daemon/power_manager_unittest.cc | 28 | ||||
-rw-r--r-- | daemon/wake_lock_manager.cc | 6 | ||||
-rw-r--r-- | daemon/wake_lock_manager.h | 29 | ||||
-rw-r--r-- | daemon/wake_lock_manager_stub.cc | 27 | ||||
-rw-r--r-- | daemon/wake_lock_manager_stub.h | 24 | ||||
-rw-r--r-- | include/nativepower/power_manager_stub.h | 44 |
10 files changed, 145 insertions, 105 deletions
diff --git a/client/wake_lock_unittest.cc b/client/wake_lock_unittest.cc index 09f3dbf..dbd1505 100644 --- a/client/wake_lock_unittest.cc +++ b/client/wake_lock_unittest.cc @@ -50,15 +50,17 @@ class WakeLockTest : public BinderTestBase { }; TEST_F(WakeLockTest, CreateAndDestroy) { + const uid_t kUid = 123; + binder_wrapper()->set_calling_uid(kUid); std::unique_ptr<WakeLock> lock(client_.CreateWakeLock("foo", "bar")); - ASSERT_EQ(1u, power_manager_->num_locks()); + ASSERT_EQ(1, power_manager_->GetNumWakeLocks()); ASSERT_EQ(1u, binder_wrapper()->local_binders().size()); EXPECT_EQ( - PowerManagerStub::ConstructLockString("foo", "bar", -1), - power_manager_->GetLockString(binder_wrapper()->local_binders()[0])); + PowerManagerStub::ConstructWakeLockString("foo", "bar", kUid), + power_manager_->GetWakeLockString(binder_wrapper()->local_binders()[0])); lock.reset(); - EXPECT_EQ(0u, power_manager_->num_locks()); + EXPECT_EQ(0, power_manager_->GetNumWakeLocks()); } TEST_F(WakeLockTest, PowerManagerDeath) { @@ -68,7 +70,7 @@ TEST_F(WakeLockTest, PowerManagerDeath) { // Since PowerManagerClient was informed that the power manager died, WakeLock // shouldn't try to release its lock on destruction. lock.reset(); - EXPECT_EQ(1u, power_manager_->num_locks()); + EXPECT_EQ(1, power_manager_->GetNumWakeLocks()); } } // namespace android diff --git a/daemon/Android.mk b/daemon/Android.mk index 0eb0f5d..53d9b08 100644 --- a/daemon/Android.mk +++ b/daemon/Android.mk @@ -18,9 +18,13 @@ LOCAL_PATH := $(call my-dir) nativepowerman_CommonCFlags := -Wall -Werror -Wno-unused-parameter nativepowerman_CommonCFlags += -Wno-sign-promo # for libchrome -nativepowerman_CommonCIncludes := $(LOCAL_PATH)/../include +nativepowerman_CommonCIncludes := \ + $(LOCAL_PATH)/../include \ + external/gtest/include \ + nativepowerman_CommonSharedLibraries := \ libbinder \ + libbinderwrapper \ libchrome \ libcutils \ libpowermanager \ @@ -39,7 +43,6 @@ LOCAL_CFLAGS := $(nativepowerman_CommonCFlags) LOCAL_STATIC_LIBRARIES := libnativepowerman LOCAL_SHARED_LIBRARIES := \ $(nativepowerman_CommonSharedLibraries) \ - libbinderwrapper \ libchromeos \ libchromeos-binder \ @@ -69,11 +72,10 @@ include $(CLEAR_VARS) LOCAL_MODULE := libnativepowerman LOCAL_CPP_EXTENSION := .cc LOCAL_CFLAGS := $(nativepowerman_CommonCFlags) -LOCAL_C_INCLUDES := $(nativepowerman_CommonCIncludes) external/gtest/include +LOCAL_C_INCLUDES := $(nativepowerman_CommonCIncludes) LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/../include LOCAL_SHARED_LIBRARIES := \ $(nativepowerman_CommonSharedLibraries) \ - libbinderwrapper \ libchromeos \ LOCAL_SRC_FILES := \ @@ -97,13 +99,12 @@ LOCAL_CFLAGS := $(nativepowerman_CommonCFlags) LOCAL_STATIC_LIBRARIES := libnativepowerman libgtest libBionicGtestMain LOCAL_SHARED_LIBRARIES := \ $(nativepowerman_CommonSharedLibraries) \ - libbinderwrapper \ libbinderwrapper_test_support \ + libnativepower_test_support \ LOCAL_SRC_FILES := \ power_manager_unittest.cc \ system_property_setter_stub.cc \ - wake_lock_manager_stub.cc \ wake_lock_manager_unittest.cc \ include $(BUILD_NATIVE_TEST) @@ -121,5 +122,7 @@ LOCAL_SHARED_LIBRARIES := $(nativepowerman_CommonSharedLibraries) LOCAL_SRC_FILES := \ BnPowerManager.cc \ power_manager_stub.cc \ + wake_lock_manager.cc \ + wake_lock_manager_stub.cc \ include $(BUILD_SHARED_LIBRARY) diff --git a/daemon/power_manager.cc b/daemon/power_manager.cc index bb642b1..0498965 100644 --- a/daemon/power_manager.cc +++ b/daemon/power_manager.cc @@ -62,7 +62,10 @@ status_t PowerManager::acquireWakeLock(int flags, const String16& tag, const String16& packageName, bool isOneWay) { - return AddWakeLockRequest(lock, tag, packageName, -1) ? OK : UNKNOWN_ERROR; + return AddWakeLockRequest(lock, tag, packageName, + BinderWrapper::Get()->GetCallingUid()) + ? OK + : UNKNOWN_ERROR; } status_t PowerManager::acquireWakeLockWithUid(int flags, @@ -71,7 +74,9 @@ status_t PowerManager::acquireWakeLockWithUid(int flags, const String16& packageName, int uid, bool isOneWay) { - return AddWakeLockRequest(lock, tag, packageName, uid) ? OK : UNKNOWN_ERROR; + return AddWakeLockRequest(lock, tag, packageName, static_cast<uid_t>(uid)) + ? OK + : UNKNOWN_ERROR; } status_t PowerManager::releaseWakeLock(const sp<IBinder>& lock, diff --git a/daemon/power_manager_stub.cc b/daemon/power_manager_stub.cc index 7e71bcd..bb2588a 100644 --- a/daemon/power_manager_stub.cc +++ b/daemon/power_manager_stub.cc @@ -17,21 +17,13 @@ #include <base/format_macros.h> #include <base/logging.h> #include <base/strings/stringprintf.h> +#include <binderwrapper/binder_wrapper.h> #include <nativepower/power_manager_stub.h> #include <utils/String8.h> -namespace android { - -PowerManagerStub::LockInfo::LockInfo() : uid(-1) {} - -PowerManagerStub::LockInfo::LockInfo(const LockInfo& info) = default; +#include "wake_lock_manager_stub.h" -PowerManagerStub::LockInfo::LockInfo(const std::string& tag, - const std::string& package, - int uid) - : tag(tag), - package(package), - uid(uid) {} +namespace android { PowerManagerStub::SuspendRequest::SuspendRequest(int64_t event_time_ms, int reason, @@ -41,10 +33,11 @@ PowerManagerStub::SuspendRequest::SuspendRequest(int64_t event_time_ms, flags(flags) {} // static -std::string PowerManagerStub::ConstructLockString(const std::string& tag, - const std::string& package, - int uid) { - return base::StringPrintf("%s,%s,%d", tag.c_str(), package.c_str(), uid); +std::string PowerManagerStub::ConstructWakeLockString( + const std::string& tag, + const std::string& package, + uid_t uid) { + return WakeLockManagerStub::ConstructRequestString(tag, package, uid); } // static @@ -55,17 +48,18 @@ std::string PowerManagerStub::ConstructSuspendRequestString( return base::StringPrintf("%" PRId64 ",%d,%d", event_time_ms, reason, flags); } -PowerManagerStub::PowerManagerStub() = default; +PowerManagerStub::PowerManagerStub() + : wake_lock_manager_(new WakeLockManagerStub()) {} PowerManagerStub::~PowerManagerStub() = default; -std::string PowerManagerStub::GetLockString(const sp<IBinder>& binder) const { - const auto it = locks_.find(binder); - if (it == locks_.end()) - return std::string(); +int PowerManagerStub::GetNumWakeLocks() const { + return wake_lock_manager_->num_requests(); +} - const LockInfo& info = it->second; - return ConstructLockString(info.tag, info.package, info.uid); +std::string PowerManagerStub::GetWakeLockString( + const sp<IBinder>& binder) const { + return wake_lock_manager_->GetRequestString(binder); } std::string PowerManagerStub::GetSuspendRequestString(size_t index) const { @@ -82,10 +76,9 @@ status_t PowerManagerStub::acquireWakeLock(int flags, const String16& tag, const String16& packageName, bool isOneWay) { - CHECK(!locks_.count(lock)) << "Got acquireWakeLock() request for " - << "already-registered binder " << lock.get(); - locks_[lock] = - LockInfo(String8(tag).string(), String8(packageName).string(), -1); + CHECK(wake_lock_manager_->AddRequest(lock, String8(tag).string(), + String8(packageName).string(), + BinderWrapper::Get()->GetCallingUid())); return OK; } @@ -95,19 +88,16 @@ status_t PowerManagerStub::acquireWakeLockWithUid(int flags, const String16& packageName, int uid, bool isOneWay) { - CHECK(!locks_.count(lock)) << "Got acquireWakeLockWithUid() request for " - << "already-registered binder " << lock.get(); - locks_[lock] = - LockInfo(String8(tag).string(), String8(packageName).string(), uid); + CHECK(wake_lock_manager_->AddRequest(lock, String8(tag).string(), + String8(packageName).string(), + static_cast<uid_t>(uid))); return OK; } status_t PowerManagerStub::releaseWakeLock(const sp<IBinder>& lock, int flags, bool isOneWay) { - CHECK(locks_.count(lock)) << "Got releaseWakeLock() request for unregistered " - << "binder " << lock.get(); - locks_.erase(lock); + CHECK(wake_lock_manager_->RemoveRequest(lock)); return OK; } diff --git a/daemon/power_manager_unittest.cc b/daemon/power_manager_unittest.cc index 148035c..a81f935 100644 --- a/daemon/power_manager_unittest.cc +++ b/daemon/power_manager_unittest.cc @@ -88,13 +88,33 @@ TEST_F(PowerManagerTest, RegisterService) { } TEST_F(PowerManagerTest, AcquireAndReleaseWakeLock) { + const char kTag[] = "foo"; + const char kPackage[] = "bar"; sp<BBinder> binder = binder_wrapper()->CreateLocalBinder(); - EXPECT_EQ(OK, interface_->acquireWakeLock(0, binder, String16(), String16())); - ASSERT_EQ(1u, wake_lock_manager_->request_binders().size()); - EXPECT_TRUE(wake_lock_manager_->has_request_binder(binder)); + + // Check that PowerManager looks up the calling UID when necessary. + const uid_t kCallingUid = 100; + binder_wrapper()->set_calling_uid(kCallingUid); + EXPECT_EQ(OK, interface_->acquireWakeLock(0, binder, String16(kTag), + String16(kPackage))); + EXPECT_EQ(1, wake_lock_manager_->num_requests()); + EXPECT_EQ( + WakeLockManagerStub::ConstructRequestString(kTag, kPackage, kCallingUid), + wake_lock_manager_->GetRequestString( + binder_wrapper()->local_binders()[0])); EXPECT_EQ(OK, interface_->releaseWakeLock(binder, 0)); - EXPECT_TRUE(wake_lock_manager_->request_binders().empty()); + EXPECT_EQ(0, wake_lock_manager_->num_requests()); + + // If a UID is passed, it should be used instead. + const uid_t kPassedUid = 200; + EXPECT_EQ(OK, interface_->acquireWakeLockWithUid( + 0, binder, String16(kTag), String16(kPackage), kPassedUid)); + EXPECT_EQ(1, wake_lock_manager_->num_requests()); + EXPECT_EQ( + WakeLockManagerStub::ConstructRequestString(kTag, kPackage, kPassedUid), + wake_lock_manager_->GetRequestString( + binder_wrapper()->local_binders()[0])); } TEST_F(PowerManagerTest, GoToSleep) { diff --git a/daemon/wake_lock_manager.cc b/daemon/wake_lock_manager.cc index 3c875f6..37de1e3 100644 --- a/daemon/wake_lock_manager.cc +++ b/daemon/wake_lock_manager.cc @@ -51,14 +51,14 @@ const char WakeLockManager::kLockName[] = "nativepowerman"; WakeLockManager::Request::Request(const std::string& tag, const std::string& package, - int uid) + uid_t uid) : tag(tag), package(package), uid(uid) {} WakeLockManager::Request::Request(const Request& request) = default; -WakeLockManager::Request::Request() : uid(0) {} +WakeLockManager::Request::Request() : uid(-1) {} WakeLockManager::WakeLockManager() : lock_path_(kLockPath), @@ -82,7 +82,7 @@ bool WakeLockManager::Init() { bool WakeLockManager::AddRequest(sp<IBinder> client_binder, const std::string& tag, const std::string& package, - int uid) { + uid_t uid) { const bool new_request = !requests_.count(client_binder); LOG(INFO) << (new_request ? "Adding" : "Updating") << " request for binder " << client_binder.get() << ": tag=\"" << tag << "\"" diff --git a/daemon/wake_lock_manager.h b/daemon/wake_lock_manager.h index 471ad10..cc4dcc9 100644 --- a/daemon/wake_lock_manager.h +++ b/daemon/wake_lock_manager.h @@ -17,6 +17,8 @@ #ifndef SYSTEM_NATIVEPOWER_DAEMON_WAKE_LOCK_MANAGER_H_ #define SYSTEM_NATIVEPOWER_DAEMON_WAKE_LOCK_MANAGER_H_ +#include <sys/types.h> + #include <map> #include <string> @@ -37,8 +39,20 @@ class WakeLockManagerInterface { virtual bool AddRequest(sp<IBinder> client_binder, const std::string& tag, const std::string& package, - int uid) = 0; + uid_t uid) = 0; virtual bool RemoveRequest(sp<IBinder> client_binder) = 0; + + protected: + // Information about a request from a client. + struct Request { + Request(const std::string& tag, const std::string& package, uid_t uid); + Request(const Request& request); + Request(); + + std::string tag; + std::string package; + uid_t uid; + }; }; class WakeLockManager : public WakeLockManagerInterface { @@ -61,21 +75,10 @@ class WakeLockManager : public WakeLockManagerInterface { bool AddRequest(sp<IBinder> client_binder, const std::string& tag, const std::string& package, - int uid) override; + uid_t uid) override; bool RemoveRequest(sp<IBinder> client_binder) override; private: - // Information about a request from a client. - struct Request { - Request(const std::string& tag, const std::string& package, int uid); - Request(const Request& request); - Request(); - - std::string tag; - std::string package; - int uid; - }; - void HandleBinderDeath(sp<IBinder> binder); base::FilePath lock_path_; diff --git a/daemon/wake_lock_manager_stub.cc b/daemon/wake_lock_manager_stub.cc index da7dc5e..3b56c4e 100644 --- a/daemon/wake_lock_manager_stub.cc +++ b/daemon/wake_lock_manager_stub.cc @@ -16,27 +16,46 @@ #include "wake_lock_manager_stub.h" +#include <base/strings/stringprintf.h> #include <binder/IBinder.h> namespace android { +// static +std::string WakeLockManagerStub::ConstructRequestString( + const std::string& tag, + const std::string& package, + uid_t uid) { + return base::StringPrintf("%s,%s,%d", tag.c_str(), package.c_str(), uid); +} + WakeLockManagerStub::WakeLockManagerStub() = default; WakeLockManagerStub::~WakeLockManagerStub() = default; +std::string WakeLockManagerStub::GetRequestString( + const sp<IBinder>& binder) const { + const auto it = requests_.find(binder); + if (it == requests_.end()) + return std::string(); + + const Request& req = it->second; + return ConstructRequestString(req.tag, req.package, req.uid); +} + bool WakeLockManagerStub::AddRequest(sp<IBinder> client_binder, const std::string& tag, const std::string& package, - int uid) { - request_binders_.insert(client_binder); + uid_t uid) { + requests_[client_binder] = Request(tag, package, uid); return true; } bool WakeLockManagerStub::RemoveRequest(sp<IBinder> client_binder) { - if (!request_binders_.count(client_binder)) + if (!requests_.count(client_binder)) return false; - request_binders_.erase(client_binder); + requests_.erase(client_binder); return true; } diff --git a/daemon/wake_lock_manager_stub.h b/daemon/wake_lock_manager_stub.h index 3b73749..d720143 100644 --- a/daemon/wake_lock_manager_stub.h +++ b/daemon/wake_lock_manager_stub.h @@ -19,6 +19,7 @@ #include <set> #include <string> +#include <sys/types.h> #include <base/macros.h> #include <utils/StrongPointer.h> @@ -32,26 +33,31 @@ class IBinder; // Stub implementation used by tests. class WakeLockManagerStub : public WakeLockManagerInterface { public: - using BinderSet = std::set<sp<IBinder>>; - WakeLockManagerStub(); ~WakeLockManagerStub() override; - const BinderSet& request_binders() const { return request_binders_; } - bool has_request_binder(const sp<IBinder>& binder) const { - return request_binders_.count(binder); - } + // Constructs a string that can be compared with one returned by + // GetRequestString(). + static std::string ConstructRequestString(const std::string& tag, + const std::string& package, + uid_t uid); + + int num_requests() const { return requests_.size(); } + + // Returns a string describing the request associated with |binder|, or an + // empty string if no request is present. + std::string GetRequestString(const sp<IBinder>& binder) const; // WakeLockManagerInterface: bool AddRequest(sp<IBinder> client_binder, const std::string& tag, const std::string& package, - int uid) override; + uid_t uid) override; bool RemoveRequest(sp<IBinder> client_binder) override; private: - // Binders passed to AddRequest() for currently-active requests. - BinderSet request_binders_; + // Currently-active requests, keyed by client binders. + std::map<sp<IBinder>, Request> requests_; DISALLOW_COPY_AND_ASSIGN(WakeLockManagerStub); }; diff --git a/include/nativepower/power_manager_stub.h b/include/nativepower/power_manager_stub.h index d31c130..47ae1dd 100644 --- a/include/nativepower/power_manager_stub.h +++ b/include/nativepower/power_manager_stub.h @@ -14,7 +14,10 @@ * limitations under the License. */ +#include <sys/types.h> + #include <map> +#include <memory> #include <string> #include <vector> @@ -23,17 +26,21 @@ namespace android { +class WakeLockManagerStub; + // Stub implementation of BnPowerManager for use in tests. +// +// The BinderWrapper singleton must be initialized before using this class. class PowerManagerStub : public BnPowerManager { public: PowerManagerStub(); ~PowerManagerStub() override; // Constructs a string that can be compared with one returned by - // GetLockString(). - static std::string ConstructLockString(const std::string& tag, - const std::string& package, - int uid); + // GetWakeLockString(). + static std::string ConstructWakeLockString(const std::string& tag, + const std::string& package, + uid_t uid); // Constructs a string that can be compared with one returned by // GetSuspendRequestString(). @@ -41,7 +48,6 @@ class PowerManagerStub : public BnPowerManager { int reason, int flags); - size_t num_locks() const { return locks_.size(); } size_t num_suspend_requests() const { return suspend_requests_.size(); } const std::vector<std::string>& reboot_reasons() const { return reboot_reasons_; @@ -50,9 +56,12 @@ class PowerManagerStub : public BnPowerManager { return shutdown_reasons_; } - // Returns a string describing the lock registered for |binder|, or an empty - // string if no lock is present. - std::string GetLockString(const sp<IBinder>& binder) const; + // Returns the number of currently-registered wake locks. + int GetNumWakeLocks() const; + + // Returns a string describing the wake lock registered for |binder|, or an + // empty string if no wake lock is present. + std::string GetWakeLockString(const sp<IBinder>& binder) const; // Returns a string describing position |index| in |suspend_requests_|. std::string GetSuspendRequestString(size_t index) const; @@ -83,22 +92,6 @@ class PowerManagerStub : public BnPowerManager { status_t crash(const String16& message) override; private: - // Contains information passed to acquireWakeLock() or - // acquireWakeLockWithUid(). - struct LockInfo { - LockInfo(); - LockInfo(const LockInfo& info); - LockInfo(const std::string& tag, - const std::string& package, - int uid); - - std::string tag; - std::string package; - - // -1 if acquireWakeLock() was used. - int uid; - }; - // Details about a request passed to goToSleep(). struct SuspendRequest { SuspendRequest(int64 uptime_ms, int reason, int flags); @@ -108,8 +101,7 @@ class PowerManagerStub : public BnPowerManager { int flags; }; - using LockInfoMap = std::map<sp<IBinder>, LockInfo>; - LockInfoMap locks_; + std::unique_ptr<WakeLockManagerStub> wake_lock_manager_; // Information about calls to goToSleep(), in the order they were made. using SuspendRequests = std::vector<SuspendRequest>; |