OSDN Git Service

Add ISystemSuspendCallback.
authorTri Vo <trong@google.com>
Fri, 31 Aug 2018 00:58:41 +0000 (17:58 -0700)
committerTri Vo <trong@google.com>
Sun, 9 Sep 2018 23:24:59 +0000 (16:24 -0700)
The callback will be used to synchronously report system suspend events.

Current usage: BatteryStatsService needs to be notified about
each system suspend attempt and whether it was successful.

Bug: 78888165
Test: SystemSuspendV1_0UnitTest
Change-Id: I234f74f6f3c35e5d6bb33ceda569e11520850ccf

suspend/1.0/Android.bp
suspend/1.0/ISystemSuspend.hal
suspend/1.0/ISystemSuspendCallback.hal [new file with mode: 0644]
suspend/1.0/default/Android.bp
suspend/1.0/default/SystemSuspend.cpp
suspend/1.0/default/SystemSuspend.h
suspend/1.0/default/SystemSuspendUnitTest.cpp

index 3f7bf93..cdf4435 100644 (file)
@@ -6,6 +6,7 @@ hidl_interface {
     },
     srcs: [
         "ISystemSuspend.hal",
+        "ISystemSuspendCallback.hal",
         "IWakeLock.hal",
     ],
     interfaces: [
index 3452d14..344aa51 100644 (file)
 
 package android.system.suspend@1.0;
 
+import ISystemSuspendCallback;
 import IWakeLock;
 
 interface ISystemSuspend {
     /**
+     * Registers a callback for system suspend events. ISystemSuspend must keep
+     * track of all registered callbacks unless the client process that
+     * registered the callback dies.
+     *
+     * @param callback the callback to register.
+     * @return status true on success, false otherwise.
+     */
+    registerCallback(ISystemSuspendCallback callback) generates (bool success);
+
+    /**
      * Starts automatic system suspension.
      *
      * @return status true on success, false otherwise.
diff --git a/suspend/1.0/ISystemSuspendCallback.hal b/suspend/1.0/ISystemSuspendCallback.hal
new file mode 100644 (file)
index 0000000..3bc88d7
--- /dev/null
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2018 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.
+ */
+
+package android.system.suspend@1.0;
+
+interface ISystemSuspendCallback {
+    /**
+     * An implementation of ISystemSuspend must call notifyWakeup after every
+     * system wakeup.
+     *
+     * @param success whether previous system suspend attempt was successful.
+     */
+    notifyWakeup(bool success);
+};
index 10842f5..25bdff8 100644 (file)
@@ -84,7 +84,10 @@ cc_test {
         "system_suspend_defaults",
         "system_suspend_stats_defaults",
     ],
-    static_libs: ["android.system.suspend@1.0"],
+    static_libs: [
+        "android.system.suspend@1.0",
+        "libgmock",
+    ],
     srcs: [
         "SystemSuspend.cpp",
         "SystemSuspendUnitTest.cpp"
index 14800a8..bf22f12 100644 (file)
@@ -73,17 +73,12 @@ Return<void> WakeLock::release() {
 void WakeLock::releaseOnce() {
     std::call_once(mReleased, [this]() {
         mSystemSuspend->decSuspendCounter();
-        mSystemSuspend->deleteWakeLockStatsEntry(reinterpret_cast<uint64_t>(this));
+        mSystemSuspend->deleteWakeLockStatsEntry(reinterpret_cast<WakeLockIdType>(this));
     });
 }
 
 SystemSuspend::SystemSuspend(unique_fd wakeupCountFd, unique_fd stateFd)
-    : mCounterLock(),
-      mCounterCondVar(),
-      mSuspendCounter(0),
-      mWakeupCountFd(std::move(wakeupCountFd)),
-      mStateFd(std::move(stateFd)),
-      mStats() {}
+    : mSuspendCounter(0), mWakeupCountFd(std::move(wakeupCountFd)), mStateFd(std::move(stateFd)) {}
 
 Return<bool> SystemSuspend::enableAutosuspend() {
     static bool initialized = false;
@@ -105,11 +100,33 @@ Return<sp<IWakeLock>> SystemSuspend::acquireWakeLock(const hidl_string& name) {
         wlStats.set_name(name);
         wlStats.set_pid(getCallingPid());
         // Use WakeLock's address as a unique identifier.
-        (*mStats.mutable_wake_lock_stats())[reinterpret_cast<uint64_t>(wl)] = wlStats;
+        (*mStats.mutable_wake_lock_stats())[reinterpret_cast<WakeLockIdType>(wl)] = wlStats;
     }
     return wl;
 }
 
+Return<bool> SystemSuspend::registerCallback(const sp<ISystemSuspendCallback>& cb) {
+    if (!cb) {
+        return false;
+    }
+    auto l = std::lock_guard(mCallbackLock);
+    if (findCb(cb) == mCallbacks.end()) {
+        auto linkRet = cb->linkToDeath(this, 0 /* cookie */);
+        if (!linkRet.withDefault(false)) {
+            LOG(ERROR) << __func__ << "Cannot link to death: "
+                       << (linkRet.isOk() ? "linkToDeath returns false" : linkRet.description());
+            return false;
+        }
+        mCallbacks.push_back(cb);
+    }
+    return true;
+}
+
+void SystemSuspend::serviceDied(uint64_t, const wp<IBase>& service) {
+    auto l = std::lock_guard(mCallbackLock);
+    mCallbacks.erase(findCb(service.promote()));
+}
+
 Return<void> SystemSuspend::debug(const hidl_handle& handle,
                                   const hidl_vec<hidl_string>& /* options */) {
     if (handle == nullptr || handle->numFds < 1 || handle->data[0] < 0) {
@@ -139,7 +156,7 @@ void SystemSuspend::decSuspendCounter() {
     }
 }
 
-void SystemSuspend::deleteWakeLockStatsEntry(uint64_t id) {
+void SystemSuspend::deleteWakeLockStatsEntry(WakeLockIdType id) {
     auto l = std::lock_guard(mStatsLock);
     mStats.mutable_wake_lock_stats()->erase(id);
 }
@@ -154,19 +171,26 @@ void SystemSuspend::initAutosuspend() {
                 continue;
             }
 
-            auto l = std::unique_lock(mCounterLock);
-            mCounterCondVar.wait(l, [this] { return mSuspendCounter == 0; });
-            // The mutex is locked and *MUST* remain locked until the end of the scope. Otherwise,
-            // a WakeLock might be acquired after we check mSuspendCounter and before we write to
-            // /sys/power/state.
+            auto counterLock = std::unique_lock(mCounterLock);
+            mCounterCondVar.wait(counterLock, [this] { return mSuspendCounter == 0; });
+            // The mutex is locked and *MUST* remain locked until we write to /sys/power/state.
+            // Otherwise, a WakeLock might be acquired after we check mSuspendCounter and before we
+            // write to /sys/power/state.
 
             if (!WriteStringToFd(wakeupCount, mWakeupCountFd)) {
                 PLOG(VERBOSE) << "error writing from /sys/power/wakeup_count";
                 continue;
             }
-            if (!WriteStringToFd(kSleepState, mStateFd)) {
+            bool success = WriteStringToFd(kSleepState, mStateFd);
+            counterLock.unlock();
+
+            if (!success) {
                 PLOG(VERBOSE) << "error writing to /sys/power/state";
             }
+            auto callbackLock = std::lock_guard(mCallbackLock);
+            for (const auto& callback : mCallbacks) {
+                callback->notifyWakeup(success).isOk();  // ignore errors
+            }
         }
     });
     autosuspendThread.detach();
index d5f2ea1..fc397fc 100644 (file)
@@ -19,6 +19,8 @@
 
 #include <android-base/unique_fd.h>
 #include <android/system/suspend/1.0/ISystemSuspend.h>
+#include <android/system/suspend/1.0/ISystemSuspendCallback.h>
+#include <hidl/HidlTransportSupport.h>
 #include <system/hardware/interfaces/suspend/1.0/default/SystemSuspendStats.pb.h>
 
 #include <condition_variable>
@@ -31,11 +33,15 @@ namespace suspend {
 namespace V1_0 {
 
 using ::android::base::unique_fd;
+using ::android::hardware::hidl_death_recipient;
 using ::android::hardware::hidl_handle;
 using ::android::hardware::hidl_string;
 using ::android::hardware::hidl_vec;
+using ::android::hardware::interfacesEqual;
 using ::android::hardware::Return;
 
+using WakeLockIdType = uint64_t;
+
 class SystemSuspend;
 
 std::string readFd(int fd);
@@ -54,15 +60,17 @@ class WakeLock : public IWakeLock {
     SystemSuspend* mSystemSuspend;
 };
 
-class SystemSuspend : public ISystemSuspend {
+class SystemSuspend : public ISystemSuspend, public hidl_death_recipient {
    public:
     SystemSuspend(unique_fd wakeupCountFd, unique_fd stateFd);
     Return<bool> enableAutosuspend() override;
     Return<sp<IWakeLock>> acquireWakeLock(const hidl_string& name) override;
+    Return<bool> registerCallback(const sp<ISystemSuspendCallback>& cb) override;
     Return<void> debug(const hidl_handle& handle, const hidl_vec<hidl_string>& options) override;
+    void serviceDied(uint64_t /* cookie */, const wp<IBase>& service);
     void incSuspendCounter();
     void decSuspendCounter();
-    void deleteWakeLockStatsEntry(uint64_t id);
+    void deleteWakeLockStatsEntry(WakeLockIdType id);
 
    private:
     void initAutosuspend();
@@ -78,6 +86,14 @@ class SystemSuspend : public ISystemSuspend {
     // Never hold both locks at the same time to avoid deadlock.
     std::mutex mStatsLock;
     SystemSuspendStats mStats;
+
+    using CbType = sp<ISystemSuspendCallback>;
+    std::mutex mCallbackLock;
+    std::vector<CbType> mCallbacks;
+    std::vector<CbType>::iterator findCb(const sp<IBase>& cb) {
+        return std::find_if(mCallbacks.begin(), mCallbacks.end(),
+                            [&cb](const CbType& i) { return interfacesEqual(i, cb); });
+    }
 };
 
 }  // namespace V1_0
index 4c85968..d7055d1 100644 (file)
@@ -20,6 +20,7 @@
 #include <android-base/logging.h>
 #include <android-base/unique_fd.h>
 #include <cutils/native_handle.h>
+#include <gmock/gmock.h>
 #include <google/protobuf/text_format.h>
 #include <gtest/gtest.h>
 #include <hidl/HidlTransportSupport.h>
@@ -44,6 +45,7 @@ using android::hardware::joinRpcThreadpool;
 using android::hardware::Return;
 using android::hardware::Void;
 using android::system::suspend::V1_0::ISystemSuspend;
+using android::system::suspend::V1_0::ISystemSuspendCallback;
 using android::system::suspend::V1_0::IWakeLock;
 using android::system::suspend::V1_0::readFd;
 using android::system::suspend::V1_0::SystemSuspend;
@@ -151,6 +153,18 @@ class SystemSuspendTest : public ::testing::Test {
 
     size_t getWakeLockCount() { return getDebugDump().wake_lock_stats().size(); }
 
+    void checkLoop(int numIter) {
+        for (int i = 0; i < numIter; i++) {
+            // Mock value for /sys/power/wakeup_count.
+            std::string wakeupCount = std::to_string(rand());
+            ASSERT_TRUE(WriteStringToFd(wakeupCount, wakeupCountFd));
+            ASSERT_EQ(readFd(wakeupCountFd), wakeupCount)
+                << "wakeup count value written by SystemSuspend is not equal to value given to it";
+            ASSERT_EQ(readFd(stateFd), "mem")
+                << "SystemSuspend failed to write correct sleep state.";
+        }
+    }
+
     sp<ISystemSuspend> suspendService;
     int stateFd;
     int wakeupCountFd;
@@ -162,14 +176,7 @@ TEST_F(SystemSuspendTest, OnlyOneEnableAutosuspend) {
 }
 
 TEST_F(SystemSuspendTest, AutosuspendLoop) {
-    for (int i = 0; i < 2; i++) {
-        // Mock value for /sys/power/wakeup_count.
-        std::string wakeupCount = std::to_string(rand());
-        ASSERT_TRUE(WriteStringToFd(wakeupCount, wakeupCountFd));
-        ASSERT_EQ(readFd(wakeupCountFd), wakeupCount)
-            << "wakeup count value written by SystemSuspend is not equal to value given to it";
-        ASSERT_EQ(readFd(stateFd), "mem") << "SystemSuspend failed to write correct sleep state.";
-    }
+    checkLoop(5);
 }
 
 // Tests that upon WakeLock destruction SystemSuspend HAL is unblocked.
@@ -270,11 +277,66 @@ TEST_F(SystemSuspendTest, WakeLockStressTest) {
     ASSERT_EQ(getWakeLockCount(), 0);
 }
 
+// Callbacks are passed around as sp<>. However, mock expectations are verified when mock objects
+// are destroyed, i.e. the test needs to control lifetime of the mock object.
+// MockCallbackImpl can be destroyed independently of its wrapper MockCallback which is passed to
+// SystemSuspend.
+struct MockCallbackImpl {
+    MOCK_METHOD1(notifyWakeup, Return<void>(bool));
+};
+
+class MockCallback : public ISystemSuspendCallback {
+   public:
+    MockCallback(MockCallbackImpl* impl) : mImpl(impl), mDisabled(false) {}
+    Return<void> notifyWakeup(bool x) { return mDisabled ? Void() : mImpl->notifyWakeup(x); }
+    // In case we pull the rug from under MockCallback, but SystemSuspend still has an sp<> to the
+    // object.
+    void disable() { mDisabled = true; }
+
+   private:
+    MockCallbackImpl* mImpl;
+    bool mDisabled;
+};
+
+// Tests that nullptr can't be registered as callbacks.
+TEST_F(SystemSuspendTest, RegisterInvalidCallback) {
+    ASSERT_FALSE(suspendService->registerCallback(nullptr));
+}
+
+// Tests that SystemSuspend HAL correctly notifies wakeup events.
+TEST_F(SystemSuspendTest, CallbackNotifyWakeup) {
+    constexpr int numWakeups = 5;
+    MockCallbackImpl impl;
+    // SystemSuspend should suspend numWakeup + 1 times. However, it might
+    // only be able to notify numWakeup times. The test case might have
+    // finished by the time last notification completes.
+    EXPECT_CALL(impl, notifyWakeup).Times(testing::AtLeast(numWakeups));
+    sp<MockCallback> cb = new MockCallback(&impl);
+    ASSERT_TRUE(suspendService->registerCallback(cb));
+    checkLoop(numWakeups + 1);
+    cb->disable();
+}
+
+// Tests that SystemSuspend HAL correctly deals with a dead callback.
+TEST_F(SystemSuspendTest, DeadCallback) {
+    ASSERT_EXIT(
+        {
+            sp<MockCallback> cb = new MockCallback(nullptr);
+            ASSERT_TRUE(suspendService->registerCallback(cb));
+            std::exit(0);
+        },
+        ::testing::ExitedWithCode(0), "");
+
+    // Dead process callback must still be dealt with either by unregistering it
+    // or checking isOk() on every call.
+    checkLoop(3);
+}
 }  // namespace android
 
 int main(int argc, char** argv) {
     setenv("TREBLE_TESTING_OVERRIDE", "true", true);
     ::testing::AddGlobalTestEnvironment(android::SystemSuspendTestEnvironment::Instance());
+    ::testing::InitGoogleMock(&argc, argv);
     ::testing::InitGoogleTest(&argc, argv);
     return RUN_ALL_TESTS();
 }