Merge "RAII style wakelocks: Add tryGet() factory method." am: ddff8e6dd0 am: e3a6357533 am: 9176bf1929

Original change: https://android-review.googlesource.com/c/platform/hardware/libhardware_legacy/+/1592033

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: I51ad002d2d778d6e674a86363e74eb5040d3219a

4 files changed, 45 insertions, 8 deletions

diff --git a/block_suspend.cpp b/block_suspend.cpp
index 18c93d4..d780904 100644
--- a/block_suspend.cpp
+++ b/block_suspend.cpp
@@ -34,7 +34,11 @@ int main(int argc, char ** /* argv */) {
         return 0;
     }
 
-    android::wakelock::WakeLock wl{gWakeLockName};  // RAII object
+    auto wl = android::wakelock::WakeLock::tryGet(gWakeLockName);  // RAII object
+    if (!wl.has_value()) {
+        return EXIT_FAILURE;
+    }
+
     while (true) { sleep(1000000); };
     std::abort();  // should never reach here
     return 0;
diff --git a/include/wakelock/wakelock.h b/include/wakelock/wakelock.h
index 75a7c72..44c962c 100644
--- a/include/wakelock/wakelock.h
+++ b/include/wakelock/wakelock.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <memory>
+#include <optional>
 #include <string>
 
 namespace android {
@@ -24,13 +25,17 @@ namespace wakelock {
 
 // RAII-style wake lock implementation
 class WakeLock {
-  public:
-    WakeLock(const std::string& name);
-    ~WakeLock();
-
   private:
     class WakeLockImpl;
     std::unique_ptr<WakeLockImpl> mImpl;
+
+  public:
+    static std::optional<WakeLock> tryGet(const std::string& name);
+    // Constructor is only made public for use with std::optional.
+    // It is not intended to be and cannot be invoked from public context,
+    // since private WakeLockImpl prevents calling the constructor directly.
+    WakeLock(std::unique_ptr<WakeLockImpl> wlImpl);
+    ~WakeLock();
 };
 
 }  // namespace wakelock
diff --git a/power.cpp b/power.cpp
index 16ed961..a39b32b 100644
--- a/power.cpp
+++ b/power.cpp
@@ -88,26 +88,51 @@ class WakeLock::WakeLockImpl {
   public:
     WakeLockImpl(const std::string& name);
     ~WakeLockImpl();
+    bool acquireOk();
 
   private:
     sp<IWakeLock> mWakeLock;
 };
 
-WakeLock::WakeLock(const std::string& name) : mImpl(std::make_unique<WakeLockImpl>(name)) {}
+std::optional<WakeLock> WakeLock::tryGet(const std::string& name) {
+    std::unique_ptr<WakeLockImpl> wlImpl = std::make_unique<WakeLockImpl>(name);
+    if (wlImpl->acquireOk()) {
+        return { std::move(wlImpl) };
+    } else {
+        LOG(ERROR) << "Failed to acquire wakelock: " << name;
+        return {};
+    }
+}
+
+WakeLock::WakeLock(std::unique_ptr<WakeLockImpl> wlImpl) : mImpl(std::move(wlImpl)) {}
 
 WakeLock::~WakeLock() = default;
 
 WakeLock::WakeLockImpl::WakeLockImpl(const std::string& name) : mWakeLock(nullptr) {
     static sp<ISystemSuspend> suspendService = ISystemSuspend::getService();
-    mWakeLock = suspendService->acquireWakeLock(WakeLockType::PARTIAL, name);
+    auto ret = suspendService->acquireWakeLock(WakeLockType::PARTIAL, name);
+    // It's possible that during device SystemSuspend service is not avaiable. In these
+    // situations HIDL calls to it will result in a DEAD_OBJECT transaction error.
+    if (ret.isDeadObject()) {
+        LOG(ERROR) << "ISuspendService::acquireWakeLock() call failed: " << ret.description();
+    } else {
+        mWakeLock = ret;
+    }
 }
 
 WakeLock::WakeLockImpl::~WakeLockImpl() {
+    if (!acquireOk()) {
+        return;
+    }
     auto ret = mWakeLock->release();
     if (!ret.isOk()) {
         LOG(ERROR) << "IWakeLock::release() call failed: " << ret.description();
     }
 }
 
+bool WakeLock::WakeLockImpl::acquireOk() {
+    return mWakeLock != nullptr;
+}
+
 }  // namespace wakelock
 }  // namespace android
diff --git a/power_test.cpp b/power_test.cpp
index 32cff70..9a68651 100644
--- a/power_test.cpp
+++ b/power_test.cpp
@@ -121,7 +121,10 @@ class WakeLockTest : public ::testing::Test {
 TEST_F(WakeLockTest, WakeLockDestructor) {
     auto name = std::to_string(rand());
     {
-        android::wakelock::WakeLock wl{name};
+        auto wl = android::wakelock::WakeLock::tryGet(name);
+        if (!wl.has_value()) {
+            return;
+        }
 
         WakeLockInfo info;
         auto success = findWakeLockInfoByName(controlService, name, &info);