diff options
4 files changed, 17 insertions, 10 deletions
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index 18f7f44fa5..fac9024121 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -441,7 +441,7 @@ void BufferQueueLayer::onFrameAvailable(const BufferItem& item) { mQueueItemCondition.broadcast(); } - mFlinger->mInterceptor->saveBufferUpdate(this, item.mGraphicBuffer->getWidth(), + mFlinger->mInterceptor->saveBufferUpdate(layerId, item.mGraphicBuffer->getWidth(), item.mGraphicBuffer->getHeight(), item.mFrameNumber); mFlinger->signalLayerUpdate(); diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp index 1f9d46c330..5b3cd698b5 100644 --- a/services/surfaceflinger/SurfaceInterceptor.cpp +++ b/services/surfaceflinger/SurfaceInterceptor.cpp @@ -524,11 +524,11 @@ void SurfaceInterceptor::addSurfaceDeletionLocked(Increment* increment, deletion->set_id(getLayerId(layer)); } -void SurfaceInterceptor::addBufferUpdateLocked(Increment* increment, const sp<const Layer>& layer, +void SurfaceInterceptor::addBufferUpdateLocked(Increment* increment, int32_t layerId, uint32_t width, uint32_t height, uint64_t frameNumber) { BufferUpdate* update(increment->mutable_buffer_update()); - update->set_id(getLayerId(layer)); + update->set_id(layerId); update->set_w(width); update->set_h(height); update->set_frame_number(frameNumber); @@ -644,15 +644,22 @@ void SurfaceInterceptor::saveSurfaceDeletion(const sp<const Layer>& layer) { addSurfaceDeletionLocked(createTraceIncrementLocked(), layer); } -void SurfaceInterceptor::saveBufferUpdate(const sp<const Layer>& layer, uint32_t width, +/** + * Here we pass the layer by ID instead of by sp<> since this is called without + * holding the state-lock from a Binder thread. If we required the caller + * to pass 'this' by sp<> the temporary sp<> constructed could end up + * being the last reference and we might accidentally destroy the Layer + * from this binder thread. + */ +void SurfaceInterceptor::saveBufferUpdate(int32_t layerId, uint32_t width, uint32_t height, uint64_t frameNumber) { - if (!mEnabled || layer == nullptr) { + if (!mEnabled) { return; } ATRACE_CALL(); std::lock_guard<std::mutex> protoGuard(mTraceMutex); - addBufferUpdateLocked(createTraceIncrementLocked(), layer, width, height, frameNumber); + addBufferUpdateLocked(createTraceIncrementLocked(), layerId, width, height, frameNumber); } void SurfaceInterceptor::saveVSyncEvent(nsecs_t timestamp) { diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h index a665f62ad7..896bdcc259 100644 --- a/services/surfaceflinger/SurfaceInterceptor.h +++ b/services/surfaceflinger/SurfaceInterceptor.h @@ -67,7 +67,7 @@ public: // Intercept surface data virtual void saveSurfaceCreation(const sp<const Layer>& layer) = 0; virtual void saveSurfaceDeletion(const sp<const Layer>& layer) = 0; - virtual void saveBufferUpdate(const sp<const Layer>& layer, uint32_t width, uint32_t height, + virtual void saveBufferUpdate(int32_t layerId, uint32_t width, uint32_t height, uint64_t frameNumber) = 0; // Intercept display data @@ -102,7 +102,7 @@ public: // Intercept surface data void saveSurfaceCreation(const sp<const Layer>& layer) override; void saveSurfaceDeletion(const sp<const Layer>& layer) override; - void saveBufferUpdate(const sp<const Layer>& layer, uint32_t width, uint32_t height, + void saveBufferUpdate(int32_t layerId, uint32_t width, uint32_t height, uint64_t frameNumber) override; // Intercept display data @@ -130,7 +130,7 @@ private: Increment* createTraceIncrementLocked(); void addSurfaceCreationLocked(Increment* increment, const sp<const Layer>& layer); void addSurfaceDeletionLocked(Increment* increment, const sp<const Layer>& layer); - void addBufferUpdateLocked(Increment* increment, const sp<const Layer>& layer, uint32_t width, + void addBufferUpdateLocked(Increment* increment, int32_t layerId, uint32_t width, uint32_t height, uint64_t frameNumber); void addVSyncUpdateLocked(Increment* increment, nsecs_t timestamp); void addDisplayCreationLocked(Increment* increment, const DisplayDeviceState& info); diff --git a/services/surfaceflinger/tests/unittests/mock/MockSurfaceInterceptor.h b/services/surfaceflinger/tests/unittests/mock/MockSurfaceInterceptor.h index 458b2f3da2..5beee1c0ec 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockSurfaceInterceptor.h +++ b/services/surfaceflinger/tests/unittests/mock/MockSurfaceInterceptor.h @@ -39,7 +39,7 @@ public: const Vector<DisplayState>&, uint32_t)); MOCK_METHOD1(saveSurfaceCreation, void(const sp<const Layer>&)); MOCK_METHOD1(saveSurfaceDeletion, void(const sp<const Layer>&)); - MOCK_METHOD4(saveBufferUpdate, void(const sp<const Layer>&, uint32_t, uint32_t, uint64_t)); + MOCK_METHOD4(saveBufferUpdate, void(int32_t, uint32_t, uint32_t, uint64_t)); MOCK_METHOD1(saveDisplayCreation, void(const DisplayDeviceState&)); MOCK_METHOD1(saveDisplayDeletion, void(int32_t)); MOCK_METHOD2(savePowerModeUpdate, void(int32_t, int32_t)); |