SF: avoid a nullptr access in RefreshRateOverlay / HDR overlay
Bug: 302312658
Test: presubmit + manual
Change-Id: I3a69e4520be7f52779e1f92e5621a6138de08797
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 1faf6a1..5b6591a 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -428,10 +428,12 @@
return;
}
- mHdrSdrRatioOverlay = std::make_unique<HdrSdrRatioOverlay>();
- mHdrSdrRatioOverlay->setLayerStack(getLayerStack());
- mHdrSdrRatioOverlay->setViewport(getSize());
- updateHdrSdrRatioOverlayRatio(mHdrSdrRatio);
+ mHdrSdrRatioOverlay = HdrSdrRatioOverlay::create();
+ if (mHdrSdrRatioOverlay) {
+ mHdrSdrRatioOverlay->setLayerStack(getLayerStack());
+ mHdrSdrRatioOverlay->setViewport(getSize());
+ updateHdrSdrRatioOverlayRatio(mHdrSdrRatio);
+ }
}
void DisplayDevice::updateHdrSdrRatioOverlayRatio(float currentHdrSdrRatio) {
@@ -468,11 +470,13 @@
// TODO(b/296636258) Update to use the render rate range in VRR mode.
const auto fpsRange = mRefreshRateSelector->getSupportedRefreshRateRange();
- mRefreshRateOverlay = std::make_unique<RefreshRateOverlay>(fpsRange, features);
- mRefreshRateOverlay->setLayerStack(getLayerStack());
- mRefreshRateOverlay->setViewport(getSize());
- updateRefreshRateOverlayRate(getActiveMode().modePtr->getVsyncRate(), getActiveMode().fps,
- setByHwc);
+ mRefreshRateOverlay = RefreshRateOverlay::create(fpsRange, features);
+ if (mRefreshRateOverlay) {
+ mRefreshRateOverlay->setLayerStack(getLayerStack());
+ mRefreshRateOverlay->setViewport(getSize());
+ updateRefreshRateOverlayRate(getActiveMode().modePtr->getVsyncRate(), getActiveMode().fps,
+ setByHwc);
+ }
}
void DisplayDevice::updateRefreshRateOverlayRate(Fps vsyncRate, Fps renderFps, bool setByHwc) {
diff --git a/services/surfaceflinger/HdrSdrRatioOverlay.cpp b/services/surfaceflinger/HdrSdrRatioOverlay.cpp
index 186e878..dfb1c1e 100644
--- a/services/surfaceflinger/HdrSdrRatioOverlay.cpp
+++ b/services/surfaceflinger/HdrSdrRatioOverlay.cpp
@@ -96,7 +96,18 @@
return buffer;
}
-HdrSdrRatioOverlay::HdrSdrRatioOverlay()
+std::unique_ptr<HdrSdrRatioOverlay> HdrSdrRatioOverlay::create() {
+ std::unique_ptr<HdrSdrRatioOverlay> overlay =
+ std::make_unique<HdrSdrRatioOverlay>(ConstructorTag{});
+ if (overlay->initCheck()) {
+ return overlay;
+ }
+
+ ALOGE("%s: Failed to create HdrSdrRatioOverlay", __func__);
+ return {};
+}
+
+HdrSdrRatioOverlay::HdrSdrRatioOverlay(ConstructorTag)
: mSurfaceControl(
SurfaceControlHolder::createSurfaceControlHolder(String8("HdrSdrRatioOverlay"))) {
if (!mSurfaceControl) {
@@ -109,6 +120,10 @@
.apply();
}
+bool HdrSdrRatioOverlay::initCheck() const {
+ return mSurfaceControl != nullptr;
+}
+
void HdrSdrRatioOverlay::changeHdrSdrRatio(float currentHdrSdrRatio) {
mCurrentHdrSdrRatio = currentHdrSdrRatio;
animate();
diff --git a/services/surfaceflinger/HdrSdrRatioOverlay.h b/services/surfaceflinger/HdrSdrRatioOverlay.h
index 69f95ec..72d401d 100644
--- a/services/surfaceflinger/HdrSdrRatioOverlay.h
+++ b/services/surfaceflinger/HdrSdrRatioOverlay.h
@@ -25,15 +25,22 @@
namespace android {
class HdrSdrRatioOverlay {
+private:
+ // Effectively making the constructor private, while keeping std::make_unique work
+ struct ConstructorTag {};
+
public:
- HdrSdrRatioOverlay();
+ static std::unique_ptr<HdrSdrRatioOverlay> create();
+
void setLayerStack(ui::LayerStack);
void setViewport(ui::Size);
void animate();
void changeHdrSdrRatio(float currentRatio);
+ HdrSdrRatioOverlay(ConstructorTag);
+
private:
- float mCurrentHdrSdrRatio = 1.f;
+ bool initCheck() const;
static sp<GraphicBuffer> draw(float currentHdrSdrRatio, SkColor, ui::Transform::RotationFlags,
sp<GraphicBuffer>& ringBufer);
@@ -41,6 +48,7 @@
const sp<GraphicBuffer> getOrCreateBuffers(float currentHdrSdrRatio);
+ float mCurrentHdrSdrRatio = 1.f;
const std::unique_ptr<SurfaceControlHolder> mSurfaceControl;
size_t mIndex = 0;
diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp
index be04c09..42676c6 100644
--- a/services/surfaceflinger/RefreshRateOverlay.cpp
+++ b/services/surfaceflinger/RefreshRateOverlay.cpp
@@ -138,7 +138,20 @@
SegmentDrawer::drawDigit(number % 10, left, color, canvas);
}
-RefreshRateOverlay::RefreshRateOverlay(FpsRange fpsRange, ftl::Flags<Features> features)
+std::unique_ptr<RefreshRateOverlay> RefreshRateOverlay::create(FpsRange range,
+ ftl::Flags<Features> features) {
+ std::unique_ptr<RefreshRateOverlay> overlay =
+ std::make_unique<RefreshRateOverlay>(ConstructorTag{}, range, features);
+ if (overlay->initCheck()) {
+ return overlay;
+ }
+
+ ALOGE("%s: Failed to create RefreshRateOverlay", __func__);
+ return {};
+}
+
+RefreshRateOverlay::RefreshRateOverlay(ConstructorTag, FpsRange fpsRange,
+ ftl::Flags<Features> features)
: mFpsRange(fpsRange),
mFeatures(features),
mSurfaceControl(
@@ -154,6 +167,10 @@
.apply();
}
+bool RefreshRateOverlay::initCheck() const {
+ return mSurfaceControl != nullptr;
+}
+
auto RefreshRateOverlay::getOrCreateBuffers(Fps vsyncRate, Fps renderFps) -> const Buffers& {
static const Buffers kNoBuffers;
if (!mSurfaceControl) return kNoBuffers;
diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h
index ae334e5..0fec470 100644
--- a/services/surfaceflinger/RefreshRateOverlay.h
+++ b/services/surfaceflinger/RefreshRateOverlay.h
@@ -37,6 +37,10 @@
class SurfaceFlinger;
class RefreshRateOverlay {
+private:
+ // Effectively making the constructor private, while keeping std::make_unique work
+ struct ConstructorTag {};
+
public:
enum class Features {
Spinner = 1 << 0,
@@ -45,7 +49,7 @@
SetByHwc = 1 << 3,
};
- RefreshRateOverlay(FpsRange, ftl::Flags<Features>);
+ static std::unique_ptr<RefreshRateOverlay> create(FpsRange, ftl::Flags<Features>);
void setLayerStack(ui::LayerStack);
void setViewport(ui::Size);
@@ -54,7 +58,11 @@
void animate();
bool isSetByHwc() const { return mFeatures.test(RefreshRateOverlay::Features::SetByHwc); }
+ RefreshRateOverlay(ConstructorTag, FpsRange, ftl::Flags<Features>);
+
private:
+ bool initCheck() const;
+
using Buffers = std::vector<sp<GraphicBuffer>>;
static Buffers draw(int vsyncRate, int renderFps, SkColor, ui::Transform::RotationFlags,