Merge "SurfaceFlinger: Share ownership of layers between State and Handle." (8feddee9) · Commits · e / os / android_frameworks_native

services/surfaceflinger/Layer.cpp

+29 −18

Original line number	Original line	Diff line number	Diff line
	@@ -119,13 +119,20 @@ Layer::~Layer() {
	c->detachLayer(this);		c->detachLayer(this);
	}		}

	for (auto& point : mRemoteSyncPoints) {		mFrameTracker.logAndResetStats(mName);
	point->setTransactionApplied();
	}		// The remote sync points are cleared out when we are
			// removed from current state.
			Mutex::Autolock lock(mLocalSyncPointMutex);
	for (auto& point : mLocalSyncPoints) {		for (auto& point : mLocalSyncPoints) {
	point->setFrameAvailable();		point->setFrameAvailable();
	}		}
	mFrameTracker.logAndResetStats(mName);
			abandon();

			destroyAllHwcLayers();

			mFlinger->onLayerDestroyed();
	}		}

	// ---------------------------------------------------------------------------		// ---------------------------------------------------------------------------
	@@ -140,10 +147,9 @@ Layer::~Layer() {
	void Layer::onLayerDisplayed(const sp<Fence>& /releaseFence/) {}		void Layer::onLayerDisplayed(const sp<Fence>& /releaseFence/) {}

	void Layer::onRemovedFromCurrentState() {		void Layer::onRemovedFromCurrentState() {
	// the layer is removed from SF mCurrentState to mLayersPendingRemoval		mRemovedFromCurrentState = true;

	mPendingRemoval = true;

			// the layer is removed from SF mCurrentState to mLayersPendingRemoval
	if (mCurrentState.zOrderRelativeOf != nullptr) {		if (mCurrentState.zOrderRelativeOf != nullptr) {
	sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();		sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
	if (strongRelative != nullptr) {		if (strongRelative != nullptr) {
	@@ -153,19 +159,18 @@ void Layer::onRemovedFromCurrentState() {
	mCurrentState.zOrderRelativeOf = nullptr;		mCurrentState.zOrderRelativeOf = nullptr;
	}		}

	for (const auto& child : mCurrentChildren) {		// Since we are no longer reachable from CurrentState SurfaceFlinger
	child->onRemovedFromCurrentState();		// will no longer invoke doTransaction for us, and so we will
	}		// never finish applying transactions. We signal the sync point
			// now so that another layer will not become indefinitely
			// blocked.
			for (auto& point: mRemoteSyncPoints) {
			point->setTransactionApplied();
	}		}
			mRemoteSyncPoints.clear();
	void Layer::onRemoved() {
	// the layer is removed from SF mLayersPendingRemoval
	abandon();

	destroyAllHwcLayers();

	for (const auto& child : mCurrentChildren) {		for (const auto& child : mCurrentChildren) {
	child->onRemoved();		child->onRemovedFromCurrentState();
	}		}
	}		}

	@@ -228,6 +233,10 @@ void Layer::destroyAllHwcLayers() {
	}		}
	LOG_ALWAYS_FATAL_IF(!getBE().mHwcLayers.empty(),		LOG_ALWAYS_FATAL_IF(!getBE().mHwcLayers.empty(),
	"All hardware composer layers should have been destroyed");		"All hardware composer layers should have been destroyed");

			for (const sp<Layer>& child : mDrawingChildren) {
			child->destroyAllHwcLayers();
			}
	}		}

	Rect Layer::getContentCrop() const {		Rect Layer::getContentCrop() const {
	@@ -825,7 +834,9 @@ void Layer::pushPendingState() {

	// If this transaction is waiting on the receipt of a frame, generate a sync		// If this transaction is waiting on the receipt of a frame, generate a sync
	// point and send it to the remote layer.		// point and send it to the remote layer.
	if (mCurrentState.barrierLayer_legacy != nullptr) {		// We don't allow installing sync points after we are removed from the current state
			// as we won't be able to signal our end.
			if (mCurrentState.barrierLayer_legacy != nullptr && !mRemovedFromCurrentState) {
	sp<Layer> barrierLayer = mCurrentState.barrierLayer_legacy.promote();		sp<Layer> barrierLayer = mCurrentState.barrierLayer_legacy.promote();
	if (barrierLayer == nullptr) {		if (barrierLayer == nullptr) {
	ALOGE("[%s] Unable to promote barrier Layer.", mName.string());		ALOGE("[%s] Unable to promote barrier Layer.", mName.string());

services/surfaceflinger/Layer.h

+6 −10

Original line number	Original line	Diff line number	Diff line
	@@ -346,7 +346,7 @@ public:
	virtual bool isCreatedFromMainThread() const { return false; }		virtual bool isCreatedFromMainThread() const { return false; }


	bool isPendingRemoval() const { return mPendingRemoval; }		bool isRemovedFromCurrentState() const { return mRemovedFromCurrentState; }

	void writeToProto(LayerProto* layerInfo,		void writeToProto(LayerProto* layerInfo,
	LayerVector::StateSet stateSet = LayerVector::StateSet::Drawing);		LayerVector::StateSet stateSet = LayerVector::StateSet::Drawing);
	@@ -475,12 +475,6 @@ public:
	*/		*/
	void onRemovedFromCurrentState();		void onRemovedFromCurrentState();

	/*
	* called with the state lock from the main thread when the layer is
	* removed from the pending removal list
	*/
	void onRemoved();

	// Updates the transform hint in our SurfaceFlingerConsumer to match		// Updates the transform hint in our SurfaceFlingerConsumer to match
	// the current orientation of the display device.		// the current orientation of the display device.
	void updateTransformHint(const sp<const DisplayDevice>& display) const;		void updateTransformHint(const sp<const DisplayDevice>& display) const;
	@@ -595,12 +589,12 @@ protected:
	*/		*/
	class LayerCleaner {		class LayerCleaner {
	sp<SurfaceFlinger> mFlinger;		sp<SurfaceFlinger> mFlinger;
	wp<Layer> mLayer;		sp<Layer> mLayer;

	protected:		protected:
	~LayerCleaner() {		~LayerCleaner() {
	// destroy client resources		// destroy client resources
	mFlinger->onLayerDestroyed(mLayer);		mFlinger->onHandleDestroyed(mLayer);
	}		}

	public:		public:
	@@ -702,6 +696,8 @@ public:
	virtual PixelFormat getPixelFormat() const { return PIXEL_FORMAT_NONE; }		virtual PixelFormat getPixelFormat() const { return PIXEL_FORMAT_NONE; }
	bool getPremultipledAlpha() const;		bool getPremultipledAlpha() const;

			bool mPendingHWCDestroy{false};

	protected:		protected:
	// -----------------------------------------------------------------------		// -----------------------------------------------------------------------
	bool usingRelativeZ(LayerVector::StateSet stateSet);		bool usingRelativeZ(LayerVector::StateSet stateSet);
	@@ -745,7 +741,7 @@ protected:
	// Whether filtering is needed b/c of the drawingstate		// Whether filtering is needed b/c of the drawingstate
	bool mNeedsFiltering{false};		bool mNeedsFiltering{false};

	bool mPendingRemoval{false};		bool mRemovedFromCurrentState{false};

	// page-flip thread (currently main thread)		// page-flip thread (currently main thread)
	bool mProtectedByApp{false}; // application requires protected path to external sink		bool mProtectedByApp{false}; // application requires protected path to external sink

services/surfaceflinger/SurfaceFlinger.cpp

+21 −59

Original line number	Original line	Diff line number	Diff line
	@@ -2715,7 +2715,14 @@ void SurfaceFlinger::commitTransaction()
	for (const auto& l : mLayersPendingRemoval) {		for (const auto& l : mLayersPendingRemoval) {
	recordBufferingStats(l->getName().string(),		recordBufferingStats(l->getName().string(),
	l->getOccupancyHistory(true));		l->getOccupancyHistory(true));
	l->onRemoved();
			// We need to release the HWC layers when the Layer is removed
			// from the current state otherwise the HWC layer just continues
			// showing at it's last configured state until we eventually
			// abandon the buffer queue.
			if (l->isRemovedFromCurrentState()) {
			l->destroyAllHwcLayers();
			}
	}		}
	mLayersPendingRemoval.clear();		mLayersPendingRemoval.clear();
	}		}
	@@ -3148,10 +3155,6 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client,
	if (parent == nullptr) {		if (parent == nullptr) {
	mCurrentState.layersSortedByZ.add(lbc);		mCurrentState.layersSortedByZ.add(lbc);
	} else {		} else {
	if (parent->isPendingRemoval()) {
	ALOGE("addClientLayer called with a removed parent");
	return NAME_NOT_FOUND;
	}
	parent->addChild(lbc);		parent->addChild(lbc);
	}		}

	@@ -3178,52 +3181,22 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly)
	return removeLayerLocked(mStateLock, layer, topLevelOnly);		return removeLayerLocked(mStateLock, layer, topLevelOnly);
	}		}

	status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer,		status_t SurfaceFlinger::removeLayerLocked(const Mutex& lock, const sp<Layer>& layer,
	bool topLevelOnly) {		bool topLevelOnly) {
	if (layer->isPendingRemoval()) {
	return NO_ERROR;
	}

	const auto& p = layer->getParent();		const auto& p = layer->getParent();
	ssize_t index;		ssize_t index;
	if (p != nullptr) {		if (p != nullptr) {
	if (topLevelOnly) {		if (topLevelOnly) {
	return NO_ERROR;		return NO_ERROR;
	}		}

	sp<Layer> ancestor = p;
	while (ancestor->getParent() != nullptr) {
	ancestor = ancestor->getParent();
	}
	if (mCurrentState.layersSortedByZ.indexOf(ancestor) < 0) {
	ALOGE("removeLayer called with a layer whose parent has been removed");
	return NAME_NOT_FOUND;
	}

	index = p->removeChild(layer);		index = p->removeChild(layer);
	} else {		} else {
	index = mCurrentState.layersSortedByZ.remove(layer);		index = mCurrentState.layersSortedByZ.remove(layer);
	}		}

	// As a matter of normal operation, the LayerCleaner will produce a second
	// attempt to remove the surface. The Layer will be kept alive in mDrawingState
	// so we will succeed in promoting it, but it's already been removed
	// from mCurrentState. As long as we can find it in mDrawingState we have no problem
	// otherwise something has gone wrong and we are leaking the layer.
	if (index < 0 && mDrawingState.layersSortedByZ.indexOf(layer) < 0) {
	ALOGE("Failed to find layer (%s) in layer parent (%s).",
	layer->getName().string(),
	(p != nullptr) ? p->getName().string() : "no-parent");
	return BAD_VALUE;
	} else if (index < 0) {
	return NO_ERROR;
	}

	layer->onRemovedFromCurrentState();		layer->onRemovedFromCurrentState();
	mLayersPendingRemoval.add(layer);
	mLayersRemoved = true;		markLayerPendingRemovalLocked(lock, layer);
	mNumLayers -= 1 + layer->getChildrenCount();
	setTransactionFlags(eTransactionNeeded);
	return NO_ERROR;		return NO_ERROR;
	}		}

	@@ -3424,11 +3397,6 @@ uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState
	return 0;		return 0;
	}		}

	if (layer->isPendingRemoval()) {
	ALOGW("Attempting to set client state on removed layer: %s", layer->getName().string());
	return 0;
	}

	uint32_t flags = 0;		uint32_t flags = 0;

	const uint32_t what = s.what;		const uint32_t what = s.what;
	@@ -3632,11 +3600,6 @@ void SurfaceFlinger::setDestroyStateLocked(const ComposerState& composerState) {
	return;		return;
	}		}

	if (layer->isPendingRemoval()) {
	ALOGW("Attempting to destroy on removed layer: %s", layer->getName().string());
	return;
	}

	if (state.what & layer_state_t::eDestroySurface) {		if (state.what & layer_state_t::eDestroySurface) {
	removeLayerLocked(mStateLock, layer);		removeLayerLocked(mStateLock, layer);
	}		}
	@@ -3809,17 +3772,16 @@ status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBind
	return err;		return err;
	}		}

	status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)		void SurfaceFlinger::markLayerPendingRemovalLocked(const Mutex&, const sp<Layer>& layer) {
	{		mLayersPendingRemoval.add(layer);
	// called by ~LayerCleaner() when all references to the IBinder (handle)		mLayersRemoved = true;
	// are gone		setTransactionFlags(eTransactionNeeded);
	sp<Layer> l = layer.promote();
	if (l == nullptr) {
	// The layer has already been removed, carry on
	return NO_ERROR;
	}		}
	// If we have a parent, then we can continue to live as long as it does.
	return removeLayer(l, true);		void SurfaceFlinger::onHandleDestroyed(const sp<Layer>& layer)
			{
			Mutex::Autolock lock(mStateLock);
			markLayerPendingRemovalLocked(mStateLock, layer);
	}		}

	// ---------------------------------------------------------------------------		// ---------------------------------------------------------------------------
	@@ -5212,7 +5174,7 @@ status_t SurfaceFlinger::captureLayers(const sp<IBinder>& layerHandleBinder,
	auto layerHandle = reinterpret_cast<Layer::Handle*>(layerHandleBinder.get());		auto layerHandle = reinterpret_cast<Layer::Handle*>(layerHandleBinder.get());
	auto parent = layerHandle->owner.promote();		auto parent = layerHandle->owner.promote();

	if (parent == nullptr \|\| parent->isPendingRemoval()) {		if (parent == nullptr \|\| parent->isRemovedFromCurrentState()) {
	ALOGE("captureLayers called with a removed parent");		ALOGE("captureLayers called with a removed parent");
	return NAME_NOT_FOUND;		return NAME_NOT_FOUND;
	}		}

services/surfaceflinger/SurfaceFlinger.h

+5 −1

Original line number	Original line	Diff line number	Diff line
	@@ -356,6 +356,8 @@ public:
	bool authenticateSurfaceTextureLocked(		bool authenticateSurfaceTextureLocked(
	const sp<IGraphicBufferProducer>& bufferProducer) const;		const sp<IGraphicBufferProducer>& bufferProducer) const;

			inline void onLayerDestroyed() { mNumLayers--; }

	private:		private:
	friend class Client;		friend class Client;
	friend class DisplayEventConnection;		friend class DisplayEventConnection;
	@@ -561,10 +563,12 @@ private:
	// ISurfaceComposerClient::destroySurface()		// ISurfaceComposerClient::destroySurface()
	status_t onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle);		status_t onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle);

			void markLayerPendingRemovalLocked(const Mutex& /* mStateLock */, const sp<Layer>& layer);

	// called when all clients have released all their references to		// called when all clients have released all their references to
	// this layer meaning it is entirely safe to destroy all		// this layer meaning it is entirely safe to destroy all
	// resources associated to this layer.		// resources associated to this layer.
	status_t onLayerDestroyed(const wp<Layer>& layer);		void onHandleDestroyed(const sp<Layer>& layer);

	// remove a layer from SurfaceFlinger immediately		// remove a layer from SurfaceFlinger immediately
	status_t removeLayer(const sp<Layer>& layer, bool topLevelOnly = false);		status_t removeLayer(const sp<Layer>& layer, bool topLevelOnly = false);

services/surfaceflinger/tests/Transaction_test.cpp

+31 −0

Original line number	Original line	Diff line number	Diff line
	@@ -2550,6 +2550,37 @@ TEST_F(ChildLayerTest, ReparentChildren) {
	}		}
	}		}

			TEST_F(ChildLayerTest, ChildrenSurviveParentDestruction) {
			sp<SurfaceControl> mGrandChild =
			mClient->createSurface(String8("Grand Child"), 10, 10,
			PIXEL_FORMAT_RGBA_8888, 0, mChild.get());
			fillSurfaceRGBA8(mGrandChild, 111, 111, 111);

			{
			SCOPED_TRACE("Grandchild visible");
			ScreenCapture::captureScreen(&mCapture);
			mCapture->checkPixel(64, 64, 111, 111, 111);
			}

			mChild->clear();

			{
			SCOPED_TRACE("After destroying child");
			ScreenCapture::captureScreen(&mCapture);
			mCapture->expectFGColor(64, 64);
			}

			asTransaction([&](Transaction& t) {
			t.reparent(mGrandChild, mFGSurfaceControl->getHandle());
			});

			{
			SCOPED_TRACE("After reparenting grandchild");
			ScreenCapture::captureScreen(&mCapture);
			mCapture->checkPixel(64, 64, 111, 111, 111);
			}
			}

	TEST_F(ChildLayerTest, DetachChildrenSameClient) {		TEST_F(ChildLayerTest, DetachChildrenSameClient) {
	asTransaction([&](Transaction& t) {		asTransaction([&](Transaction& t) {
	t.show(mChild);		t.show(mChild);