more clean-up of Surfaceflinger's client management

SurfaceComposerClient now only exist on the WindowManager side,
the client side uses the new SurfaceClient class, which only
exposes what a client needs.

also instead of keeping mappings from IBinder to SurfaceComposerClients
we have a SurfaceClient per Surface (referring to the same IBinder), this
is made possible by the fact that SurfaceClient is very light.

Change-Id: I6a1f7015424f07871632a25ed6a502c55abfcfa6

2 files changed, 170 insertions, 238 deletions

diff --git a/libs/surfaceflinger_client/Surface.cpp b/libs/surfaceflinger_client/Surface.cpp
index a26733e..35a4e8b 100644
--- a/libs/surfaceflinger_client/Surface.cpp
+++ b/libs/surfaceflinger_client/Surface.cpp
@@ -278,14 +278,16 @@ sp<Surface> SurfaceControl::getSurface() const
 //  Surface
 // ============================================================================
 
+
 Surface::Surface(const sp<SurfaceControl>& surface)
-    : mClient(surface->mClient), mSurface(surface->mSurface),
+    : mSurface(surface->mSurface),
       mToken(surface->mToken), mIdentity(surface->mIdentity),
       mFormat(surface->mFormat), mFlags(surface->mFlags),
       mBufferMapper(GraphicBufferMapper::get()), mSharedBufferClient(NULL),
       mInitCheck(NO_INIT),
       mWidth(surface->mWidth), mHeight(surface->mHeight)
 {
+    mClient = new SurfaceClient(surface->mClient);
     init();
 }
 
@@ -293,7 +295,7 @@ Surface::Surface(const Parcel& parcel)
     :  mBufferMapper(GraphicBufferMapper::get()),
        mSharedBufferClient(NULL), mInitCheck(NO_INIT)
 {
-    sp<IBinder> clientBinder = parcel.readStrongBinder();
+    sp<IBinder> conn = parcel.readStrongBinder();
     mSurface    = interface_cast<ISurface>(parcel.readStrongBinder());
     mToken      = parcel.readInt32();
     mIdentity   = parcel.readInt32();
@@ -301,12 +303,7 @@ Surface::Surface(const Parcel& parcel)
     mHeight     = parcel.readInt32();
     mFormat     = parcel.readInt32();
     mFlags      = parcel.readInt32();
-
-    // FIXME: what does that mean if clientBinder is NULL here?
-    if (clientBinder != NULL) {
-        mClient = SurfaceComposerClient::clientForConnection(clientBinder);
-    }
-
+    mClient = new SurfaceClient(conn);
     init();
 }
 
@@ -334,7 +331,7 @@ void Surface::init()
     mBuffers.setCapacity(2);
     mBuffers.insertAt(0, 2);
 
-    if (mClient != 0) {
+    if (mClient != 0 && mClient->initCheck() == NO_ERROR) {
         mSharedBufferClient = new SharedBufferClient(
                 mClient->getSharedClient(), mToken, 2, mIdentity);
     }
@@ -364,7 +361,7 @@ Surface::~Surface()
 
 status_t Surface::initCheck() const
 {
-    if (mToken<0 || mClient==0) {
+    if (mToken<0 || mClient==0 || mClient->initCheck() != NO_ERROR) {
         return NO_INIT;
     }
     SharedClient const* cblk = mClient->getSharedClient();
@@ -565,8 +562,7 @@ int Surface::queueBuffer(android_native_buffer_t* buffer)
 
     if (err == NO_ERROR) {
         // FIXME: can we avoid this IPC if we know there is one pending?
-        const sp<SurfaceComposerClient>& client(mClient);
-        client->signalServer();
+        mClient->signalServer();
     }
     return err;
 }
diff --git a/libs/surfaceflinger_client/SurfaceComposerClient.cpp b/libs/surfaceflinger_client/SurfaceComposerClient.cpp
index 96ed566..8d39c85 100644
--- a/libs/surfaceflinger_client/SurfaceComposerClient.cpp
+++ b/libs/surfaceflinger_client/SurfaceComposerClient.cpp
@@ -17,25 +17,18 @@
 #define LOG_TAG "SurfaceComposerClient"
 
 #include <stdint.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <errno.h>
 #include <sys/types.h>
-#include <sys/stat.h>
 
-#include <cutils/memory.h>
-
-#include <utils/Atomic.h>
 #include <utils/Errors.h>
 #include <utils/threads.h>
-#include <utils/KeyedVector.h>
+#include <utils/SortedVector.h>
 #include <utils/Log.h>
+#include <utils/Singleton.h>
 
 #include <binder/IServiceManager.h>
 #include <binder/IMemory.h>
 
 #include <ui/DisplayInfo.h>
-#include <ui/Rect.h>
 
 #include <surfaceflinger/ISurfaceComposer.h>
 #include <surfaceflinger/ISurfaceFlingerClient.h>
@@ -45,69 +38,110 @@
 #include <private/surfaceflinger/LayerState.h>
 #include <private/surfaceflinger/SharedBufferStack.h>
 
-#define VERBOSE(...)	((void)0)
-//#define VERBOSE			LOGD
-
-#define LIKELY( exp )       (__builtin_expect( (exp) != 0, true  ))
-#define UNLIKELY( exp )     (__builtin_expect( (exp) != 0, false ))
 
 namespace android {
-
 // ---------------------------------------------------------------------------
 
-// Must not be holding SurfaceComposerClient::mLock when acquiring gLock here.
-static Mutex                                                gLock;
-static sp<ISurfaceComposer>                                 gSurfaceManager;
-static DefaultKeyedVector< sp<IBinder>, wp<SurfaceComposerClient> > gActiveConnections;
-static SortedVector<sp<SurfaceComposerClient> >             gOpenTransactions;
-static sp<IMemoryHeap>                                      gServerCblkMemory;
-static volatile surface_flinger_cblk_t*                     gServerCblk;
-
-static sp<ISurfaceComposer> getComposerService()
-{
-    sp<ISurfaceComposer> sc;
-    Mutex::Autolock _l(gLock);
-    if (gSurfaceManager != 0) {
-        sc = gSurfaceManager;
-    } else {
-        // release the lock while we're waiting...
-        gLock.unlock();
-
-        sp<IBinder> binder;
-        sp<IServiceManager> sm = defaultServiceManager();
-        do {
-            binder = sm->getService(String16("SurfaceFlinger"));
-            if (binder == 0) {
-                LOGW("SurfaceFlinger not published, waiting...");
-                usleep(500000); // 0.5 s
-            }
-        } while(binder == 0);
-
-        // grab the lock again for updating gSurfaceManager
-        gLock.lock();
-        if (gSurfaceManager == 0) {
-            sc = interface_cast<ISurfaceComposer>(binder);
-            gSurfaceManager = sc;
-        } else {
-            sc = gSurfaceManager;
+class Composer : public Singleton<Composer>
+{
+    // these are constants
+    sp<ISurfaceComposer> mComposerService;
+    sp<IMemoryHeap> mServerCblkMemory;
+    surface_flinger_cblk_t volatile* mServerCblk;
+
+    Mutex mLock;
+    SortedVector< wp<SurfaceComposerClient> > mActiveConnections;
+    SortedVector<sp<SurfaceComposerClient> > mOpenTransactions;
+
+    Composer() : Singleton<Composer>() {
+        const String16 name("SurfaceFlinger");
+        while (getService(name, &mComposerService) != NO_ERROR) {
+            usleep(250000);
         }
+        mServerCblkMemory = mComposerService->getCblk();
+        mServerCblk = static_cast<surface_flinger_cblk_t volatile *>(
+                mServerCblkMemory->getBase());
     }
-    return sc;
-}
 
-static volatile surface_flinger_cblk_t const * get_cblk()
-{
-    if (gServerCblk == 0) {
-        sp<ISurfaceComposer> sm(getComposerService());
-        Mutex::Autolock _l(gLock);
-        if (gServerCblk == 0) {
-            gServerCblkMemory = sm->getCblk();
-            LOGE_IF(gServerCblkMemory==0, "Can't get server control block");
-            gServerCblk = (surface_flinger_cblk_t *)gServerCblkMemory->getBase();
-            LOGE_IF(gServerCblk==0, "Can't get server control block address");
+    void addClientImpl(const sp<SurfaceComposerClient>& client) {
+        Mutex::Autolock _l(mLock);
+        mActiveConnections.add(client);
+    }
+
+    void removeClientImpl(const sp<SurfaceComposerClient>& client) {
+        Mutex::Autolock _l(mLock);
+        mActiveConnections.remove(client);
+    }
+
+    void openGlobalTransactionImpl()
+    {
+        Mutex::Autolock _l(mLock);
+        if (mOpenTransactions.size()) {
+            LOGE("openGlobalTransaction() called more than once. skipping.");
+            return;
         }
+        const size_t N = mActiveConnections.size();
+        for (size_t i=0; i<N; i++) {
+            sp<SurfaceComposerClient> client(mActiveConnections[i].promote());
+            if (client != 0 && mOpenTransactions.indexOf(client) < 0) {
+                if (client->openTransaction() == NO_ERROR) {
+                    mOpenTransactions.add(client);
+                } else {
+                    LOGE("openTransaction on client %p failed", client.get());
+                    // let it go, it'll fail later when the user
+                    // tries to do something with the transaction
+                }
+            }
+        }
+    }
+
+    void closeGlobalTransactionImpl()
+    {
+        mLock.lock();
+            SortedVector< sp<SurfaceComposerClient> > clients(mOpenTransactions);
+            mOpenTransactions.clear();
+        mLock.unlock();
+
+        sp<ISurfaceComposer> sm(mComposerService);
+        sm->openGlobalTransaction();
+            const size_t N = clients.size();
+            for (size_t i=0; i<N; i++) {
+                clients[i]->closeTransaction();
+            }
+        sm->closeGlobalTransaction();
+    }
+
+    friend class Singleton<Composer>;
+
+public:
+    static sp<ISurfaceComposer> getComposerService() {
+        return Composer::getInstance().mComposerService;
+    }
+    static surface_flinger_cblk_t const volatile * getControlBlock() {
+        return Composer::getInstance().mServerCblk;
+    }
+    static void addClient(const sp<SurfaceComposerClient>& client) {
+        Composer::getInstance().addClientImpl(client);
+    }
+    static void removeClient(const sp<SurfaceComposerClient>& client) {
+        Composer::getInstance().removeClientImpl(client);
+    }
+    static void openGlobalTransaction() {
+        Composer::getInstance().openGlobalTransactionImpl();
+    }
+    static void closeGlobalTransaction() {
+        Composer::getInstance().closeGlobalTransactionImpl();
     }
-    return gServerCblk;
+};
+
+ANDROID_SINGLETON_STATIC_INSTANCE(Composer);
+
+static inline sp<ISurfaceComposer> getComposerService() {
+    return Composer::getComposerService();
+}
+
+static inline surface_flinger_cblk_t const volatile * get_cblk() {
+    return Composer::getControlBlock();
 }
 
 // ---------------------------------------------------------------------------
@@ -120,66 +154,28 @@ static inline int compare_type( const layer_state_t& lhs,
 }
 
 SurfaceComposerClient::SurfaceComposerClient()
+    : mTransactionOpen(0), mPrebuiltLayerState(0), mStatus(NO_INIT)
 {
-    sp<ISurfaceComposer> sm(getComposerService());
-    if (sm == 0) {
-        init(0, 0);
-        return;
-    }
-
-    init(sm, sm->createConnection());
-
-    if (mClient != 0) {
-        Mutex::Autolock _l(gLock);
-        VERBOSE("Adding client %p to map", this);
-        gActiveConnections.add(mClient->asBinder(), this);
-    }
-}
-
-SurfaceComposerClient::SurfaceComposerClient(
-        const sp<ISurfaceComposer>& sm, const sp<IBinder>& conn)
-{
-    init(sm, interface_cast<ISurfaceFlingerClient>(conn));
 }
 
-SurfaceComposerClient::~SurfaceComposerClient()
-{
-    VERBOSE("Destroying client %p, conn %p", this, mClient.get());
-    dispose();
-}
-
-status_t SurfaceComposerClient::linkToComposerDeath(
-        const sp<IBinder::DeathRecipient>& recipient,
-        void* cookie, uint32_t flags)
+void SurfaceComposerClient::onFirstRef()
 {
     sp<ISurfaceComposer> sm(getComposerService());
-    return sm->asBinder()->linkToDeath(recipient, cookie, flags);    
-}
-
-void SurfaceComposerClient::init(
-        const sp<ISurfaceComposer>& sm, const sp<ISurfaceFlingerClient>& conn)
-{
-    VERBOSE("Creating client %p, conn %p", this, conn.get());
-
-    mPrebuiltLayerState = 0;
-    mTransactionOpen = 0;
-    mStatus = NO_ERROR;
-    mControl = 0;
-
-    mClient = conn;
-    if (mClient == 0) {
-        mStatus = NO_INIT;
-        return;
+    if (sm != 0) {
+        sp<ISurfaceFlingerClient> conn = sm->createConnection();
+        if (conn != 0) {
+            mClient = conn;
+            Composer::addClient(this);
+            mPrebuiltLayerState = new layer_state_t;
+            mStatus = NO_ERROR;
+        }
     }
-
-    mControlMemory = mClient->getControlBlock();
-    mSignalServer = sm;
-    mControl = static_cast<SharedClient *>(mControlMemory->getBase());
 }
 
-SharedClient* SurfaceComposerClient::getSharedClient() const
+SurfaceComposerClient::~SurfaceComposerClient()
 {
-    return mControl;
+    delete mPrebuiltLayerState;
+    dispose();
 }
 
 status_t SurfaceComposerClient::initCheck() const
@@ -192,63 +188,25 @@ sp<IBinder> SurfaceComposerClient::connection() const
     return (mClient != 0) ? mClient->asBinder() : 0;
 }
 
-sp<SurfaceComposerClient>
-SurfaceComposerClient::clientForConnection(const sp<IBinder>& conn)
+status_t SurfaceComposerClient::linkToComposerDeath(
+        const sp<IBinder::DeathRecipient>& recipient,
+        void* cookie, uint32_t flags)
 {
-    sp<SurfaceComposerClient> client;
-
-    { // scope for lock
-        Mutex::Autolock _l(gLock);
-        client = gActiveConnections.valueFor(conn).promote();
-    }
-
-    if (client == 0) {
-        // Need to make a new client.
-        sp<ISurfaceComposer> sm(getComposerService());
-        client = new SurfaceComposerClient(sm, conn);
-        if (client != 0 && client->initCheck() == NO_ERROR) {
-            Mutex::Autolock _l(gLock);
-            gActiveConnections.add(conn, client);
-            //LOGD("we have %d connections", gActiveConnections.size());
-        } else {
-            client.clear();
-        }
-    }
-
-    return client;
+    sp<ISurfaceComposer> sm(getComposerService());
+    return sm->asBinder()->linkToDeath(recipient, cookie, flags);
 }
 
 void SurfaceComposerClient::dispose()
 {
     // this can be called more than once.
-
-    sp<IMemoryHeap>             controlMemory;
-    sp<ISurfaceFlingerClient>   client;
-
-    {
-        Mutex::Autolock _lg(gLock);
-        Mutex::Autolock _lm(mLock);
-
-        mSignalServer = 0;
-
-        if (mClient != 0) {
-            client = mClient;
-            mClient.clear();
-
-            ssize_t i = gActiveConnections.indexOfKey(client->asBinder());
-            if (i >= 0 && gActiveConnections.valueAt(i) == this) {
-                VERBOSE("Removing client %p from map at %d", this, int(i));
-                gActiveConnections.removeItemsAt(i);
-            }
-        }
-
-        delete mPrebuiltLayerState;
-        mPrebuiltLayerState = 0;
-        controlMemory = mControlMemory;
-        mControlMemory.clear();
-        mControl = 0;
-        mStatus = NO_INIT;
+    sp<ISurfaceFlingerClient> client;
+    Mutex::Autolock _lm(mLock);
+    if (mClient != 0) {
+        Composer::removeClient(this);
+        client = mClient; // hold ref while lock is held
+        mClient.clear();
     }
+    mStatus = NO_INIT;
 }
 
 status_t SurfaceComposerClient::getDisplayInfo(
@@ -309,12 +267,6 @@ ssize_t SurfaceComposerClient::getNumberOfDisplays()
     return n;
 }
 
-
-void SurfaceComposerClient::signalServer()
-{
-    mSignalServer->signal();
-}
-
 sp<SurfaceControl> SurfaceComposerClient::createSurface(
         int pid,
         DisplayID display,
@@ -331,7 +283,6 @@ sp<SurfaceControl> SurfaceComposerClient::createSurface(
 
     return SurfaceComposerClient::createSurface(pid, name, display,
             w, h, format, flags);
-
 }
 
 sp<SurfaceControl> SurfaceComposerClient::createSurface(
@@ -377,56 +328,14 @@ status_t SurfaceComposerClient::destroySurface(SurfaceID sid)
 
 void SurfaceComposerClient::openGlobalTransaction()
 {
-    Mutex::Autolock _l(gLock);
-
-    if (gOpenTransactions.size()) {
-        LOGE("openGlobalTransaction() called more than once. skipping.");
-        return;
-    }
-
-    const size_t N = gActiveConnections.size();
-    VERBOSE("openGlobalTransaction (%ld clients)", N);
-    for (size_t i=0; i<N; i++) {
-        sp<SurfaceComposerClient> client(gActiveConnections.valueAt(i).promote());
-        if (client != 0 && gOpenTransactions.indexOf(client) < 0) {
-            if (client->openTransaction() == NO_ERROR) {
-                if (gOpenTransactions.add(client) < 0) {
-                    // Ooops!
-                    LOGE(   "Unable to add a SurfaceComposerClient "
-                            "to the global transaction set (out of memory?)");
-                    client->closeTransaction();
-                    // let it go, it'll fail later when the user
-                    // tries to do something with the transaction
-                }
-            } else {
-                LOGE("openTransaction on client %p failed", client.get());
-                // let it go, it'll fail later when the user
-                // tries to do something with the transaction
-            }
-        }
-    }
+    Composer::openGlobalTransaction();
 }
 
 void SurfaceComposerClient::closeGlobalTransaction()
 {
-    gLock.lock();
-        SortedVector< sp<SurfaceComposerClient> > clients(gOpenTransactions);
-        gOpenTransactions.clear();
-    gLock.unlock();
-
-    const size_t N = clients.size();
-    VERBOSE("closeGlobalTransaction (%ld clients)", N);
-
-    sp<ISurfaceComposer> sm(getComposerService());
-    sm->openGlobalTransaction();
-    for (size_t i=0; i<N; i++) {
-        clients[i]->closeTransaction();
-    }
-    sm->closeGlobalTransaction();
-
+    Composer::closeGlobalTransaction();
 }
 
-
 status_t SurfaceComposerClient::freezeDisplay(DisplayID dpy, uint32_t flags)
 {
     sp<ISurfaceComposer> sm(getComposerService());
@@ -451,26 +360,16 @@ status_t SurfaceComposerClient::openTransaction()
     if (mStatus != NO_ERROR)
         return mStatus;
     Mutex::Autolock _l(mLock);
-    VERBOSE(   "openTransaction (client %p, mTransactionOpen=%d)",
-            this, mTransactionOpen);
     mTransactionOpen++;
-    if (mPrebuiltLayerState == 0) {
-        mPrebuiltLayerState = new layer_state_t;
-    }
     return NO_ERROR;
 }
 
-
 status_t SurfaceComposerClient::closeTransaction()
 {
     if (mStatus != NO_ERROR)
         return mStatus;
 
     Mutex::Autolock _l(mLock);
-
-    VERBOSE(   "closeTransaction (client %p, mTransactionOpen=%d)",
-            this, mTransactionOpen);
-
     if (mTransactionOpen <= 0) {
         LOGE(   "closeTransaction (client %p, mTransactionOpen=%d) "
                 "called more times than openTransaction()",
@@ -502,7 +401,7 @@ layer_state_t* SurfaceComposerClient::get_state_l(SurfaceID index)
     }
 
     // use mPrebuiltLayerState just to find out if we already have it
-    layer_state_t& dummy = *mPrebuiltLayerState;
+    layer_state_t& dummy(*mPrebuiltLayerState);
     dummy.surface = index;
     ssize_t i = mStates.indexOf(dummy);
     if (i < 0) {
@@ -642,5 +541,42 @@ status_t SurfaceComposerClient::setFreezeTint(SurfaceID id, uint32_t tint)
     return NO_ERROR;
 }
 
+// ----------------------------------------------------------------------------
+
+SurfaceClient::SurfaceClient(const sp<SurfaceComposerClient>& client)
+    : mStatus(NO_INIT), mControl(0)
+{
+    if (client != 0) {
+        sp<IBinder> conn = client->connection();
+        init(conn);
+    }
+}
+SurfaceClient::SurfaceClient(const sp<IBinder>& conn)
+    : mStatus(NO_INIT), mControl(0)
+{
+    init(conn);
+}
+void SurfaceClient::init(const sp<IBinder>& conn)
+{
+    mSignalServer = getComposerService();
+    sp<ISurfaceFlingerClient> sf(interface_cast<ISurfaceFlingerClient>(conn));
+    if (sf != 0) {
+        mConnection = conn;
+        mControlMemory = sf->getControlBlock();
+        mControl = static_cast<SharedClient *>(mControlMemory->getBase());
+        mStatus = NO_ERROR;
+    }
+}
+status_t SurfaceClient::initCheck() const {
+    return mStatus;
+}
+SharedClient* SurfaceClient::getSharedClient() const {
+    return mControl;
+}
+void SurfaceClient::signalServer() const {
+    mSignalServer->signal();
+}
+
+// ----------------------------------------------------------------------------
 }; // namespace android