graphics: add Composer to HAL support library

Extract IComposer implementation from HwcHal and move it to the HAL
support library.  This requires removal of

  ComposerHal::removeClient
  ComposerHal::enableCallback

and addition of

  ComposerHal::dumpDebugInfo
  ComposerHal::registerEventCallback
  ComposerHal::unregisterEventCallback

since HwcHal does not own a client to send the events to anymore.

Test: boots and VTS
Change-Id: I491e3d2c31d686661d4d3a44842bcac62cc2b2dc
diff --git a/graphics/composer/2.1/default/Hwc.cpp b/graphics/composer/2.1/default/Hwc.cpp
index e108fca..412af59 100644
--- a/graphics/composer/2.1/default/Hwc.cpp
+++ b/graphics/composer/2.1/default/Hwc.cpp
@@ -20,6 +20,8 @@
 
 #include <chrono>
 #include <type_traits>
+
+#include <composer-hal/2.1/Composer.h>
 #include <log/log.h>
 
 #include "hardware/fb.h"
@@ -212,32 +214,7 @@
     return (mCapabilities.count(capability) > 0);
 }
 
-Return<void> HwcHal::getCapabilities(getCapabilities_cb hidl_cb)
-{
-    std::vector<Capability> caps;
-    caps.reserve(mCapabilities.size());
-    for (auto cap : mCapabilities) {
-        switch (cap) {
-            case HWC2_CAPABILITY_SIDEBAND_STREAM:
-            case HWC2_CAPABILITY_SKIP_CLIENT_COLOR_TRANSFORM:
-            case HWC2_CAPABILITY_PRESENT_FENCE_IS_NOT_RELIABLE:
-                caps.push_back(static_cast<Capability>(cap));
-                break;
-            default:
-                // not all HWC2 caps are defined in HIDL
-                break;
-        }
-    }
-
-    hidl_vec<Capability> caps_reply;
-    caps_reply.setToExternal(caps.data(), caps.size());
-    hidl_cb(caps_reply);
-
-    return Void();
-}
-
-Return<void> HwcHal::dumpDebugInfo(dumpDebugInfo_cb hidl_cb)
-{
+std::string HwcHal::dumpDebugInfo() {
     uint32_t len = 0;
     mDispatch.dump(mDevice, &len, nullptr);
 
@@ -246,123 +223,56 @@
     buf.resize(len + 1);
     buf[len] = '\0';
 
-    hidl_string buf_reply;
-    buf_reply.setToExternal(buf.data(), len);
-    hidl_cb(buf_reply);
-
-    return Void();
-}
-
-Return<void> HwcHal::createClient(createClient_cb hidl_cb)
-{
-    Error err = Error::NONE;
-    sp<ComposerClient> client;
-
-    {
-        std::unique_lock<std::mutex> lock(mClientMutex);
-
-        if (mClient != nullptr) {
-            // In surface flinger we delete a composer client on one thread and
-            // then create a new client on another thread. Although surface
-            // flinger ensures the calls are made in that sequence (destroy and
-            // then create), sometimes the calls land in the composer service
-            // inverted (create and then destroy). Wait for a brief period to
-            // see if the existing client is destroyed.
-            ALOGI("HwcHal::createClient: Client already exists. Waiting for"
-                    " it to be destroyed.");
-            mClientDestroyedWait.wait_for(lock, 1s,
-                    [this] { return mClient == nullptr; });
-            std::string doneMsg = mClient == nullptr ?
-                    "Existing client was destroyed." :
-                    "Existing client was never destroyed!";
-            ALOGI("HwcHal::createClient: Done waiting. %s", doneMsg.c_str());
-        }
-
-        // only one client is allowed
-        if (mClient == nullptr) {
-            // We assume Composer outlives ComposerClient here.  It is true
-            // only because Composer is binderized.
-            client = ComposerClient::create(this).release();
-            if (client) {
-                mClient = client;
-            } else {
-                err = Error::NO_RESOURCES;
-            }
-        } else {
-            err = Error::NO_RESOURCES;
-        }
-    }
-
-    hidl_cb(err, client);
-
-    return Void();
-}
-
-sp<ComposerClient> HwcHal::getClient()
-{
-    std::lock_guard<std::mutex> lock(mClientMutex);
-    return (mClient != nullptr) ? mClient.promote() : nullptr;
-}
-
-void HwcHal::removeClient()
-{
-    std::lock_guard<std::mutex> lock(mClientMutex);
-    mClient = nullptr;
-    mClientDestroyedWait.notify_all();
+    return buf.data();
 }
 
 void HwcHal::hotplugHook(hwc2_callback_data_t callbackData,
         hwc2_display_t display, int32_t connected)
 {
-    auto hal = reinterpret_cast<HwcHal*>(callbackData);
-    auto client = hal->getClient();
-    if (client != nullptr) {
-        client->onHotplug(display,
-                static_cast<IComposerCallback::Connection>(connected));
-    }
+    auto hal = static_cast<HwcHal*>(callbackData);
+    hal->mEventCallback->onHotplug(display, static_cast<IComposerCallback::Connection>(connected));
 }
 
 void HwcHal::refreshHook(hwc2_callback_data_t callbackData,
         hwc2_display_t display)
 {
-    auto hal = reinterpret_cast<HwcHal*>(callbackData);
+    auto hal = static_cast<HwcHal*>(callbackData);
     hal->mMustValidateDisplay = true;
-
-    auto client = hal->getClient();
-    if (client != nullptr) {
-        client->onRefresh(display);
-    }
+    hal->mEventCallback->onRefresh(display);
 }
 
 void HwcHal::vsyncHook(hwc2_callback_data_t callbackData,
         hwc2_display_t display, int64_t timestamp)
 {
-    auto hal = reinterpret_cast<HwcHal*>(callbackData);
-    auto client = hal->getClient();
-    if (client != nullptr) {
-        client->onVsync(display, timestamp);
-    }
+    auto hal = static_cast<HwcHal*>(callbackData);
+    hal->mEventCallback->onVsync(display, timestamp);
 }
 
-void HwcHal::enableCallback(bool enable)
-{
-    if (enable) {
-        mMustValidateDisplay = true;
+void HwcHal::registerEventCallback(EventCallback* callback) {
+    mMustValidateDisplay = true;
+    mEventCallback = callback;
 
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_HOTPLUG, this,
-                reinterpret_cast<hwc2_function_pointer_t>(hotplugHook));
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_REFRESH, this,
-                reinterpret_cast<hwc2_function_pointer_t>(refreshHook));
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC, this,
-                reinterpret_cast<hwc2_function_pointer_t>(vsyncHook));
-    } else {
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_HOTPLUG, this,
-                nullptr);
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_REFRESH, this,
-                nullptr);
-        mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC, this,
-                nullptr);
-    }
+    mDispatch.registerCallback(mDevice, HWC2_CALLBACK_HOTPLUG, this,
+            reinterpret_cast<hwc2_function_pointer_t>(hotplugHook));
+    mDispatch.registerCallback(mDevice, HWC2_CALLBACK_REFRESH, this,
+            reinterpret_cast<hwc2_function_pointer_t>(refreshHook));
+    mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC, this,
+            reinterpret_cast<hwc2_function_pointer_t>(vsyncHook));
+}
+
+void HwcHal::unregisterEventCallback() {
+    // we assume the callback functions
+    //
+    //  - can be unregistered
+    //  - can be in-flight
+    //  - will never be called afterward
+    //
+    // which is likely incorrect
+    mDispatch.registerCallback(mDevice, HWC2_CALLBACK_HOTPLUG, this, nullptr);
+    mDispatch.registerCallback(mDevice, HWC2_CALLBACK_REFRESH, this, nullptr);
+    mDispatch.registerCallback(mDevice, HWC2_CALLBACK_VSYNC, this, nullptr);
+
+    mEventCallback = nullptr;
 }
 
 uint32_t HwcHal::getMaxVirtualDisplayCount()
@@ -810,7 +720,7 @@
         return nullptr;
     }
 
-    return new HwcHal(module);
+    return Composer::create(std::make_unique<HwcHal>(module)).release();
 }
 
 } // namespace implementation
diff --git a/graphics/composer/2.1/default/Hwc.h b/graphics/composer/2.1/default/Hwc.h
index ce91dd6..835b49e 100644
--- a/graphics/composer/2.1/default/Hwc.h
+++ b/graphics/composer/2.1/default/Hwc.h
@@ -54,20 +54,17 @@
 using android::hardware::graphics::common::V1_0::ColorTransform;
 using android::hardware::graphics::common::V1_0::Hdr;
 
-class HwcHal : public IComposer, public ComposerHal {
+class HwcHal : public ComposerHal {
 public:
     HwcHal(const hw_module_t* module);
     virtual ~HwcHal();
 
-    // IComposer interface
-    Return<void> getCapabilities(getCapabilities_cb hidl_cb) override;
-    Return<void> dumpDebugInfo(dumpDebugInfo_cb hidl_cb) override;
-    Return<void> createClient(createClient_cb hidl_cb) override;
-
-    // ComposerHal interface
     bool hasCapability(hwc2_capability_t capability) override;
-    void removeClient() override;
-    void enableCallback(bool enable) override;
+
+    std::string dumpDebugInfo() override;
+    void registerEventCallback(EventCallback* callback) override;
+    void unregisterEventCallback() override;
+
     uint32_t getMaxVirtualDisplayCount() override;
     Error createVirtualDisplay(uint32_t width, uint32_t height,
         PixelFormat* format, Display* outDisplay) override;
@@ -157,8 +154,6 @@
     void initDispatch(hwc2_function_descriptor_t desc, T* outPfn);
     void initDispatch();
 
-    sp<ComposerClient> getClient();
-
     static void hotplugHook(hwc2_callback_data_t callbackData,
         hwc2_display_t display, int32_t connected);
     static void refreshHook(hwc2_callback_data_t callbackData,
@@ -216,10 +211,6 @@
         HWC2_PFN_VALIDATE_DISPLAY validateDisplay;
     } mDispatch;
 
-    std::mutex mClientMutex;
-    std::condition_variable mClientDestroyedWait;
-    wp<ComposerClient> mClient;
-
     std::atomic<bool> mMustValidateDisplay;
 
     // If the HWC implementation version is < 2.0, use an adapter to interface
@@ -229,6 +220,8 @@
     // If there is no HWC implementation, use an adapter to interface between
     // HWC 2.0 <-> FB HAL.
     std::unique_ptr<HWC2OnFbAdapter> mFbAdapter;
+
+    EventCallback* mEventCallback = nullptr;
 };
 
 extern "C" IComposer* HIDL_FETCH_IComposer(const char* name);
diff --git a/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/Composer.h b/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/Composer.h
new file mode 100644
index 0000000..581dc96
--- /dev/null
+++ b/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/Composer.h
@@ -0,0 +1,157 @@
+/*
+ * Copyright 2016 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.
+ */
+
+#pragma once
+
+#ifndef LOG_TAG
+#warning "Composer.h included without LOG_TAG"
+#endif
+
+#include <array>
+#include <chrono>
+#include <condition_variable>
+#include <memory>
+#include <mutex>
+#include <vector>
+
+#include <android/hardware/graphics/composer/2.1/IComposer.h>
+#include <android/hardware/graphics/composer/2.1/IComposerClient.h>
+#include <composer-hal/2.1/ComposerClient.h>
+#include <composer-hal/2.1/ComposerHal.h>
+
+namespace android {
+namespace hardware {
+namespace graphics {
+namespace composer {
+namespace V2_1 {
+namespace hal {
+
+namespace detail {
+
+// ComposerImpl implements V2_*::IComposer on top of V2_*::ComposerHal
+template <typename Interface, typename Hal>
+class ComposerImpl : public Interface {
+   public:
+    static std::unique_ptr<ComposerImpl> create(std::unique_ptr<Hal> hal) {
+        return std::make_unique<ComposerImpl>(std::move(hal));
+    }
+
+    ComposerImpl(std::unique_ptr<Hal> hal) : mHal(std::move(hal)) {}
+
+    // IComposer 2.1 interface
+
+    Return<void> getCapabilities(IComposer::getCapabilities_cb hidl_cb) override {
+        const std::array<IComposer::Capability, 3> all_caps = {{
+            IComposer::Capability::SIDEBAND_STREAM,
+            IComposer::Capability::SKIP_CLIENT_COLOR_TRANSFORM,
+            IComposer::Capability::PRESENT_FENCE_IS_NOT_RELIABLE,
+        }};
+
+        std::vector<IComposer::Capability> caps;
+        for (auto cap : all_caps) {
+            if (mHal->hasCapability(static_cast<hwc2_capability_t>(cap))) {
+                caps.push_back(cap);
+            }
+        }
+
+        hidl_vec<IComposer::Capability> caps_reply;
+        caps_reply.setToExternal(caps.data(), caps.size());
+        hidl_cb(caps_reply);
+        return Void();
+    }
+
+    Return<void> dumpDebugInfo(IComposer::dumpDebugInfo_cb hidl_cb) override {
+        hidl_cb(mHal->dumpDebugInfo());
+        return Void();
+    }
+
+    Return<void> createClient(IComposer::createClient_cb hidl_cb) override {
+        std::unique_lock<std::mutex> lock(mClientMutex);
+        if (!waitForClientDestroyedLocked(lock)) {
+            hidl_cb(Error::NO_RESOURCES, nullptr);
+            return Void();
+        }
+
+        sp<IComposerClient> client = createClient();
+        if (!client) {
+            hidl_cb(Error::NO_RESOURCES, nullptr);
+            return Void();
+        }
+
+        mClient = client;
+        hidl_cb(Error::NONE, client);
+        return Void();
+    }
+
+   protected:
+    bool waitForClientDestroyedLocked(std::unique_lock<std::mutex>& lock) {
+        if (mClient != nullptr) {
+            using namespace std::chrono_literals;
+
+            // In surface flinger we delete a composer client on one thread and
+            // then create a new client on another thread. Although surface
+            // flinger ensures the calls are made in that sequence (destroy and
+            // then create), sometimes the calls land in the composer service
+            // inverted (create and then destroy). Wait for a brief period to
+            // see if the existing client is destroyed.
+            ALOGD("waiting for previous client to be destroyed");
+            mClientDestroyedCondition.wait_for(
+                lock, 1s, [this]() -> bool { return mClient.promote() == nullptr; });
+            if (mClient.promote() != nullptr) {
+                ALOGD("previous client was not destroyed");
+            } else {
+                mClient.clear();
+            }
+        }
+
+        return mClient == nullptr;
+    }
+
+    void onClientDestroyed() {
+        std::lock_guard<std::mutex> lock(mClientMutex);
+        mClient.clear();
+        mClientDestroyedCondition.notify_all();
+    }
+
+    virtual IComposerClient* createClient() {
+        auto client = ComposerClient::create(mHal.get());
+        if (!client) {
+            return nullptr;
+        }
+
+        auto clientDestroyed = [this]() { onClientDestroyed(); };
+        client->setOnClientDestroyed(clientDestroyed);
+
+        return client.release();
+    }
+
+    const std::unique_ptr<Hal> mHal;
+
+    std::mutex mClientMutex;
+    wp<IComposerClient> mClient;
+    std::condition_variable mClientDestroyedCondition;
+};
+
+}  // namespace detail
+
+using Composer = detail::ComposerImpl<IComposer, ComposerHal>;
+
+}  // namespace hal
+}  // namespace V2_1
+}  // namespace composer
+}  // namespace graphics
+}  // namespace hardware
+}  // namespace android
diff --git a/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/ComposerClient.h b/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/ComposerClient.h
index b8a3f75..86525b8 100644
--- a/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/ComposerClient.h
+++ b/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/ComposerClient.h
@@ -58,9 +58,11 @@
 
         ALOGD("destroying composer client");
 
-        mHal->enableCallback(false);
+        mHal->unregisterEventCallback();
         destroyResources();
-        mHal->removeClient();
+        if (mOnClientDestroyed) {
+            mOnClientDestroyed();
+        }
 
         ALOGD("removed composer client");
     }
@@ -77,34 +79,47 @@
         return true;
     }
 
-    void onHotplug(Display display, IComposerCallback::Connection connected) {
-        if (connected == IComposerCallback::Connection::CONNECTED) {
-            mResources->addPhysicalDisplay(display);
-        } else if (connected == IComposerCallback::Connection::DISCONNECTED) {
-            mResources->removeDisplay(display);
-        }
-
-        auto ret = mCallback->onHotplug(display, connected);
-        ALOGE_IF(!ret.isOk(), "failed to send onHotplug: %s", ret.description().c_str());
-    }
-
-    void onRefresh(Display display) {
-        auto ret = mCallback->onRefresh(display);
-        ALOGE_IF(!ret.isOk(), "failed to send onRefresh: %s", ret.description().c_str());
-    }
-
-    void onVsync(Display display, int64_t timestamp) {
-        auto ret = mCallback->onVsync(display, timestamp);
-        ALOGE_IF(!ret.isOk(), "failed to send onVsync: %s", ret.description().c_str());
+    void setOnClientDestroyed(std::function<void()> onClientDestroyed) {
+        mOnClientDestroyed = onClientDestroyed;
     }
 
     // IComposerClient 2.1 interface
 
+    class HalEventCallback : public Hal::EventCallback {
+       public:
+        HalEventCallback(const sp<IComposerCallback> callback, ComposerResources* resources)
+            : mCallback(callback), mResources(resources) {}
+
+        void onHotplug(Display display, IComposerCallback::Connection connected) {
+            if (connected == IComposerCallback::Connection::CONNECTED) {
+                mResources->addPhysicalDisplay(display);
+            } else if (connected == IComposerCallback::Connection::DISCONNECTED) {
+                mResources->removeDisplay(display);
+            }
+
+            auto ret = mCallback->onHotplug(display, connected);
+            ALOGE_IF(!ret.isOk(), "failed to send onHotplug: %s", ret.description().c_str());
+        }
+
+        void onRefresh(Display display) {
+            auto ret = mCallback->onRefresh(display);
+            ALOGE_IF(!ret.isOk(), "failed to send onRefresh: %s", ret.description().c_str());
+        }
+
+        void onVsync(Display display, int64_t timestamp) {
+            auto ret = mCallback->onVsync(display, timestamp);
+            ALOGE_IF(!ret.isOk(), "failed to send onVsync: %s", ret.description().c_str());
+        }
+
+       protected:
+        const sp<IComposerCallback> mCallback;
+        ComposerResources* const mResources;
+    };
+
     Return<void> registerCallback(const sp<IComposerCallback>& callback) override {
         // no locking as we require this function to be called only once
-        mCallback = callback;
-        mHal->enableCallback(callback != nullptr);
-
+        mHalEventCallback = std::make_unique<HalEventCallback>(callback, mResources.get());
+        mHal->registerEventCallback(mHalEventCallback.get());
         return Void();
     }
 
@@ -365,7 +380,8 @@
     std::mutex mCommandEngineMutex;
     std::unique_ptr<ComposerCommandEngine> mCommandEngine;
 
-    sp<IComposerCallback> mCallback;
+    std::function<void()> mOnClientDestroyed;
+    std::unique_ptr<HalEventCallback> mHalEventCallback;
 };
 
 }  // namespace detail
diff --git a/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/ComposerHal.h b/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/ComposerHal.h
index 3956ecd..c9793fd 100644
--- a/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/ComposerHal.h
+++ b/graphics/composer/2.1/utils/hal/include/composer-hal/2.1/ComposerHal.h
@@ -16,6 +16,9 @@
 
 #pragma once
 
+#include <string>
+#include <vector>
+
 #include <android/hardware/graphics/composer/2.1/IComposer.h>
 #include <android/hardware/graphics/composer/2.1/IComposerClient.h>
 
@@ -40,15 +43,32 @@
 using common::V1_0::PixelFormat;
 using common::V1_0::Transform;
 
-// TODO remove removeClient and better support for callback
 class ComposerHal {
    public:
     virtual ~ComposerHal() = default;
 
     virtual bool hasCapability(hwc2_capability_t capability) = 0;
 
-    virtual void removeClient() = 0;
-    virtual void enableCallback(bool enable) = 0;
+    // dump the debug information
+    virtual std::string dumpDebugInfo() = 0;
+
+    class EventCallback {
+       public:
+        virtual ~EventCallback() = default;
+        virtual void onHotplug(Display display, IComposerCallback::Connection connected) = 0;
+        virtual void onRefresh(Display display) = 0;
+        virtual void onVsync(Display display, int64_t timestamp) = 0;
+    };
+
+    // Register the event callback object.  The event callback object is valid
+    // until it is unregistered.  A hotplug event must be generated for each
+    // connected physical display, before or after this function returns.
+    virtual void registerEventCallback(EventCallback* callback) = 0;
+
+    // Unregister the event callback object.  It must block if there is any
+    // callback in-flight.
+    virtual void unregisterEventCallback() = 0;
+
     virtual uint32_t getMaxVirtualDisplayCount() = 0;
     virtual Error createVirtualDisplay(uint32_t width, uint32_t height, PixelFormat* format,
                                        Display* outDisplay) = 0;