SF: Clean up HWC2::Layer ownership
Changes the HWC2::Display::createLayer() call to return a shared_ptr
instead of a bare pointer. Also if the HWC2::Display is disconnected,
instead of directly destroying the HWC2::Layers associated with the
display, a new call is made to clear the display information from the
layer. For safety, checks are added to take an early out if the display
information isn't set for any call where it is used.
The CompositionEngine code was already creating a shared_ptr for the
createLayer call, and expecting the pointer to be valid until it
released it via destroyLayer.
However a hotplug disconnect could leave the CompositionEngine code
holding an invalid pointer until the next display refresh, and it could
lead to bad memory accesses if the pointer was dereferenced.
CompositionEngine itself did not do so -- it released its ownership of
the layer (and the associated display) without accessing them. This
seems to have only affected "dumpsys SurfaceFlinger", if it happened to
be executed after the disconnect, and before another refresh, and only
because it tried to print out the HWC layer id.
Bug: 181061001
Test: libsurfaceflinger_unittest
Test: libcompositionengine_test
Test: dumpsys SurfaceFlinger # after hotplug event
Change-Id: I508d6aa8ef7a6af848dd54198408d5f311175070
Merged-In: I508d6aa8ef7a6af848dd54198408d5f311175070
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 88fb811..b33434e 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -94,6 +94,7 @@
"VSyncReactorTest.cpp",
"VsyncConfigurationTest.cpp",
"mock/DisplayHardware/MockComposer.cpp",
+ "mock/DisplayHardware/MockHWC2.cpp",
"mock/DisplayHardware/MockPowerAdvisor.cpp",
"mock/MockEventThread.cpp",
"mock/MockFrameTimeline.cpp",
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index 6b82170..cbf8cc2 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -37,6 +37,7 @@
#include "DisplayHardware/HWComposer.h"
#include "DisplayHardware/Hal.h"
#include "mock/DisplayHardware/MockComposer.h"
+#include "mock/DisplayHardware/MockHWC2.h"
// TODO(b/129481165): remove the #pragma below and fix conversion issues
#pragma clang diagnostic pop // ignored "-Wconversion"
@@ -120,13 +121,19 @@
static constexpr hal::HWLayerId kLayerId = static_cast<hal::HWLayerId>(1002);
HWComposerLayerTest(const std::unordered_set<hal::Capability>& capabilities)
- : mCapabilies(capabilities) {}
+ : mCapabilies(capabilities) {
+ EXPECT_CALL(mDisplay, getId()).WillRepeatedly(Return(kDisplayId));
+ }
- ~HWComposerLayerTest() override { EXPECT_CALL(*mHal, destroyLayer(kDisplayId, kLayerId)); }
+ ~HWComposerLayerTest() override {
+ EXPECT_CALL(mDisplay, onLayerDestroyed(kLayerId));
+ EXPECT_CALL(*mHal, destroyLayer(kDisplayId, kLayerId));
+ }
std::unique_ptr<Hwc2::mock::Composer> mHal{new StrictMock<Hwc2::mock::Composer>()};
const std::unordered_set<hal::Capability> mCapabilies;
- HWC2::impl::Layer mLayer{*mHal, mCapabilies, kDisplayId, kLayerId};
+ StrictMock<HWC2::mock::Display> mDisplay;
+ HWC2::impl::Layer mLayer{*mHal, mCapabilies, mDisplay, kLayerId};
};
struct HWComposerLayerGenericMetadataTest : public HWComposerLayerTest {
@@ -176,4 +183,4 @@
}
} // namespace
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.cpp b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.cpp
new file mode 100644
index 0000000..2647bf4
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.cpp
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef LOG_TAG
+#define LOG_TAG "MockHWC2"
+
+#include "mock/DisplayHardware/MockHWC2.h"
+
+namespace android::HWC2::mock {
+
+// Explicit default instantiation is recommended.
+Display::Display() = default;
+Display::~Display() = default;
+
+Layer::Layer() = default;
+Layer::~Layer() = default;
+
+} // namespace android::HWC2::mock
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
new file mode 100644
index 0000000..c3919d9
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
@@ -0,0 +1,122 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <gmock/gmock.h>
+
+#include "DisplayHardware/HWC2.h"
+
+namespace android::HWC2::mock {
+
+class Display : public HWC2::Display {
+public:
+ Display();
+ ~Display() override;
+
+ MOCK_METHOD(hal::HWDisplayId, getId, (), (const, override));
+ MOCK_METHOD(bool, isConnected, (), (const, override));
+ MOCK_METHOD(void, setConnected, (bool), (override));
+ MOCK_METHOD(const std::unordered_set<hal::DisplayCapability> &, getCapabilities, (),
+ (const, override));
+ MOCK_METHOD(bool, isVsyncPeriodSwitchSupported, (), (const, override));
+ MOCK_METHOD(void, onLayerDestroyed, (hal::HWLayerId), (override));
+
+ MOCK_METHOD(hal::Error, acceptChanges, (), (override));
+ MOCK_METHOD((base::expected<std::shared_ptr<HWC2::Layer>, hal::Error>), createLayer, (),
+ (override));
+ MOCK_METHOD(hal::Error, getChangedCompositionTypes,
+ ((std::unordered_map<Layer *, hal::Composition> *)), (override));
+ MOCK_METHOD(hal::Error, getColorModes, (std::vector<hal::ColorMode> *), (const, override));
+ MOCK_METHOD(int32_t, getSupportedPerFrameMetadata, (), (const, override));
+ MOCK_METHOD(hal::Error, getRenderIntents, (hal::ColorMode, std::vector<hal::RenderIntent> *),
+ (const, override));
+ MOCK_METHOD(hal::Error, getDataspaceSaturationMatrix, (hal::Dataspace, android::mat4 *),
+ (override));
+ MOCK_METHOD(hal::Error, getName, (std::string *), (const, override));
+ MOCK_METHOD(hal::Error, getRequests,
+ (hal::DisplayRequest *, (std::unordered_map<Layer *, hal::LayerRequest> *)),
+ (override));
+ MOCK_METHOD(hal::Error, getConnectionType, (ui::DisplayConnectionType *), (const, override));
+ MOCK_METHOD(hal::Error, supportsDoze, (bool *), (const, override));
+ MOCK_METHOD(hal::Error, getHdrCapabilities, (android::HdrCapabilities *), (const, override));
+ MOCK_METHOD(hal::Error, getDisplayedContentSamplingAttributes,
+ (hal::PixelFormat *, hal::Dataspace *, uint8_t *), (const, override));
+ MOCK_METHOD(hal::Error, setDisplayContentSamplingEnabled, (bool, uint8_t, uint64_t),
+ (const, override));
+ MOCK_METHOD(hal::Error, getDisplayedContentSample,
+ (uint64_t, uint64_t, android::DisplayedFrameStats *), (const, override));
+ MOCK_METHOD(hal::Error, getReleaseFences,
+ ((std::unordered_map<Layer *, android::sp<android::Fence>> *)), (const, override));
+ MOCK_METHOD(hal::Error, present, (android::sp<android::Fence> *), (override));
+ MOCK_METHOD(hal::Error, setClientTarget,
+ (uint32_t, const android::sp<android::GraphicBuffer> &,
+ const android::sp<android::Fence> &, hal::Dataspace),
+ (override));
+ MOCK_METHOD(hal::Error, setColorMode, (hal::ColorMode, hal::RenderIntent), (override));
+ MOCK_METHOD(hal::Error, setColorTransform, (const android::mat4 &, hal::ColorTransform),
+ (override));
+ MOCK_METHOD(hal::Error, setOutputBuffer,
+ (const android::sp<android::GraphicBuffer> &, const android::sp<android::Fence> &),
+ (override));
+ MOCK_METHOD(hal::Error, setPowerMode, (hal::PowerMode), (override));
+ MOCK_METHOD(hal::Error, setVsyncEnabled, (hal::Vsync), (override));
+ MOCK_METHOD(hal::Error, validate, (uint32_t *, uint32_t *), (override));
+ MOCK_METHOD(hal::Error, presentOrValidate,
+ (uint32_t *, uint32_t *, android::sp<android::Fence> *, uint32_t *), (override));
+ MOCK_METHOD(std::future<hal::Error>, setDisplayBrightness, (float), (override));
+ MOCK_METHOD(hal::Error, setActiveConfigWithConstraints,
+ (hal::HWConfigId, const hal::VsyncPeriodChangeConstraints &,
+ hal::VsyncPeriodChangeTimeline *),
+ (override));
+ MOCK_METHOD(hal::Error, setAutoLowLatencyMode, (bool), (override));
+ MOCK_METHOD(hal::Error, getSupportedContentTypes, (std::vector<hal::ContentType> *),
+ (const, override));
+ MOCK_METHOD(hal::Error, setContentType, (hal::ContentType), (override));
+ MOCK_METHOD(hal::Error, getClientTargetProperty, (hal::ClientTargetProperty *), (override));
+};
+
+class Layer : public HWC2::Layer {
+public:
+ Layer();
+ ~Layer() override;
+
+ MOCK_METHOD(hal::HWLayerId, getId, (), (const, override));
+ MOCK_METHOD(hal::Error, setCursorPosition, (int32_t, int32_t), (override));
+ MOCK_METHOD(hal::Error, setBuffer,
+ (uint32_t, const android::sp<android::GraphicBuffer> &,
+ const android::sp<android::Fence> &),
+ (override));
+ MOCK_METHOD(hal::Error, setSurfaceDamage, (const android::Region &), (override));
+ MOCK_METHOD(hal::Error, setBlendMode, (hal::BlendMode), (override));
+ MOCK_METHOD(hal::Error, setColor, (hal::Color), (override));
+ MOCK_METHOD(hal::Error, setCompositionType, (hal::Composition), (override));
+ MOCK_METHOD(hal::Error, setDataspace, (android::ui::Dataspace), (override));
+ MOCK_METHOD(hal::Error, setPerFrameMetadata, (const int32_t, const android::HdrMetadata &),
+ (override));
+ MOCK_METHOD(hal::Error, setDisplayFrame, (const android::Rect &), (override));
+ MOCK_METHOD(hal::Error, setPlaneAlpha, (float), (override));
+ MOCK_METHOD(hal::Error, setSidebandStream, (const native_handle_t *), (override));
+ MOCK_METHOD(hal::Error, setSourceCrop, (const android::FloatRect &), (override));
+ MOCK_METHOD(hal::Error, setTransform, (hal::Transform), (override));
+ MOCK_METHOD(hal::Error, setVisibleRegion, (const android::Region &), (override));
+ MOCK_METHOD(hal::Error, setZOrder, (uint32_t), (override));
+ MOCK_METHOD(hal::Error, setColorTransform, (const android::mat4 &), (override));
+ MOCK_METHOD(hal::Error, setLayerGenericMetadata,
+ (const std::string &, bool, const std::vector<uint8_t> &), (override));
+};
+
+} // namespace android::HWC2::mock