Merge "Remove Debug Code"
diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h
index c24c0db..0af7e6a 100644
--- a/include/utils/RefBase.h
+++ b/include/utils/RefBase.h
@@ -112,10 +112,24 @@
getWeakRefs()->trackMe(enable, retain);
}
+ // used to override the RefBase destruction.
+ class Destroyer {
+ friend class RefBase;
+ friend class weakref_type;
+ public:
+ virtual ~Destroyer();
+ private:
+ virtual void destroy(RefBase const* base) = 0;
+ };
+
+ // Make sure to never acquire a strong reference from this function. The
+ // same restrictions than for destructors apply.
+ void setDestroyer(Destroyer* destroyer);
+
protected:
RefBase();
virtual ~RefBase();
-
+
//! Flags for extendObjectLifetime()
enum {
OBJECT_LIFETIME_WEAK = 0x0001,
diff --git a/libs/utils/RefBase.cpp b/libs/utils/RefBase.cpp
index d28b751..545da7d 100644
--- a/libs/utils/RefBase.cpp
+++ b/libs/utils/RefBase.cpp
@@ -48,6 +48,11 @@
// ---------------------------------------------------------------------------
+RefBase::Destroyer::~Destroyer() {
+}
+
+// ---------------------------------------------------------------------------
+
class RefBase::weakref_impl : public RefBase::weakref_type
{
public:
@@ -55,7 +60,7 @@
volatile int32_t mWeak;
RefBase* const mBase;
volatile int32_t mFlags;
-
+ Destroyer* mDestroyer;
#if !DEBUG_REFS
@@ -64,6 +69,7 @@
, mWeak(0)
, mBase(base)
, mFlags(0)
+ , mDestroyer(0)
{
}
@@ -310,7 +316,11 @@
if (c == 1) {
const_cast<RefBase*>(this)->onLastStrongRef(id);
if ((refs->mFlags&OBJECT_LIFETIME_WEAK) != OBJECT_LIFETIME_WEAK) {
- delete this;
+ if (refs->mDestroyer) {
+ refs->mDestroyer->destroy(this);
+ } else {
+ delete this;
+ }
}
}
refs->removeWeakRef(id);
@@ -345,7 +355,9 @@
return mRefs->mStrong;
}
-
+void RefBase::setDestroyer(RefBase::Destroyer* destroyer) {
+ mRefs->mDestroyer = destroyer;
+}
RefBase* RefBase::weakref_type::refBase() const
{
@@ -369,16 +381,28 @@
if (c != 1) return;
if ((impl->mFlags&OBJECT_LIFETIME_WEAK) != OBJECT_LIFETIME_WEAK) {
- if (impl->mStrong == INITIAL_STRONG_VALUE)
- delete impl->mBase;
- else {
-// LOGV("Freeing refs %p of old RefBase %p\n", this, impl->mBase);
+ if (impl->mStrong == INITIAL_STRONG_VALUE) {
+ if (impl->mBase) {
+ if (impl->mDestroyer) {
+ impl->mDestroyer->destroy(impl->mBase);
+ } else {
+ delete impl->mBase;
+ }
+ }
+ } else {
+ // LOGV("Freeing refs %p of old RefBase %p\n", this, impl->mBase);
delete impl;
}
} else {
impl->mBase->onLastWeakRef(id);
if ((impl->mFlags&OBJECT_LIFETIME_FOREVER) != OBJECT_LIFETIME_FOREVER) {
- delete impl->mBase;
+ if (impl->mBase) {
+ if (impl->mDestroyer) {
+ impl->mDestroyer->destroy(impl->mBase);
+ } else {
+ delete impl->mBase;
+ }
+ }
}
}
}
@@ -502,10 +526,10 @@
RefBase::~RefBase()
{
-// LOGV("Destroying RefBase %p (refs %p)\n", this, mRefs);
- if (mRefs->mWeak == 0) {
-// LOGV("Freeing refs %p of old RefBase %p\n", mRefs, this);
- delete mRefs;
+ if ((mRefs->mFlags & OBJECT_LIFETIME_WEAK) == OBJECT_LIFETIME_WEAK) {
+ if (mRefs->mWeak == 0) {
+ delete mRefs;
+ }
}
}
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index dfee803..da06f61 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -60,6 +60,7 @@
mWidth(0), mHeight(0), mNeedsScaling(false), mFixedSize(false),
mBypassState(false)
{
+ setDestroyer(this);
}
Layer::~Layer()
@@ -76,6 +77,10 @@
}
}
+void Layer::destroy(RefBase const* base) {
+ mFlinger->destroyLayer(static_cast<LayerBase const*>(base));
+}
+
status_t Layer::setToken(const sp<UserClient>& userClient,
SharedClient* sharedClient, int32_t token)
{
@@ -123,22 +128,6 @@
return mSurface;
}
-status_t Layer::ditch()
-{
- // NOTE: Called from the main UI thread
-
- // the layer is not on screen anymore. free as much resources as possible
- mFreezeLock.clear();
-
- EGLDisplay dpy(mFlinger->graphicPlane(0).getEGLDisplay());
- mBufferManager.destroy(dpy);
- mSurface.clear();
-
- Mutex::Autolock _l(mLock);
- mWidth = mHeight = 0;
- return NO_ERROR;
-}
-
status_t Layer::setBuffers( uint32_t w, uint32_t h,
PixelFormat format, uint32_t flags)
{
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 7bb207a..2c4f756 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -44,7 +44,7 @@
// ---------------------------------------------------------------------------
-class Layer : public LayerBaseClient
+class Layer : public LayerBaseClient, private RefBase::Destroyer
{
public:
Layer(SurfaceFlinger* flinger, DisplayID display,
@@ -78,7 +78,6 @@
virtual bool needsFiltering() const;
virtual bool isSecure() const { return mSecure; }
virtual sp<Surface> createSurface() const;
- virtual status_t ditch();
virtual void onRemoved();
virtual bool setBypass(bool enable);
@@ -95,6 +94,7 @@
return mFreezeLock; }
protected:
+ virtual void destroy(RefBase const* base);
virtual void dump(String8& result, char* scratch, size_t size) const;
private:
diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp
index 21c36e1..3986fde 100644
--- a/services/surfaceflinger/LayerBase.cpp
+++ b/services/surfaceflinger/LayerBase.cpp
@@ -583,10 +583,7 @@
*/
// destroy client resources
- sp<LayerBaseClient> layer = getOwner();
- if (layer != 0) {
- mFlinger->destroySurface(layer);
- }
+ mFlinger->destroySurface(mOwner);
}
sp<LayerBaseClient> LayerBaseClient::Surface::getOwner() const {
diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h
index afc5ec8..e69cb6a 100644
--- a/services/surfaceflinger/LayerBase.h
+++ b/services/surfaceflinger/LayerBase.h
@@ -196,10 +196,6 @@
*/
virtual bool isSecure() const { return false; }
- /** Called from the main thread, when the surface is removed from the
- * draw list */
- virtual status_t ditch() { return NO_ERROR; }
-
/** called with the state lock when the surface is removed from the
* current list */
virtual void onRemoved() { };
@@ -264,7 +260,8 @@
volatile int32_t mInvalidate;
-protected:
+public:
+ // called from class SurfaceFlinger
virtual ~LayerBase();
private:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index c08e2c9..a93d756 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -358,6 +358,9 @@
{
waitForEvent();
+ // call Layer's destructor
+ handleDestroyLayers();
+
// check for transactions
if (UNLIKELY(mConsoleSignals)) {
handleConsoleEvents();
@@ -366,7 +369,7 @@
if (LIKELY(mTransactionCount == 0)) {
// if we're in a global transaction, don't do anything.
const uint32_t mask = eTransactionNeeded | eTraversalNeeded;
- uint32_t transactionFlags = getTransactionFlags(mask);
+ uint32_t transactionFlags = peekTransactionFlags(mask);
if (LIKELY(transactionFlags)) {
handleTransaction(transactionFlags);
}
@@ -467,37 +470,26 @@
void SurfaceFlinger::handleTransaction(uint32_t transactionFlags)
{
- Vector< sp<LayerBase> > ditchedLayers;
+ Mutex::Autolock _l(mStateLock);
+ const nsecs_t now = systemTime();
+ mDebugInTransaction = now;
- /*
- * Perform and commit the transaction
- */
+ // Here we're guaranteed that some transaction flags are set
+ // so we can call handleTransactionLocked() unconditionally.
+ // We call getTransactionFlags(), which will also clear the flags,
+ // with mStateLock held to guarantee that mCurrentState won't change
+ // until the transaction is committed.
- { // scope for the lock
- Mutex::Autolock _l(mStateLock);
- const nsecs_t now = systemTime();
- mDebugInTransaction = now;
- handleTransactionLocked(transactionFlags, ditchedLayers);
- mLastTransactionTime = systemTime() - now;
- mDebugInTransaction = 0;
- // here the transaction has been committed
- }
+ const uint32_t mask = eTransactionNeeded | eTraversalNeeded;
+ transactionFlags = getTransactionFlags(mask);
+ handleTransactionLocked(transactionFlags);
- /*
- * Clean-up all layers that went away
- * (do this without the lock held)
- */
- const size_t count = ditchedLayers.size();
- for (size_t i=0 ; i<count ; i++) {
- if (ditchedLayers[i] != 0) {
- //LOGD("ditching layer %p", ditchedLayers[i].get());
- ditchedLayers[i]->ditch();
- }
- }
+ mLastTransactionTime = systemTime() - now;
+ mDebugInTransaction = 0;
+ // here the transaction has been committed
}
-void SurfaceFlinger::handleTransactionLocked(
- uint32_t transactionFlags, Vector< sp<LayerBase> >& ditchedLayers)
+void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
{
const LayerVector& currentLayers(mCurrentState.layersSortedByZ);
const size_t count = currentLayers.size();
@@ -569,7 +561,6 @@
const sp<LayerBase>& layer(previousLayers[i]);
if (currentLayers.indexOf( layer ) < 0) {
// this layer is not visible anymore
- ditchedLayers.add(layer);
mDirtyRegionRemovedLayer.orSelf(layer->visibleRegionScreen);
}
}
@@ -579,6 +570,31 @@
commitTransaction();
}
+void SurfaceFlinger::destroyLayer(LayerBase const* layer)
+{
+ Mutex::Autolock _l(mDestroyedLayerLock);
+ mDestroyedLayers.add(layer);
+ signalEvent();
+}
+
+void SurfaceFlinger::handleDestroyLayers()
+{
+ Vector<LayerBase const *> destroyedLayers;
+
+ { // scope for the lock
+ Mutex::Autolock _l(mDestroyedLayerLock);
+ destroyedLayers = mDestroyedLayers;
+ mDestroyedLayers.clear();
+ }
+
+ // call destructors without a lock held
+ const size_t count = destroyedLayers.size();
+ for (size_t i=0 ; i<count ; i++) {
+ //LOGD("destroying %s", destroyedLayers[i]->getName().string());
+ delete destroyedLayers[i];
+ }
+}
+
sp<FreezeLock> SurfaceFlinger::getFreezeLock() const
{
return new FreezeLock(const_cast<SurfaceFlinger *>(this));
@@ -1030,15 +1046,15 @@
ssize_t SurfaceFlinger::addClientLayer(const sp<Client>& client,
const sp<LayerBaseClient>& lbc)
{
- Mutex::Autolock _l(mStateLock);
-
// attach this layer to the client
- ssize_t name = client->attachLayer(lbc);
+ size_t name = client->attachLayer(lbc);
+
+ Mutex::Autolock _l(mStateLock);
// add this layer to the current state list
addLayer_l(lbc);
- return name;
+ return ssize_t(name);
}
status_t SurfaceFlinger::removeLayer(const sp<LayerBase>& layer)
@@ -1085,6 +1101,11 @@
return NO_ERROR;
}
+uint32_t SurfaceFlinger::peekTransactionFlags(uint32_t flags)
+{
+ return android_atomic_release_load(&mTransactionFlags);
+}
+
uint32_t SurfaceFlinger::getTransactionFlags(uint32_t flags)
{
return android_atomic_and(~flags, &mTransactionFlags) & flags;
@@ -1314,38 +1335,18 @@
return err;
}
-status_t SurfaceFlinger::destroySurface(const sp<LayerBaseClient>& layer)
+status_t SurfaceFlinger::destroySurface(const wp<LayerBaseClient>& layer)
{
// called by ~ISurface() when all references are gone
-
- class MessageDestroySurface : public MessageBase {
- SurfaceFlinger* flinger;
- sp<LayerBaseClient> layer;
- public:
- MessageDestroySurface(
- SurfaceFlinger* flinger, const sp<LayerBaseClient>& layer)
- : flinger(flinger), layer(layer) { }
- virtual bool handler() {
- sp<LayerBaseClient> l(layer);
- layer.clear(); // clear it outside of the lock;
- Mutex::Autolock _l(flinger->mStateLock);
- /*
- * remove the layer from the current list -- chances are that it's
- * not in the list anyway, because it should have been removed
- * already upon request of the client (eg: window manager).
- * However, a buggy client could have not done that.
- * Since we know we don't have any more clients, we don't need
- * to use the purgatory.
- */
- status_t err = flinger->removeLayer_l(l);
- LOGE_IF(err<0 && err != NAME_NOT_FOUND,
- "error removing layer=%p (%s)", l.get(), strerror(-err));
- return true;
- }
- };
-
- postMessageAsync( new MessageDestroySurface(this, layer) );
- return NO_ERROR;
+ status_t err = NO_ERROR;
+ sp<LayerBaseClient> l(layer.promote());
+ if (l != NULL) {
+ Mutex::Autolock _l(mStateLock);
+ err = removeLayer_l(l);
+ LOGE_IF(err<0 && err != NAME_NOT_FOUND,
+ "error removing layer=%p (%s)", l.get(), strerror(-err));
+ }
+ return err;
}
status_t SurfaceFlinger::setClientState(
@@ -2253,15 +2254,17 @@
return NO_ERROR;
}
-ssize_t Client::attachLayer(const sp<LayerBaseClient>& layer)
+size_t Client::attachLayer(const sp<LayerBaseClient>& layer)
{
- int32_t name = android_atomic_inc(&mNameGenerator);
+ Mutex::Autolock _l(mLock);
+ size_t name = mNameGenerator++;
mLayers.add(name, layer);
return name;
}
void Client::detachLayer(const LayerBaseClient* layer)
{
+ Mutex::Autolock _l(mLock);
// we do a linear search here, because this doesn't happen often
const size_t count = mLayers.size();
for (size_t i=0 ; i<count ; i++) {
@@ -2271,9 +2274,11 @@
}
}
}
-sp<LayerBaseClient> Client::getLayerUser(int32_t i) const {
+sp<LayerBaseClient> Client::getLayerUser(int32_t i) const
+{
+ Mutex::Autolock _l(mLock);
sp<LayerBaseClient> lbc;
- const wp<LayerBaseClient>& layer(mLayers.valueFor(i));
+ wp<LayerBaseClient> layer(mLayers.valueFor(i));
if (layer != 0) {
lbc = layer.promote();
LOGE_IF(lbc==0, "getLayerUser(name=%d) is dead", int(i));
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index df1ca48..9fa98cf 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -66,7 +66,7 @@
status_t initCheck() const;
// protected by SurfaceFlinger::mStateLock
- ssize_t attachLayer(const sp<LayerBaseClient>& layer);
+ size_t attachLayer(const sp<LayerBaseClient>& layer);
void detachLayer(const LayerBaseClient* layer);
sp<LayerBaseClient> getLayerUser(int32_t i) const;
@@ -82,9 +82,15 @@
virtual status_t destroySurface(SurfaceID surfaceId);
virtual status_t setState(int32_t count, const layer_state_t* states);
- DefaultKeyedVector< size_t, wp<LayerBaseClient> > mLayers;
+ // constant
sp<SurfaceFlinger> mFlinger;
- int32_t mNameGenerator;
+
+ // protected by mLock
+ DefaultKeyedVector< size_t, wp<LayerBaseClient> > mLayers;
+ size_t mNameGenerator;
+
+ // thread-safe
+ mutable Mutex mLock;
};
class UserClient : public BnSurfaceComposerClient
@@ -212,6 +218,7 @@
status_t removeLayer(const sp<LayerBase>& layer);
status_t addLayer(const sp<LayerBase>& layer);
status_t invalidateLayerVisibility(const sp<LayerBase>& layer);
+ void destroyLayer(LayerBase const* layer);
sp<Layer> getLayer(const sp<ISurface>& sur) const;
@@ -249,7 +256,7 @@
uint32_t w, uint32_t h, uint32_t flags);
status_t removeSurface(const sp<Client>& client, SurfaceID sid);
- status_t destroySurface(const sp<LayerBaseClient>& layer);
+ status_t destroySurface(const wp<LayerBaseClient>& layer);
status_t setClientState(const sp<Client>& client,
int32_t count, const layer_state_t* states);
@@ -294,9 +301,8 @@
private:
void handleConsoleEvents();
void handleTransaction(uint32_t transactionFlags);
- void handleTransactionLocked(
- uint32_t transactionFlags,
- Vector< sp<LayerBase> >& ditchedLayers);
+ void handleTransactionLocked(uint32_t transactionFlags);
+ void handleDestroyLayers();
void computeVisibleRegions(
LayerVector& currentLayers,
@@ -319,6 +325,7 @@
status_t purgatorizeLayer_l(const sp<LayerBase>& layer);
uint32_t getTransactionFlags(uint32_t flags);
+ uint32_t peekTransactionFlags(uint32_t flags);
uint32_t setTransactionFlags(uint32_t flags);
void commitTransaction();
@@ -415,6 +422,11 @@
// these are thread safe
mutable Barrier mReadyToRunBarrier;
+
+ // protected by mDestroyedLayerLock;
+ mutable Mutex mDestroyedLayerLock;
+ Vector<LayerBase const *> mDestroyedLayers;
+
// atomic variables
enum {
eConsoleReleased = 1,