From a8c7c54eed57e5a4b56905a4fb00e27e25b1b908 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Tue, 20 Jul 2021 18:49:42 -0700 Subject: SurfaceFlinger: Safely cast from IBinder to Layer::Handle Bug: b/193034677, b/193034683, b/193033243 Test: go/wm-smoke, presubmit Change-Id: Iece64fca254edfd0b82e05ad9629824b2364cc13 Merged-In: Iece64fca254edfd0b82e05ad9629824b2364cc13 --- services/surfaceflinger/SurfaceFlinger.cpp | 50 +++++++----------------------- 1 file changed, 11 insertions(+), 39 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 230810c936..fe43a2091d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3611,7 +3611,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( sp layer = nullptr; if (s.surface) { - layer = fromHandleLocked(s.surface).promote(); + layer = fromHandle(s.surface).promote(); } else if (s.hasBufferChanges()) { ALOGW("Transaction with buffer, but no Layer?"); continue; @@ -3778,7 +3778,7 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp, postTime, permissions, listenerCallbacksWithSurfaces); if ((flags & eAnimation) && state.state.surface) { - if (const auto layer = fromHandleLocked(state.state.surface).promote(); layer) { + if (const auto layer = fromHandle(state.state.surface).promote(); layer) { mScheduler->recordLayerHistory(layer.get(), isAutoTimestamp ? 0 : desiredPresentTime, LayerHistory::LayerUpdateType::AnimationTX); @@ -3933,13 +3933,11 @@ uint32_t SurfaceFlinger::setClientStateLocked( if (what & layer_state_t::eLayerCreated) { layer = handleLayerCreatedLocked(s.surface); if (layer) { - // put the created layer into mLayersByLocalBinderToken. - mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer); flags |= eTransactionNeeded | eTraversalNeeded; mLayersAdded = true; } } else { - layer = fromHandleLocked(s.surface).promote(); + layer = fromHandle(s.surface).promote(); } } else { // The client may provide us a null handle. Treat it as if the layer was removed. @@ -4267,7 +4265,7 @@ status_t SurfaceFlinger::mirrorLayer(const sp& client, const sp { Mutex::Autolock _l(mStateLock); - mirrorFrom = fromHandleLocked(mirrorFromHandle).promote(); + mirrorFrom = fromHandle(mirrorFromHandle).promote(); if (!mirrorFrom) { return NAME_NOT_FOUND; } @@ -4465,7 +4463,7 @@ void SurfaceFlinger::markLayerPendingRemovalLocked(const sp& layer) { setTransactionFlags(eTransactionNeeded); } -void SurfaceFlinger::onHandleDestroyed(sp& layer) { +void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp& layer) { Mutex::Autolock lock(mStateLock); // If a layer has a parent, we allow it to out-live it's handle // with the idea that the parent holds a reference and will eventually @@ -4476,17 +4474,7 @@ void SurfaceFlinger::onHandleDestroyed(sp& layer) { mCurrentState.layersSortedByZ.remove(layer); } markLayerPendingRemovalLocked(layer); - - auto it = mLayersByLocalBinderToken.begin(); - while (it != mLayersByLocalBinderToken.end()) { - if (it->second == layer) { - mBufferCountTracker.remove(it->first->localBinder()); - it = mLayersByLocalBinderToken.erase(it); - } else { - it++; - } - } - + mBufferCountTracker.remove(handle); layer.clear(); } @@ -6089,7 +6077,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, { Mutex::Autolock lock(mStateLock); - parent = fromHandleLocked(args.layerHandle).promote(); + parent = fromHandle(args.layerHandle).promote(); if (parent == nullptr || parent->isRemovedFromCurrentState()) { ALOGE("captureLayers called with an invalid or removed parent"); return NAME_NOT_FOUND; @@ -6120,7 +6108,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, reqSize = ui::Size(crop.width() * args.frameScaleX, crop.height() * args.frameScaleY); for (const auto& handle : args.excludeHandles) { - sp excludeLayer = fromHandleLocked(handle).promote(); + sp excludeLayer = fromHandle(handle).promote(); if (excludeLayer != nullptr) { excludeLayers.emplace(excludeLayer); } else { @@ -6616,24 +6604,8 @@ status_t SurfaceFlinger::getDesiredDisplayModeSpecs(const sp& displayTo } } -wp SurfaceFlinger::fromHandle(const sp& handle) { - Mutex::Autolock _l(mStateLock); - return fromHandleLocked(handle); -} - -wp SurfaceFlinger::fromHandleLocked(const sp& handle) const { - BBinder* b = nullptr; - if (handle) { - b = handle->localBinder(); - } - if (b == nullptr) { - return nullptr; - } - auto it = mLayersByLocalBinderToken.find(b); - if (it != mLayersByLocalBinderToken.end()) { - return it->second; - } - return nullptr; +wp SurfaceFlinger::fromHandle(const sp& handle) const { + return Layer::fromHandle(handle); } void SurfaceFlinger::onLayerFirstRef(Layer* layer) { @@ -6938,7 +6910,7 @@ sp SurfaceFlinger::handleLayerCreatedLocked(const sp& handle) { sp parent; bool allowAddRoot = state->addToRoot; if (state->initialParent != nullptr) { - parent = fromHandleLocked(state->initialParent.promote()).promote(); + parent = fromHandle(state->initialParent.promote()).promote(); if (parent == nullptr) { ALOGE("Invalid parent %p", state->initialParent.unsafe_get()); allowAddRoot = false; -- cgit v1.2.3-59-g8ed1b