commit 5ee65f0d441ca558bc95b60c1468f2aadfeeddbd
author Jamie Gennis <jgennis@google.com> Thu Jul 15 17:29:15 2010 -0700
committer Jamie Gennis <jgennis@google.com> Fri Jul 16 13:03:15 2010 -0700
tree fe143a0721ac44a3d2c34a46ad5240ea2d586f76
parent eec69d2923636b2aaa51df93bacc2b3bbb742736

Re-use existing Surface objects when reading them from parcels.

This change adds a process-global cache of previously deserialized Surface
objects so that if a Surface object wrapping the same ISurface gets received
again the same Surface can be used.  This is important because the 'tail'
pointer in the SharedBufferClient is stored only on the client side, and needs
to be the same for all the Surface objects wrapping an ISurface instance.  This
solves the problem by making there only be one Surface object wrapping an
ISurface per process.

Change-Id: I4bf0b8787885c56277622fca053022d2bb638902

core/jni/android_view_Surface.cpp

diff --git a/core/jni/android_view_Surface.cpp b/core/jni/android_view_Surface.cpp
index 7305032..c4d6d1b 100644
--- a/core/jni/android_view_Surface.cpp
+++ b/core/jni/android_view_Surface.cpp
@@ -237,7 +237,7 @@
         return;
     }
 
-    sp<Surface> sur(Surface::readFromParcel(*parcel, 0));
+    sp<Surface> sur(Surface::readFromParcel(*parcel));
     setSurface(env, clazz, sur);
 }
 
@@ -616,8 +616,7 @@
         return;
     }
 
-    const sp<Surface>& curr(getSurface(env, clazz));
-    sp<Surface> sur(Surface::readFromParcel(*parcel, curr));
+    sp<Surface> sur(Surface::readFromParcel(*parcel));
     setSurface(env, clazz, sur);
 }
 
@@ -729,4 +728,3 @@
 }
 
 };
-

include/surfaceflinger/Surface.h

diff --git a/include/surfaceflinger/Surface.h b/include/surfaceflinger/Surface.h
index 4fd0681..294c867 100644
--- a/include/surfaceflinger/Surface.h
+++ b/include/surfaceflinger/Surface.h
@@ -20,6 +20,7 @@
 #include <stdint.h>
 #include <sys/types.h>
 
+#include <utils/KeyedVector.h>
 #include <utils/RefBase.h>
 #include <utils/threads.h>
 
@@ -147,8 +148,7 @@
     static status_t writeToParcel(
             const sp<Surface>& control, Parcel* parcel);
 
-    static sp<Surface> readFromParcel(
-            const Parcel& data, const sp<Surface>& other);
+    static sp<Surface> readFromParcel(const Parcel& data);
 
     static bool isValid(const sp<Surface>& surface) {
         return (surface != 0) && surface->isValid();
@@ -244,6 +244,8 @@
             uint32_t *pWidth, uint32_t *pHeight,
             uint32_t *pFormat, uint32_t *pUsage) const;
 
+    static void cleanCachedSurfaces();
+
     class BufferInfo {
         uint32_t mWidth;
         uint32_t mHeight;
@@ -298,6 +300,10 @@
     // Inherently thread-safe
     mutable Mutex               mSurfaceLock;
     mutable Mutex               mApiLock;
+
+    // A cache of Surface objects that have been deserialized into this process.
+    static Mutex sCachedSurfacesLock;
+    static DefaultKeyedVector<wp<IBinder>, wp<Surface> > sCachedSurfaces;
 };
 
 }; // namespace android

libs/surfaceflinger_client/Surface.cpp

diff --git a/libs/surfaceflinger_client/Surface.cpp b/libs/surfaceflinger_client/Surface.cpp
index dc6332c..1de3a4f 100644
--- a/libs/surfaceflinger_client/Surface.cpp
+++ b/libs/surfaceflinger_client/Surface.cpp
@@ -374,15 +374,36 @@
 
 }
 
-sp<Surface> Surface::readFromParcel(
-        const Parcel& data, const sp<Surface>& other)
-{
-    sp<Surface> result(other);
+
+Mutex Surface::sCachedSurfacesLock;
+DefaultKeyedVector<wp<IBinder>, wp<Surface> > Surface::sCachedSurfaces(wp<Surface>(0));
+
+sp<Surface> Surface::readFromParcel(const Parcel& data) {
+    Mutex::Autolock _l(sCachedSurfacesLock);
     sp<IBinder> binder(data.readStrongBinder());
-    if (other==0 || binder != other->mSurface->asBinder()) {
-        result = new Surface(data, binder);
+    sp<Surface> surface = sCachedSurfaces.valueFor(binder).promote();
+    if (surface == 0) {
+       surface = new Surface(data, binder);
+       sCachedSurfaces.add(binder, surface);
+    } else {
+        LOGW("Reusing surface!");
     }
-    return result;
+    if (surface->mSurface == 0) {
+      surface = 0;
+    }
+    cleanCachedSurfaces();
+    return surface;
+}
+
+// Remove the stale entries from the surface cache.  This should only be called
+// with sCachedSurfacesLock held.
+void Surface::cleanCachedSurfaces() {
+    for (int i = sCachedSurfaces.size()-1; i >= 0; --i) {
+        wp<Surface> s(sCachedSurfaces.valueAt(i));
+        if (s == 0 || s.promote() == 0) {
+            sCachedSurfaces.removeItemsAt(i);
+        }
+    }
 }
 
 void Surface::init()