Loosen requirement that getVsyncSchedule returns an object
In some cases, a display may have been removed in between referring to
a display and when we attempt to retrieve its VsyncSchedule. Remove the
assert that there must be an associated VsyncSchedule, replacing it with
handling that is specific to the call site.
Anywhere std::null_opt is passed to getVsyncSchedule (the default),
leave the call sites unchanged. This will pick the pacesetter, which
should have a VsyncSchedule every time the method is called. This should
always return a valid VsyncSchedule. Otherwise, the old code would
LOG_ALWAYS_FATAL, while the new code would have a null pointer
dereference.
LOG_ALWAYS_FATAL if there is no associated VsyncSchedule in the
following methods. They are all called on the main thread, when the
schedule should always be available when requested. This matches the
old behavior. Where these are not documented as main-thread only, add
this annotation, since that's what makes this safe to assume.
- enableHardwareVsync
- disableHardwareVsync
- setDisplayPowerMode
- addPresentFence
In addResyncSample, which is called from a binder thread, the display
may have been removed since the callback was initiated. No-op with a
warning in this case.
In getDisplayStats, which also may be called from a binder thread, fall
back to mActiveDisplayId, just in case it is called in between Scheduler
demoting and promoting the pacesetter display. Add a test for this
method.
In the following methods, log an error and early exit. This may be more
conservative than necessary.
- resyncToHardwareVsyncLocked
- setRenderRate
In setVsyncEnabled, fail to setPendingHardwareVsyncState silently,
which matches the behavior for the call to setHWCVsyncEnabled.
Bug: 266094575
Bug: 277623302
Bug: 267636813
Bug: 278806588
Bug: 275691508
Bug: 275691150
Bug: 277364366
Test: builds, boots
Test: SurfaceFlingerGetDisplayStatsTest
Change-Id: I3541909448745b04252e88f299a4283f37e755e8
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 48b4144..043ad86 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1146,17 +1146,26 @@
std::optional<PhysicalDisplayId> displayIdOpt;
{
Mutex::Autolock lock(mStateLock);
- displayIdOpt = getPhysicalDisplayIdLocked(displayToken);
+ if (displayToken) {
+ displayIdOpt = getPhysicalDisplayIdLocked(displayToken);
+ if (!displayIdOpt) {
+ ALOGW("%s: Invalid physical display token %p", __func__, displayToken.get());
+ return NAME_NOT_FOUND;
+ }
+ } else {
+ // TODO (b/277364366): Clients should be updated to pass in the display they
+ // want, rather than us picking an arbitrary one (the active display, in this
+ // case).
+ displayIdOpt = mActiveDisplayId;
+ }
}
- // TODO (b/277364366): Clients should be updated to pass in the display they
- // want, rather than us picking an arbitrary one (the pacesetter, in this
- // case).
- if (displayToken && !displayIdOpt) {
- ALOGE("%s: Invalid physical display token %p", __func__, displayToken.get());
+ const auto schedule = mScheduler->getVsyncSchedule(displayIdOpt);
+ if (!schedule) {
+ ALOGE("%s: Missing VSYNC schedule for display %s!", __func__,
+ to_string(*displayIdOpt).c_str());
return NAME_NOT_FOUND;
}
- const auto schedule = mScheduler->getVsyncSchedule(displayIdOpt);
outStats->vsyncTime = schedule->vsyncDeadlineAfter(TimePoint::now()).ns();
outStats->vsyncPeriod = schedule->period().ns();
return NO_ERROR;
@@ -2136,7 +2145,9 @@
static_cast<void>(mScheduler->schedule([=]() FTL_FAKE_GUARD(mStateLock) {
{
ftl::FakeGuard guard(kMainThreadContext);
- mScheduler->getVsyncSchedule(id)->setPendingHardwareVsyncState(enabled);
+ if (auto schedule = mScheduler->getVsyncSchedule(id)) {
+ schedule->setPendingHardwareVsyncState(enabled);
+ }
}
ATRACE_FORMAT("%s (%d) for %" PRIu64 " (main thread)", whence, enabled, id.value);