diff options
35 files changed, 546 insertions, 183 deletions
diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h index 109e28b9dc..d9a0253781 100644 --- a/libs/gui/include/gui/DisplayEventReceiver.h +++ b/libs/gui/include/gui/DisplayEventReceiver.h @@ -65,6 +65,7 @@ public: struct VSync { uint32_t count; + nsecs_t expectedVSyncTimestamp; }; struct Hotplug { diff --git a/libs/gui/tests/RegionSampling_test.cpp b/libs/gui/tests/RegionSampling_test.cpp index dbd4ef9d7e..6746b0a827 100644 --- a/libs/gui/tests/RegionSampling_test.cpp +++ b/libs/gui/tests/RegionSampling_test.cpp @@ -240,6 +240,19 @@ protected: float const luma_gray = 0.50; }; +TEST_F(RegionSamplingTest, invalidLayerHandle_doesNotCrash) { + sp<ISurfaceComposer> composer = ComposerService::getComposerService(); + sp<Listener> listener = new Listener(); + const Rect sampleArea{100, 100, 200, 200}; + // Passing in composer service as the layer handle should not crash, we'll + // treat it as a layer that no longer exists and silently allow sampling to + // occur. + status_t status = composer->addRegionSamplingListener(sampleArea, + IInterface::asBinder(composer), listener); + ASSERT_EQ(NO_ERROR, status); + composer->removeRegionSamplingListener(listener); +} + TEST_F(RegionSamplingTest, DISABLED_CollectsLuma) { fill_render(rgba_green); diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp index 3b68e0e097..45e67f74f2 100644 --- a/services/sensorservice/SensorDevice.cpp +++ b/services/sensorservice/SensorDevice.cpp @@ -20,7 +20,6 @@ #include "android/hardware/sensors/2.1/ISensorsCallback.h" #include "android/hardware/sensors/2.1/types.h" #include "convertV2_1.h" -#include "SensorService.h" #include <android-base/logging.h> #include <android/util/ProtoOutputStream.h> @@ -30,6 +29,7 @@ #include <utils/Errors.h> #include <utils/Singleton.h> +#include <cstddef> #include <chrono> #include <cinttypes> #include <thread> @@ -143,6 +143,25 @@ void SensorDevice::initializeSensorList() { for (size_t i=0 ; i < count; i++) { sensor_t sensor; convertToSensor(convertToOldSensorInfo(list[i]), &sensor); + + if (sensor.type < static_cast<int>(SensorType::DEVICE_PRIVATE_BASE)) { + if(sensor.resolution == 0) { + // Don't crash here or the device will go into a crashloop. + ALOGW("%s must have a non-zero resolution", sensor.name); + // For simple algos, map their resolution to 1 if it's not specified + sensor.resolution = + SensorDeviceUtils::defaultResolutionForType(sensor.type); + } + + double promotedResolution = sensor.resolution; + double promotedMaxRange = sensor.maxRange; + if (fmod(promotedMaxRange, promotedResolution) != 0) { + ALOGW("%s's max range %f is not a multiple of the resolution %f", + sensor.name, sensor.maxRange, sensor.resolution); + SensorDeviceUtils::quantizeValue(&sensor.maxRange, promotedResolution); + } + } + // Sanity check and clamp power if it is 0 (or close) if (sensor.power < minPowerMa) { ALOGI("Reported power %f not deemed sane, clamping to %f", @@ -403,8 +422,8 @@ std::string SensorDevice::dump() const { if (mSensors == nullptr) return "HAL not initialized\n"; String8 result; - result.appendFormat("Total %zu h/w sensors, %zu running:\n", - mSensorList.size(), mActivationCount.size()); + result.appendFormat("Total %zu h/w sensors, %zu running %zu disabled clients:\n", + mSensorList.size(), mActivationCount.size(), mDisabledClients.size()); Mutex::Autolock _l(mLock); for (const auto & s : mSensorList) { @@ -417,16 +436,18 @@ std::string SensorDevice::dump() const { result.append("sampling_period(ms) = {"); for (size_t j = 0; j < info.batchParams.size(); j++) { const BatchParams& params = info.batchParams[j]; - result.appendFormat("%.1f%s", params.mTSample / 1e6f, - j < info.batchParams.size() - 1 ? ", " : ""); + result.appendFormat("%.1f%s%s", params.mTSample / 1e6f, + isClientDisabledLocked(info.batchParams.keyAt(j)) ? "(disabled)" : "", + (j < info.batchParams.size() - 1) ? ", " : ""); } result.appendFormat("}, selected = %.2f ms; ", info.bestBatchParams.mTSample / 1e6f); result.append("batching_period(ms) = {"); for (size_t j = 0; j < info.batchParams.size(); j++) { const BatchParams& params = info.batchParams[j]; - result.appendFormat("%.1f%s", params.mTBatch / 1e6f, - j < info.batchParams.size() - 1 ? ", " : ""); + result.appendFormat("%.1f%s%s", params.mTBatch / 1e6f, + isClientDisabledLocked(info.batchParams.keyAt(j)) ? "(disabled)" : "", + (j < info.batchParams.size() - 1) ? ", " : ""); } result.appendFormat("}, selected = %.2f ms\n", info.bestBatchParams.mTBatch / 1e6f); } @@ -508,7 +529,7 @@ ssize_t SensorDevice::pollHal(sensors_event_t* buffer, size_t count) { const auto &events, const auto &dynamicSensorsAdded) { if (result == Result::OK) { - convertToSensorEvents(convertToNewEvents(events), + convertToSensorEventsAndQuantize(convertToNewEvents(events), convertToNewSensorInfos(dynamicSensorsAdded), buffer); err = (ssize_t)events.size(); } else { @@ -571,6 +592,8 @@ ssize_t SensorDevice::pollFmq(sensors_event_t* buffer, size_t maxNumEventsToRead for (size_t i = 0; i < eventsToRead; i++) { convertToSensorEvent(mEventBuffer[i], &buffer[i]); + android::SensorDeviceUtils::quantizeSensorEventValues(&buffer[i], + getResolutionForSensor(buffer[i].sensor)); } eventsRead = eventsToRead; } else { @@ -641,7 +664,7 @@ status_t SensorDevice::activate(void* ident, int handle, int enabled) { } status_t SensorDevice::activateLocked(void* ident, int handle, int enabled) { - bool actuateHardware = false; + bool activateHardware = false; status_t err(NO_ERROR); @@ -667,7 +690,7 @@ status_t SensorDevice::activateLocked(void* ident, int handle, int enabled) { if (info.batchParams.indexOfKey(ident) >= 0) { if (info.numActiveClients() > 0 && !info.isActive) { - actuateHardware = true; + activateHardware = true; } } else { // Log error. Every activate call should be preceded by a batch() call. @@ -687,7 +710,7 @@ status_t SensorDevice::activateLocked(void* ident, int handle, int enabled) { if (info.removeBatchParamsForIdent(ident) >= 0) { if (info.numActiveClients() == 0) { // This is the last connection, we need to de-activate the underlying h/w sensor. - actuateHardware = true; + activateHardware = true; } else { // Call batch for this sensor with the previously calculated best effort // batch_rate and timeout. One of the apps has unregistered for sensor @@ -707,12 +730,8 @@ status_t SensorDevice::activateLocked(void* ident, int handle, int enabled) { } } - if (actuateHardware) { - ALOGD_IF(DEBUG_CONNECTIONS, "\t>>> actuating h/w activate handle=%d enabled=%d", handle, - enabled); - err = checkReturnAndGetStatus(mSensors->activate(handle, enabled)); - ALOGE_IF(err, "Error %s sensor %d (%s)", enabled ? "activating" : "disabling", handle, - strerror(-err)); + if (activateHardware) { + err = doActivateHardwareLocked(handle, enabled); if (err != NO_ERROR && enabled) { // Failure when enabling the sensor. Clean up on failure. @@ -728,6 +747,15 @@ status_t SensorDevice::activateLocked(void* ident, int handle, int enabled) { return err; } +status_t SensorDevice::doActivateHardwareLocked(int handle, bool enabled) { + ALOGD_IF(DEBUG_CONNECTIONS, "\t>>> actuating h/w activate handle=%d enabled=%d", handle, + enabled); + status_t err = checkReturnAndGetStatus(mSensors->activate(handle, enabled)); + ALOGE_IF(err, "Error %s sensor %d (%s)", enabled ? "activating" : "disabling", handle, + strerror(-err)); + return err; +} + status_t SensorDevice::batch( void* ident, int handle, @@ -768,6 +796,18 @@ status_t SensorDevice::batchLocked(void* ident, int handle, int flags, int64_t s info.setBatchParamsForIdent(ident, flags, samplingPeriodNs, maxBatchReportLatencyNs); } + status_t err = updateBatchParamsLocked(handle, info); + if (err != NO_ERROR) { + ALOGE("sensor batch failed %p 0x%08x %" PRId64 " %" PRId64 " err=%s", + mSensors.get(), handle, info.bestBatchParams.mTSample, + info.bestBatchParams.mTBatch, strerror(-err)); + info.removeBatchParamsForIdent(ident); + } + + return err; +} + +status_t SensorDevice::updateBatchParamsLocked(int handle, Info &info) { BatchParams prevBestBatchParams = info.bestBatchParams; // Find the minimum of all timeouts and batch_rates for this sensor. info.selectBatchParams(); @@ -785,13 +825,8 @@ status_t SensorDevice::batchLocked(void* ident, int handle, int flags, int64_t s info.bestBatchParams.mTSample, info.bestBatchParams.mTBatch); err = checkReturnAndGetStatus(mSensors->batch( handle, info.bestBatchParams.mTSample, info.bestBatchParams.mTBatch)); - if (err != NO_ERROR) { - ALOGE("sensor batch failed %p 0x%08x %" PRId64 " %" PRId64 " err=%s", - mSensors.get(), handle, info.bestBatchParams.mTSample, - info.bestBatchParams.mTBatch, strerror(-err)); - info.removeBatchParamsForIdent(ident); - } } + return err; } @@ -811,13 +846,61 @@ status_t SensorDevice::flush(void* ident, int handle) { return checkReturnAndGetStatus(mSensors->flush(handle)); } -bool SensorDevice::isClientDisabled(void* ident) { +bool SensorDevice::isClientDisabled(void* ident) const { Mutex::Autolock _l(mLock); return isClientDisabledLocked(ident); } -bool SensorDevice::isClientDisabledLocked(void* ident) { - return mDisabledClients.indexOf(ident) >= 0; +bool SensorDevice::isClientDisabledLocked(void* ident) const { + return mDisabledClients.count(ident) > 0; +} + +std::vector<void *> SensorDevice::getDisabledClientsLocked() const { + std::vector<void *> vec; + for (const auto& it : mDisabledClients) { + vec.push_back(it.first); + } + + return vec; +} + +void SensorDevice::addDisabledReasonForIdentLocked(void* ident, DisabledReason reason) { + mDisabledClients[ident] |= 1 << reason; +} + +void SensorDevice::removeDisabledReasonForIdentLocked(void* ident, DisabledReason reason) { + if (isClientDisabledLocked(ident)) { + mDisabledClients[ident] &= ~(1 << reason); + if (mDisabledClients[ident] == 0) { + mDisabledClients.erase(ident); + } + } +} + +void SensorDevice::setUidStateForConnection(void* ident, SensorService::UidState state) { + Mutex::Autolock _l(mLock); + if (state == SensorService::UID_STATE_ACTIVE) { + removeDisabledReasonForIdentLocked(ident, DisabledReason::DISABLED_REASON_UID_IDLE); + } else { + addDisabledReasonForIdentLocked(ident, DisabledReason::DISABLED_REASON_UID_IDLE); + } + + for (size_t i = 0; i< mActivationCount.size(); ++i) { + int handle = mActivationCount.keyAt(i); + Info& info = mActivationCount.editValueAt(i); + + if (info.hasBatchParamsForIdent(ident)) { + if (updateBatchParamsLocked(handle, info) != NO_ERROR) { + bool enable = info.numActiveClients() == 0 && info.isActive; + bool disable = info.numActiveClients() > 0 && !info.isActive; + + if ((enable || disable) && + doActivateHardwareLocked(handle, enable) == NO_ERROR) { + info.isActive = enable; + } + } + } + } } bool SensorDevice::isSensorActive(int handle) const { @@ -832,8 +915,12 @@ bool SensorDevice::isSensorActive(int handle) const { void SensorDevice::enableAllSensors() { if (mSensors == nullptr) return; Mutex::Autolock _l(mLock); - mDisabledClients.clear(); - ALOGI("cleared mDisabledClients"); + + for (void *client : getDisabledClientsLocked()) { + removeDisabledReasonForIdentLocked( + client, DisabledReason::DISABLED_REASON_SERVICE_RESTRICTED); + } + for (size_t i = 0; i< mActivationCount.size(); ++i) { Info& info = mActivationCount.editValueAt(i); if (info.batchParams.isEmpty()) continue; @@ -873,7 +960,8 @@ void SensorDevice::disableAllSensors() { // Add all the connections that were registered for this sensor to the disabled // clients list. for (size_t j = 0; j < info.batchParams.size(); ++j) { - mDisabledClients.add(info.batchParams.keyAt(j)); + addDisabledReasonForIdentLocked( + info.batchParams.keyAt(j), DisabledReason::DISABLED_REASON_SERVICE_RESTRICTED); ALOGI("added %p to mDisabledClients", info.batchParams.keyAt(j)); } @@ -1048,7 +1136,7 @@ ssize_t SensorDevice::Info::removeBatchParamsForIdent(void* ident) { void SensorDevice::notifyConnectionDestroyed(void* ident) { Mutex::Autolock _l(mLock); - mDisabledClients.remove(ident); + mDisabledClients.erase(ident); } bool SensorDevice::isDirectReportSupported() const { @@ -1077,7 +1165,7 @@ void SensorDevice::convertToSensorEvent( } } -void SensorDevice::convertToSensorEvents( +void SensorDevice::convertToSensorEventsAndQuantize( const hidl_vec<Event> &src, const hidl_vec<SensorInfo> &dynamicSensorsAdded, sensors_event_t *dst) { @@ -1088,7 +1176,24 @@ void SensorDevice::convertToSensorEvents( for (size_t i = 0; i < src.size(); ++i) { V2_1::implementation::convertToSensorEvent(src[i], &dst[i]); + android::SensorDeviceUtils::quantizeSensorEventValues(&dst[i], + getResolutionForSensor(dst[i].sensor)); + } +} + +float SensorDevice::getResolutionForSensor(int sensorHandle) { + for (size_t i = 0; i < mSensorList.size(); i++) { + if (sensorHandle == mSensorList[i].handle) { + return mSensorList[i].resolution; + } } + + auto it = mConnectedDynamicSensors.find(sensorHandle); + if (it != mConnectedDynamicSensors.end()) { + return it->second->resolution; + } + + return 0; } void SensorDevice::handleHidlDeath(const std::string & detail) { diff --git a/services/sensorservice/SensorDevice.h b/services/sensorservice/SensorDevice.h index 24d03c63c2..5e7d3da398 100644 --- a/services/sensorservice/SensorDevice.h +++ b/services/sensorservice/SensorDevice.h @@ -18,6 +18,7 @@ #define ANDROID_SENSOR_DEVICE_H #include "SensorDeviceUtils.h" +#include "SensorService.h" #include "SensorServiceUtils.h" #include "ISensorsWrapper.h" @@ -116,6 +117,8 @@ public: hardware::Return<void> onDynamicSensorsDisconnected( const hardware::hidl_vec<int32_t> &dynamicSensorHandlesRemoved); + void setUidStateForConnection(void* ident, SensorService::UidState state); + bool isReconnecting() const { return mReconnecting; } @@ -179,6 +182,13 @@ private: // the removed ident. If index >=0, ident is present and successfully removed. ssize_t removeBatchParamsForIdent(void* ident); + bool hasBatchParamsForIdent(void* ident) const { + return batchParams.indexOfKey(ident) >= 0; + } + + /** + * @return The number of active clients of this sensor. + */ int numActiveClients() const; }; DefaultKeyedVector<int, Info> mActivationCount; @@ -187,8 +197,26 @@ private: SensorServiceUtil::RingBuffer<HidlTransportErrorLog> mHidlTransportErrors; int mTotalHidlTransportErrors; - // Use this vector to determine which client is activated or deactivated. - SortedVector<void *> mDisabledClients; + /** + * Enums describing the reason why a client was disabled. + */ + enum DisabledReason : uint8_t { + // UID becomes idle (e.g. app goes to background). + DISABLED_REASON_UID_IDLE = 0, + + // Sensors are restricted for all clients. + DISABLED_REASON_SERVICE_RESTRICTED, + DISABLED_REASON_MAX, + }; + + static_assert(DisabledReason::DISABLED_REASON_MAX < sizeof(uint8_t) * CHAR_BIT); + + // Use this map to determine which client is activated or deactivated. + std::unordered_map<void *, uint8_t> mDisabledClients; + + void addDisabledReasonForIdentLocked(void* ident, DisabledReason reason); + void removeDisabledReasonForIdentLocked(void* ident, DisabledReason reason); + SensorDevice(); bool connectHidlService(); void initializeSensorList(); @@ -214,6 +242,9 @@ private: status_t batchLocked(void* ident, int handle, int flags, int64_t samplingPeriodNs, int64_t maxBatchReportLatencyNs); + status_t updateBatchParamsLocked(int handle, Info& info); + status_t doActivateHardwareLocked(int handle, bool enable); + void handleHidlDeath(const std::string &detail); template<typename T> void checkReturn(const Return<T>& ret) { @@ -225,19 +256,24 @@ private: //TODO(b/67425500): remove waiter after bug is resolved. sp<SensorDeviceUtils::HidlServiceRegistrationWaiter> mRestartWaiter; - bool isClientDisabled(void* ident); - bool isClientDisabledLocked(void* ident); + bool isClientDisabled(void* ident) const; + bool isClientDisabledLocked(void* ident) const; + std::vector<void *> getDisabledClientsLocked() const; + + bool clientHasNoAccessLocked(void* ident) const; using Event = hardware::sensors::V2_1::Event; using SensorInfo = hardware::sensors::V2_1::SensorInfo; void convertToSensorEvent(const Event &src, sensors_event_t *dst); - void convertToSensorEvents( + void convertToSensorEventsAndQuantize( const hardware::hidl_vec<Event> &src, const hardware::hidl_vec<SensorInfo> &dynamicSensorsAdded, sensors_event_t *dst); + float getResolutionForSensor(int sensorHandle); + bool mIsDirectReportSupported; typedef hardware::MessageQueue<uint32_t, hardware::kSynchronizedReadWrite> WakeLockQueue; diff --git a/services/sensorservice/SensorDeviceUtils.cpp b/services/sensorservice/SensorDeviceUtils.cpp index dbafffe303..0dcf8c0ea2 100644 --- a/services/sensorservice/SensorDeviceUtils.cpp +++ b/services/sensorservice/SensorDeviceUtils.cpp @@ -17,17 +17,101 @@ #include "SensorDeviceUtils.h" #include <android/hardware/sensors/1.0/ISensors.h> +#include <android/hardware/sensors/2.1/ISensors.h> #include <utils/Log.h> #include <chrono> #include <thread> using ::android::hardware::Void; +using SensorTypeV2_1 = android::hardware::sensors::V2_1::SensorType; using namespace android::hardware::sensors::V1_0; namespace android { namespace SensorDeviceUtils { +void quantizeSensorEventValues(sensors_event_t *event, float resolution) { + LOG_FATAL_IF(resolution == 0, "Resolution must be specified for all sensors!"); + if (resolution == 0) { + return; + } + + size_t axes = 0; + switch ((SensorTypeV2_1)event->type) { + case SensorTypeV2_1::ACCELEROMETER: + case SensorTypeV2_1::MAGNETIC_FIELD: + case SensorTypeV2_1::ORIENTATION: + case SensorTypeV2_1::GYROSCOPE: + case SensorTypeV2_1::GRAVITY: + case SensorTypeV2_1::LINEAR_ACCELERATION: + case SensorTypeV2_1::MAGNETIC_FIELD_UNCALIBRATED: + case SensorTypeV2_1::GYROSCOPE_UNCALIBRATED: + case SensorTypeV2_1::ACCELEROMETER_UNCALIBRATED: + axes = 3; + break; + case SensorTypeV2_1::GAME_ROTATION_VECTOR: + axes = 4; + break; + case SensorTypeV2_1::ROTATION_VECTOR: + case SensorTypeV2_1::GEOMAGNETIC_ROTATION_VECTOR: + axes = 5; + break; + case SensorTypeV2_1::DEVICE_ORIENTATION: + case SensorTypeV2_1::LIGHT: + case SensorTypeV2_1::PRESSURE: + case SensorTypeV2_1::TEMPERATURE: + case SensorTypeV2_1::PROXIMITY: + case SensorTypeV2_1::RELATIVE_HUMIDITY: + case SensorTypeV2_1::AMBIENT_TEMPERATURE: + case SensorTypeV2_1::SIGNIFICANT_MOTION: + case SensorTypeV2_1::STEP_DETECTOR: + case SensorTypeV2_1::TILT_DETECTOR: + case SensorTypeV2_1::WAKE_GESTURE: + case SensorTypeV2_1::GLANCE_GESTURE: + case SensorTypeV2_1::PICK_UP_GESTURE: + case SensorTypeV2_1::WRIST_TILT_GESTURE: + case SensorTypeV2_1::STATIONARY_DETECT: + case SensorTypeV2_1::MOTION_DETECT: + case SensorTypeV2_1::HEART_BEAT: + case SensorTypeV2_1::LOW_LATENCY_OFFBODY_DETECT: + case SensorTypeV2_1::HINGE_ANGLE: + axes = 1; + break; + case SensorTypeV2_1::POSE_6DOF: + axes = 15; + break; + default: + // No other sensors have data that needs to be rounded. + break; + } + + // sensor_event_t is a union so we're able to perform the same quanitization action for most + // sensors by only knowing the number of axes their output data has. + for (size_t i = 0; i < axes; i++) { + quantizeValue(&event->data[i], resolution); + } +} + +float defaultResolutionForType(int type) { + switch ((SensorTypeV2_1)type) { + case SensorTypeV2_1::SIGNIFICANT_MOTION: + case SensorTypeV2_1::STEP_DETECTOR: + case SensorTypeV2_1::STEP_COUNTER: + case SensorTypeV2_1::TILT_DETECTOR: + case SensorTypeV2_1::WAKE_GESTURE: + case SensorTypeV2_1::GLANCE_GESTURE: + case SensorTypeV2_1::PICK_UP_GESTURE: + case SensorTypeV2_1::WRIST_TILT_GESTURE: + case SensorTypeV2_1::STATIONARY_DETECT: + case SensorTypeV2_1::MOTION_DETECT: + return 1.0f; + default: + // fall through and return 0 for all other types + break; + } + return 0.0f; +} + HidlServiceRegistrationWaiter::HidlServiceRegistrationWaiter() { } diff --git a/services/sensorservice/SensorDeviceUtils.h b/services/sensorservice/SensorDeviceUtils.h index e2eb606b9a..d7e621c035 100644 --- a/services/sensorservice/SensorDeviceUtils.h +++ b/services/sensorservice/SensorDeviceUtils.h @@ -18,7 +18,9 @@ #define ANDROID_SENSOR_DEVICE_UTIL #include <android/hidl/manager/1.0/IServiceNotification.h> +#include <hardware/sensors.h> +#include <cmath> #include <condition_variable> #include <thread> @@ -29,6 +31,21 @@ using ::android::hidl::manager::V1_0::IServiceNotification; namespace android { namespace SensorDeviceUtils { +// Quantizes a single value using a sensor's resolution. +inline void quantizeValue(float *value, double resolution) { + // Increase the value of the sensor's nominal resolution to ensure that + // sensor accuracy improvements, like runtime calibration, are not masked + // during requantization. + double incRes = 0.25 * resolution; + *value = round(static_cast<double>(*value) / incRes) * incRes; +} + +// Ensures a sensor event doesn't provide values finer grained than its sensor resolution allows. +void quantizeSensorEventValues(sensors_event_t *event, float resolution); + +// Provides a default resolution for simple sensor types if one wasn't provided by the HAL. +float defaultResolutionForType(int type); + class HidlServiceRegistrationWaiter : public IServiceNotification { public: diff --git a/services/sensorservice/SensorDirectConnection.cpp b/services/sensorservice/SensorDirectConnection.cpp index 106efd6634..e4c33dafcb 100644 --- a/services/sensorservice/SensorDirectConnection.cpp +++ b/services/sensorservice/SensorDirectConnection.cpp @@ -93,6 +93,18 @@ sp<BitTube> SensorService::SensorDirectConnection::getSensorChannel() const { return nullptr; } +void SensorService::SensorDirectConnection::onSensorAccessChanged(bool hasAccess) { + if (!hasAccess) { + stopAll(true /* backupRecord */); + } else { + recoverAll(); + } +} + +bool SensorService::SensorDirectConnection::hasSensorAccess() const { + return mService->hasSensorAccess(mUid, mOpPackageName); +} + status_t SensorService::SensorDirectConnection::enableDisable( int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) { @@ -125,7 +137,7 @@ int32_t SensorService::SensorDirectConnection::configureChannel(int handle, int return NO_ERROR; } - if (!mService->isOperationPermitted(mOpPackageName)) { + if (!hasSensorAccess()) { return PERMISSION_DENIED; } @@ -169,12 +181,15 @@ int32_t SensorService::SensorDirectConnection::configureChannel(int handle, int } void SensorService::SensorDirectConnection::stopAll(bool backupRecord) { + Mutex::Autolock _l(mConnectionLock); + stopAllLocked(backupRecord); +} +void SensorService::SensorDirectConnection::stopAllLocked(bool backupRecord) { struct sensors_direct_cfg_t config = { .rate_level = SENSOR_DIRECT_RATE_STOP }; - Mutex::Autolock _l(mConnectionLock); SensorDevice& dev(SensorDevice::getInstance()); for (auto &i : mActivated) { dev.configureDirectChannel(i.first, getHalChannelHandle(), &config); @@ -187,21 +202,25 @@ void SensorService::SensorDirectConnection::stopAll(bool backupRecord) { } void SensorService::SensorDirectConnection::recoverAll() { - stopAll(false); - Mutex::Autolock _l(mConnectionLock); - SensorDevice& dev(SensorDevice::getInstance()); - - // recover list of report from backup - mActivated = mActivatedBackup; - mActivatedBackup.clear(); - - // re-enable them - for (auto &i : mActivated) { - struct sensors_direct_cfg_t config = { - .rate_level = i.second - }; - dev.configureDirectChannel(i.first, getHalChannelHandle(), &config); + if (!mActivatedBackup.empty()) { + stopAllLocked(false); + + SensorDevice& dev(SensorDevice::getInstance()); + + // recover list of report from backup + ALOG_ASSERT(mActivated.empty(), + "mActivated must be empty if mActivatedBackup was non-empty"); + mActivated = mActivatedBackup; + mActivatedBackup.clear(); + + // re-enable them + for (auto &i : mActivated) { + struct sensors_direct_cfg_t config = { + .rate_level = i.second + }; + dev.configureDirectChannel(i.first, getHalChannelHandle(), &config); + } } } diff --git a/services/sensorservice/SensorDirectConnection.h b/services/sensorservice/SensorDirectConnection.h index ead08d3fb6..4181b65b7d 100644 --- a/services/sensorservice/SensorDirectConnection.h +++ b/services/sensorservice/SensorDirectConnection.h @@ -42,17 +42,14 @@ public: void dump(String8& result) const; void dump(util::ProtoOutputStream* proto) const; uid_t getUid() const { return mUid; } + const String16& getOpPackageName() const { return mOpPackageName; } int32_t getHalChannelHandle() const; bool isEquivalent(const sensors_direct_mem_t *mem) const; - // stop all active sensor report. if backupRecord is set to false, - // those report can be recovered by recoverAll - // called by SensorService when enter restricted mode - void stopAll(bool backupRecord = false); - - // recover sensor reports previously stopped by stopAll(true) - // called by SensorService when return to NORMAL mode. - void recoverAll(); + // Invoked when access to sensors for this connection has changed, e.g. lost or + // regained due to changes in the sensor restricted/privacy mode or the + // app changed to idle/active status. + void onSensorAccessChanged(bool hasAccess); protected: virtual ~SensorDirectConnection(); @@ -66,6 +63,25 @@ protected: virtual int32_t configureChannel(int handle, int rateLevel); virtual void destroy(); private: + bool hasSensorAccess() const; + + // Stops all active sensor direct report requests. + // + // If backupRecord is true, stopped requests can be recovered + // by a subsequent recoverAll() call (e.g. when temporarily stopping + // sensors for sensor privacy/restrict mode or when an app becomes + // idle). + void stopAll(bool backupRecord = false); + // Same as stopAll() but with mConnectionLock held. + void stopAllLocked(bool backupRecord); + + // Recover sensor requests previously stopped by stopAll(true). + // This method can be called when a sensor access resumes (e.g. + // sensor privacy/restrict mode lifted or app becomes active). + // + // If no requests are backed up by stopAll(), this method is no-op. + void recoverAll(); + const sp<SensorService> mService; const uid_t mUid; const sensors_direct_mem_t mMem; diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index e79937295b..ccf05d916e 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -31,12 +31,11 @@ namespace android { SensorService::SensorEventConnection::SensorEventConnection( const sp<SensorService>& service, uid_t uid, String8 packageName, bool isDataInjectionMode, - const String16& opPackageName, bool hasSensorAccess) + const String16& opPackageName) : mService(service), mUid(uid), mWakeLockRefCount(0), mHasLooperCallbacks(false), mDead(false), mDataInjectionMode(isDataInjectionMode), mEventCache(nullptr), mCacheSize(0), mMaxCacheSize(0), mTimeOfLastEventDrop(0), mEventsDropped(0), - mPackageName(packageName), mOpPackageName(opPackageName), mDestroyed(false), - mHasSensorAccess(hasSensorAccess) { + mPackageName(packageName), mOpPackageName(opPackageName), mDestroyed(false) { mChannel = new BitTube(mService->mSocketBufferSize); #if DEBUG_CONNECTIONS mEventsReceived = mEventsSentFromCache = mEventsSent = 0; @@ -431,13 +430,9 @@ status_t SensorService::SensorEventConnection::sendEvents( return size < 0 ? status_t(size) : status_t(NO_ERROR); } -void SensorService::SensorEventConnection::setSensorAccess(const bool hasAccess) { - Mutex::Autolock _l(mConnectionLock); - mHasSensorAccess = hasAccess; -} - bool SensorService::SensorEventConnection::hasSensorAccess() { - return mHasSensorAccess && !mService->mSensorPrivacyPolicy->isSensorPrivacyEnabled(); + return mService->isUidActive(mUid) + && !mService->mSensorPrivacyPolicy->isSensorPrivacyEnabled(); } bool SensorService::SensorEventConnection::noteOpIfRequired(const sensors_event_t& event) { diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index 1ca35c02cf..13cee6fa3c 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -49,8 +49,7 @@ class SensorService::SensorEventConnection: public: SensorEventConnection(const sp<SensorService>& service, uid_t uid, String8 packageName, - bool isDataInjectionMode, const String16& opPackageName, - bool hasSensorAccess); + bool isDataInjectionMode, const String16& opPackageName); status_t sendEvents(sensors_event_t const* buffer, size_t count, sensors_event_t* scratch, wp<const SensorEventConnection> const * mapFlushEventsToConnections = nullptr); @@ -69,8 +68,6 @@ public: uid_t getUid() const { return mUid; } - void setSensorAccess(const bool hasAccess); - private: virtual ~SensorEventConnection(); virtual void onFirstRef(); @@ -185,7 +182,6 @@ private: mutable Mutex mDestroyLock; bool mDestroyed; - bool mHasSensorAccess; // Store a mapping of sensor handles to required AppOp for a sensor. This map only contains a // valid mapping for sensors that require a permission in order to reduce the lookup time. diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index 5fdc74fb8c..ffcd0a0f3d 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -299,15 +299,35 @@ void SensorService::onFirstRef() { } } -void SensorService::setSensorAccess(uid_t uid, bool hasAccess) { +void SensorService::onUidStateChanged(uid_t uid, UidState state) { + SensorDevice& dev(SensorDevice::getInstance()); + ConnectionSafeAutolock connLock = mConnectionHolder.lock(mLock); for (const sp<SensorEventConnection>& conn : connLock.getActiveConnections()) { if (conn->getUid() == uid) { - conn->setSensorAccess(hasAccess); + dev.setUidStateForConnection(conn.get(), state); + } + } + + for (const sp<SensorDirectConnection>& conn : connLock.getDirectConnections()) { + if (conn->getUid() == uid) { + // Update sensor subscriptions if needed + bool hasAccess = hasSensorAccessLocked(conn->getUid(), conn->getOpPackageName()); + conn->onSensorAccessChanged(hasAccess); } } } +bool SensorService::hasSensorAccess(uid_t uid, const String16& opPackageName) { + Mutex::Autolock _l(mLock); + return hasSensorAccessLocked(uid, opPackageName); +} + +bool SensorService::hasSensorAccessLocked(uid_t uid, const String16& opPackageName) { + return !mSensorPrivacyPolicy->isSensorPrivacyEnabled() + && isUidActive(uid) && !isOperationRestrictedLocked(opPackageName); +} + const Sensor& SensorService::registerSensor(SensorInterface* s, bool isDebug, bool isVirtual) { int handle = s->getSensor().getHandle(); int type = s->getSensor().getType(); @@ -638,8 +658,9 @@ void SensorService::disableAllSensors() { void SensorService::disableAllSensorsLocked(ConnectionSafeAutolock* connLock) { SensorDevice& dev(SensorDevice::getInstance()); - for (const sp<SensorDirectConnection>& connection : connLock->getDirectConnections()) { - connection->stopAll(true /* backupRecord */); + for (const sp<SensorDirectConnection>& conn : connLock->getDirectConnections()) { + bool hasAccess = hasSensorAccessLocked(conn->getUid(), conn->getOpPackageName()); + conn->onSensorAccessChanged(hasAccess); } dev.disableAllSensors(); // Clear all pending flush connections for all active sensors. If one of the active @@ -666,8 +687,9 @@ void SensorService::enableAllSensorsLocked(ConnectionSafeAutolock* connLock) { } SensorDevice& dev(SensorDevice::getInstance()); dev.enableAllSensors(); - for (const sp<SensorDirectConnection>& connection : connLock->getDirectConnections()) { - connection->recoverAll(); + for (const sp<SensorDirectConnection>& conn : connLock->getDirectConnections()) { + bool hasAccess = hasSensorAccessLocked(conn->getUid(), conn->getOpPackageName()); + conn->onSensorAccessChanged(hasAccess); } } @@ -1234,9 +1256,8 @@ sp<ISensorEventConnection> SensorService::createSensorEventConnection(const Stri (packageName == "") ? String8::format("unknown_package_pid_%d", pid) : packageName; String16 connOpPackageName = (opPackageName == String16("")) ? String16(connPackageName) : opPackageName; - bool hasSensorAccess = mUidPolicy->isUidActive(uid); sp<SensorEventConnection> result(new SensorEventConnection(this, uid, connPackageName, - requestedMode == DATA_INJECTION, connOpPackageName, hasSensorAccess)); + requestedMode == DATA_INJECTION, connOpPackageName)); if (requestedMode == DATA_INJECTION) { mConnectionHolder.addEventConnectionIfNotPresent(result); // Add the associated file descriptor to the Looper for polling whenever there is data to @@ -1887,13 +1908,12 @@ bool SensorService::isWhiteListedPackage(const String8& packageName) { return (packageName.contains(mWhiteListedPackage.string())); } -bool SensorService::isOperationPermitted(const String16& opPackageName) { - Mutex::Autolock _l(mLock); +bool SensorService::isOperationRestrictedLocked(const String16& opPackageName) { if (mCurrentOperatingMode == RESTRICTED) { String8 package(opPackageName); - return isWhiteListedPackage(package); + return !isWhiteListedPackage(package); } - return true; + return false; } void SensorService::UidPolicy::registerSelf() { @@ -1921,7 +1941,7 @@ void SensorService::UidPolicy::onUidActive(uid_t uid) { } sp<SensorService> service = mService.promote(); if (service != nullptr) { - service->setSensorAccess(uid, true); + service->onUidStateChanged(uid, UID_STATE_ACTIVE); } } @@ -1936,7 +1956,7 @@ void SensorService::UidPolicy::onUidIdle(uid_t uid, __unused bool disabled) { if (deleted) { sp<SensorService> service = mService.promote(); if (service != nullptr) { - service->setSensorAccess(uid, false); + service->onUidStateChanged(uid, UID_STATE_IDLE); } } } @@ -1964,7 +1984,7 @@ void SensorService::UidPolicy::updateOverrideUid(uid_t uid, bool active, bool in if (wasActive != isActive) { sp<SensorService> service = mService.promote(); if (service != nullptr) { - service->setSensorAccess(uid, isActive); + service->onUidStateChanged(uid, isActive ? UID_STATE_ACTIVE : UID_STATE_IDLE); } } } @@ -1990,6 +2010,10 @@ bool SensorService::UidPolicy::isUidActiveLocked(uid_t uid) { return mActiveUids.find(uid) != mActiveUids.end(); } +bool SensorService::isUidActive(uid_t uid) { + return mUidPolicy->isUidActive(uid); +} + void SensorService::SensorPrivacyPolicy::registerSelf() { SensorPrivacyManager spm; mSensorPrivacyEnabled = spm.isSensorPrivacyEnabled(); diff --git a/services/sensorservice/SensorService.h b/services/sensorservice/SensorService.h index 7d17ddabf1..3bb8421a14 100644 --- a/services/sensorservice/SensorService.h +++ b/services/sensorservice/SensorService.h @@ -75,6 +75,11 @@ class SensorService : class SensorDirectConnection; public: + enum UidState { + UID_STATE_ACTIVE = 0, + UID_STATE_IDLE, + }; + void cleanupConnection(SensorEventConnection* connection); void cleanupConnection(SensorDirectConnection* c); @@ -194,6 +199,8 @@ private: std::unordered_map<uid_t, bool> mOverrideUids; }; + bool isUidActive(uid_t uid); + // Sensor privacy allows a user to disable access to all sensors on the device. When // enabled sensor privacy will prevent all apps, including active apps, from accessing // sensors, they will not receive trigger nor on-change events, flush event behavior @@ -332,7 +339,11 @@ private: // allowed to register for or call flush on sensors. Typically only cts test packages are // allowed. bool isWhiteListedPackage(const String8& packageName); - bool isOperationPermitted(const String16& opPackageName); + + // Returns true if a connection with the specified opPackageName has no access to sensors + // in the RESTRICTED mode (i.e. the service is in RESTRICTED mode, and the package is not + // whitelisted). mLock must be held to invoke this method. + bool isOperationRestrictedLocked(const String16& opPackageName); // Reset the state of SensorService to NORMAL mode. status_t resetToNormalMode(); @@ -349,7 +360,13 @@ private: void enableSchedFifoMode(); // Sets whether the given UID can get sensor data - void setSensorAccess(uid_t uid, bool hasAccess); + void onUidStateChanged(uid_t uid, UidState state); + + // Returns true if a connection with the given uid and opPackageName + // currently has access to sensors. + bool hasSensorAccess(uid_t uid, const String16& opPackageName); + // Same as hasSensorAccess but with mLock held. + bool hasSensorAccessLocked(uid_t uid, const String16& opPackageName); // Overrides the UID state as if it is idle status_t handleSetUidState(Vector<String16>& args, int err); diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 0a0f2f132e..2e7fbc1a78 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -116,9 +116,13 @@ sp<GraphicBuffer> RefreshRateOverlay::SevenSegmentDrawer::drawNumber(int number, sp<GraphicBuffer> buffer = new GraphicBuffer(BUFFER_WIDTH, BUFFER_HEIGHT, HAL_PIXEL_FORMAT_RGBA_8888, 1, - GRALLOC_USAGE_SW_WRITE_RARELY, "RefreshRateOverlayBuffer"); + GRALLOC_USAGE_SW_WRITE_RARELY | GRALLOC_USAGE_HW_COMPOSER | + GRALLOC_USAGE_HW_TEXTURE, + "RefreshRateOverlayBuffer"); uint8_t* pixels; buffer->lock(GRALLOC_USAGE_SW_WRITE_RARELY, reinterpret_cast<void**>(&pixels)); + // Clear buffer content + drawRect(Rect(BUFFER_WIDTH, BUFFER_HEIGHT), half4(0), buffer, pixels); int left = 0; if (hundreds != 0) { drawDigit(hundreds, left, color, buffer, pixels); diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 6e59034133..27353d8c0b 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -130,7 +130,7 @@ private: mVsyncListening = false; } - void onDispSyncEvent(nsecs_t /* when */) final { + void onDispSyncEvent(nsecs_t /*when*/, nsecs_t /*expectedVSyncTimestamp*/) final { std::unique_lock<decltype(mMutex)> lock(mMutex); if (mPhaseIntervalSetting == Phase::ZERO) { @@ -199,13 +199,8 @@ RegionSamplingThread::~RegionSamplingThread() { } } -void RegionSamplingThread::addListener(const Rect& samplingArea, const sp<IBinder>& stopLayerHandle, +void RegionSamplingThread::addListener(const Rect& samplingArea, const wp<Layer>& stopLayer, const sp<IRegionSamplingListener>& listener) { - wp<Layer> stopLayer; - if (stopLayerHandle != nullptr && stopLayerHandle->localBinder() != nullptr) { - stopLayer = static_cast<Layer::Handle*>(stopLayerHandle.get())->owner; - } - sp<IBinder> asBinder = IInterface::asBinder(listener); asBinder->linkToDeath(this); std::lock_guard lock(mSamplingMutex); diff --git a/services/surfaceflinger/RegionSamplingThread.h b/services/surfaceflinger/RegionSamplingThread.h index 99c07c288e..b9b7a3c436 100644 --- a/services/surfaceflinger/RegionSamplingThread.h +++ b/services/surfaceflinger/RegionSamplingThread.h @@ -69,7 +69,7 @@ public: // Add a listener to receive luma notifications. The luma reported via listener will // report the median luma for the layers under the stopLayerHandle, in the samplingArea region. - void addListener(const Rect& samplingArea, const sp<IBinder>& stopLayerHandle, + void addListener(const Rect& samplingArea, const wp<Layer>& stopLayer, const sp<IRegionSamplingListener>& listener); // Remove the listener to stop receiving median luma notifications. void removeListener(const sp<IRegionSamplingListener>& listener); diff --git a/services/surfaceflinger/Scheduler/DispSync.cpp b/services/surfaceflinger/Scheduler/DispSync.cpp index fc6ccaeb6e..ff91bf7bc0 100644 --- a/services/surfaceflinger/Scheduler/DispSync.cpp +++ b/services/surfaceflinger/Scheduler/DispSync.cpp @@ -200,7 +200,8 @@ public: } } - callbackInvocations = gatherCallbackInvocationsLocked(now); + callbackInvocations = + gatherCallbackInvocationsLocked(now, computeNextRefreshLocked(0, now)); } if (callbackInvocations.size() > 0) { @@ -303,6 +304,11 @@ public: return BAD_VALUE; } + nsecs_t computeNextRefresh(int periodOffset, nsecs_t now) const { + Mutex::Autolock lock(mMutex); + return computeNextRefreshLocked(periodOffset, now); + } + private: struct EventListener { const char* mName; @@ -315,6 +321,7 @@ private: struct CallbackInvocation { DispSync::Callback* mCallback; nsecs_t mEventTime; + nsecs_t mExpectedVSyncTime; }; nsecs_t computeNextEventTimeLocked(nsecs_t now) { @@ -340,7 +347,8 @@ private: return duration < (3 * mPeriod) / 5; } - std::vector<CallbackInvocation> gatherCallbackInvocationsLocked(nsecs_t now) { + std::vector<CallbackInvocation> gatherCallbackInvocationsLocked(nsecs_t now, + nsecs_t expectedVSyncTime) { if (mTraceDetailedInfo) ATRACE_CALL(); ALOGV("[%s] gatherCallbackInvocationsLocked @ %" PRId64, mName, ns2us(now)); @@ -361,6 +369,10 @@ private: CallbackInvocation ci; ci.mCallback = eventListener.mCallback; ci.mEventTime = t; + ci.mExpectedVSyncTime = expectedVSyncTime; + if (eventListener.mPhase < 0) { + ci.mExpectedVSyncTime += mPeriod; + } ALOGV("[%s] [%s] Preparing to fire, latency: %" PRId64, mName, eventListener.mName, t - eventListener.mLastEventTime); callbackInvocations.push_back(ci); @@ -426,8 +438,17 @@ private: void fireCallbackInvocations(const std::vector<CallbackInvocation>& callbacks) { if (mTraceDetailedInfo) ATRACE_CALL(); for (size_t i = 0; i < callbacks.size(); i++) { - callbacks[i].mCallback->onDispSyncEvent(callbacks[i].mEventTime); + callbacks[i].mCallback->onDispSyncEvent(callbacks[i].mEventTime, + callbacks[i].mExpectedVSyncTime); + } + } + + nsecs_t computeNextRefreshLocked(int periodOffset, nsecs_t now) const { + nsecs_t phase = mReferenceTime + mPhase; + if (mPeriod == 0) { + return 0; } + return (((now - phase) / mPeriod) + periodOffset + 1) * mPeriod + phase; } const char* const mName; @@ -444,7 +465,7 @@ private: std::vector<EventListener> mEventListeners; - Mutex mMutex; + mutable Mutex mMutex; Condition mCond; // Flag to turn on logging in systrace. @@ -458,7 +479,7 @@ class ZeroPhaseTracer : public DispSync::Callback { public: ZeroPhaseTracer() : mParity("ZERO_PHASE_VSYNC", false) {} - virtual void onDispSyncEvent(nsecs_t /*when*/) { + virtual void onDispSyncEvent(nsecs_t /*when*/, nsecs_t /*expectedVSyncTimestamp*/) { mParity = !mParity; } @@ -845,7 +866,7 @@ nsecs_t DispSync::expectedPresentTime(nsecs_t now) { const uint32_t hwcLatency = 0; // Ask DispSync when the next refresh will be (CLOCK_MONOTONIC). - return computeNextRefresh(hwcLatency, now); + return mThread->computeNextRefresh(hwcLatency, now); } } // namespace impl diff --git a/services/surfaceflinger/Scheduler/DispSync.h b/services/surfaceflinger/Scheduler/DispSync.h index 2faa81ce41..832f08e07b 100644 --- a/services/surfaceflinger/Scheduler/DispSync.h +++ b/services/surfaceflinger/Scheduler/DispSync.h @@ -36,7 +36,7 @@ public: public: Callback() = default; virtual ~Callback(); - virtual void onDispSyncEvent(nsecs_t when) = 0; + virtual void onDispSyncEvent(nsecs_t when, nsecs_t expectedVSyncTimestamp) = 0; protected: Callback(Callback const&) = delete; diff --git a/services/surfaceflinger/Scheduler/DispSyncSource.cpp b/services/surfaceflinger/Scheduler/DispSyncSource.cpp index 4e3f85f53e..8752b6600e 100644 --- a/services/surfaceflinger/Scheduler/DispSyncSource.cpp +++ b/services/surfaceflinger/Scheduler/DispSyncSource.cpp @@ -92,7 +92,7 @@ void DispSyncSource::setPhaseOffset(nsecs_t phaseOffset) { } } -void DispSyncSource::onDispSyncEvent(nsecs_t when) { +void DispSyncSource::onDispSyncEvent(nsecs_t when, nsecs_t expectedVSyncTimestamp) { VSyncSource::Callback* callback; { std::lock_guard lock(mCallbackMutex); @@ -104,7 +104,7 @@ void DispSyncSource::onDispSyncEvent(nsecs_t when) { } if (callback != nullptr) { - callback->onVSyncEvent(when); + callback->onVSyncEvent(when, expectedVSyncTimestamp); } } diff --git a/services/surfaceflinger/Scheduler/DispSyncSource.h b/services/surfaceflinger/Scheduler/DispSyncSource.h index f278712407..2aee3f6321 100644 --- a/services/surfaceflinger/Scheduler/DispSyncSource.h +++ b/services/surfaceflinger/Scheduler/DispSyncSource.h @@ -40,7 +40,7 @@ public: private: // The following method is the implementation of the DispSync::Callback. - virtual void onDispSyncEvent(nsecs_t when); + void onDispSyncEvent(nsecs_t when, nsecs_t expectedVSyncTimestamp) override; const char* const mName; TracedOrdinal<int> mValue; diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index 8347650636..5dedb6a1e7 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -79,8 +79,9 @@ std::string toString(const DisplayEventReceiver::Event& event) { event.hotplug.connected ? "connected" : "disconnected"); case DisplayEventReceiver::DISPLAY_EVENT_VSYNC: return StringPrintf("VSync{displayId=%" ANDROID_PHYSICAL_DISPLAY_ID_FORMAT - ", count=%u}", - event.header.displayId, event.vsync.count); + ", count=%u, expectedVSyncTimestamp=%" PRId64 "}", + event.header.displayId, event.vsync.count, + event.vsync.expectedVSyncTimestamp); case DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED: return StringPrintf("ConfigChanged{displayId=%" ANDROID_PHYSICAL_DISPLAY_ID_FORMAT ", configId=%u}", @@ -99,10 +100,11 @@ DisplayEventReceiver::Event makeHotplug(PhysicalDisplayId displayId, nsecs_t tim } DisplayEventReceiver::Event makeVSync(PhysicalDisplayId displayId, nsecs_t timestamp, - uint32_t count) { + uint32_t count, nsecs_t expectedVSyncTimestamp) { DisplayEventReceiver::Event event; event.header = {DisplayEventReceiver::DISPLAY_EVENT_VSYNC, displayId, timestamp}; event.vsync.count = count; + event.vsync.expectedVSyncTimestamp = expectedVSyncTimestamp; return event; } @@ -312,11 +314,12 @@ void EventThread::onScreenAcquired() { mCondition.notify_all(); } -void EventThread::onVSyncEvent(nsecs_t timestamp) { +void EventThread::onVSyncEvent(nsecs_t timestamp, nsecs_t expectedVSyncTimestamp) { std::lock_guard<std::mutex> lock(mMutex); LOG_FATAL_IF(!mVSyncState); - mPendingEvents.push_back(makeVSync(mVSyncState->displayId, timestamp, ++mVSyncState->count)); + mPendingEvents.push_back(makeVSync(mVSyncState->displayId, timestamp, ++mVSyncState->count, + expectedVSyncTimestamp)); mCondition.notify_all(); } @@ -423,7 +426,8 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { } else { // Generate a fake VSYNC after a long timeout in case the driver stalls. When the // display is off, keep feeding clients at 60 Hz. - const auto timeout = mState == State::SyntheticVSync ? 16ms : 1000ms; + const std::chrono::nanoseconds timeout = + mState == State::SyntheticVSync ? 16ms : 1000ms; if (mCondition.wait_for(lock, timeout) == std::cv_status::timeout) { if (mState == State::VSync) { ALOGW("Faking VSYNC due to driver stall for thread %s", mThreadName); @@ -439,9 +443,10 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { } LOG_FATAL_IF(!mVSyncState); - mPendingEvents.push_back(makeVSync(mVSyncState->displayId, - systemTime(SYSTEM_TIME_MONOTONIC), - ++mVSyncState->count)); + const auto now = systemTime(SYSTEM_TIME_MONOTONIC); + const auto expectedVSyncTime = now + timeout.count(); + mPendingEvents.push_back(makeVSync(mVSyncState->displayId, now, + ++mVSyncState->count, expectedVSyncTime)); } } } diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index 98b1876994..9e7086eb0c 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -57,7 +57,7 @@ public: class Callback { public: virtual ~Callback() {} - virtual void onVSyncEvent(nsecs_t when) = 0; + virtual void onVSyncEvent(nsecs_t when, nsecs_t expectedVSyncTimestamp) = 0; }; virtual ~VSyncSource() {} @@ -189,7 +189,7 @@ private: REQUIRES(mMutex); // Implements VSyncSource::Callback - void onVSyncEvent(nsecs_t timestamp) override; + void onVSyncEvent(nsecs_t timestamp, nsecs_t expectedVSyncTimestamp) override; const std::unique_ptr<VSyncSource> mVSyncSource GUARDED_BY(mMutex); diff --git a/services/surfaceflinger/Scheduler/InjectVSyncSource.h b/services/surfaceflinger/Scheduler/InjectVSyncSource.h index 31da588b72..975c9db41a 100644 --- a/services/surfaceflinger/Scheduler/InjectVSyncSource.h +++ b/services/surfaceflinger/Scheduler/InjectVSyncSource.h @@ -35,10 +35,10 @@ public: mCallback = callback; } - void onInjectSyncEvent(nsecs_t when) { + void onInjectSyncEvent(nsecs_t when, nsecs_t expectedVSyncTimestamp) { std::lock_guard<std::mutex> lock(mCallbackMutex); if (mCallback) { - mCallback->onVSyncEvent(when); + mCallback->onVSyncEvent(when, expectedVSyncTimestamp); } } diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp index d8a666a6de..9d6e1d864a 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.cpp +++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp @@ -62,9 +62,9 @@ void MessageQueue::Handler::dispatchRefresh() { } } -void MessageQueue::Handler::dispatchInvalidate(nsecs_t timestamp) { +void MessageQueue::Handler::dispatchInvalidate(nsecs_t expectedVSyncTimestamp) { if ((android_atomic_or(eventMaskInvalidate, &mEventMask) & eventMaskInvalidate) == 0) { - mLastVsyncTime = timestamp; + mExpectedVSyncTime = expectedVSyncTimestamp; mQueue.mLooper->sendMessage(this, Message(MessageQueue::INVALIDATE)); } } @@ -73,11 +73,11 @@ void MessageQueue::Handler::handleMessage(const Message& message) { switch (message.what) { case INVALIDATE: android_atomic_and(~eventMaskInvalidate, &mEventMask); - mQueue.mFlinger->onMessageReceived(message.what, mLastVsyncTime); + mQueue.mFlinger->onMessageReceived(message.what, mExpectedVSyncTime); break; case REFRESH: android_atomic_and(~eventMaskRefresh, &mEventMask); - mQueue.mFlinger->onMessageReceived(message.what, mLastVsyncTime); + mQueue.mFlinger->onMessageReceived(message.what, mExpectedVSyncTime); break; } } @@ -152,7 +152,7 @@ int MessageQueue::eventReceiver(int /*fd*/, int /*events*/) { while ((n = DisplayEventReceiver::getEvents(&mEventTube, buffer, 8)) > 0) { for (int i = 0; i < n; i++) { if (buffer[i].header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) { - mHandler->dispatchInvalidate(buffer[i].header.timestamp); + mHandler->dispatchInvalidate(buffer[i].vsync.expectedVSyncTimestamp); break; } } diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h index dbd5e96f00..ebc4315ec9 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.h +++ b/services/surfaceflinger/Scheduler/MessageQueue.h @@ -101,13 +101,13 @@ class MessageQueue final : public android::MessageQueue { enum { eventMaskInvalidate = 0x1, eventMaskRefresh = 0x2, eventMaskTransaction = 0x4 }; MessageQueue& mQueue; int32_t mEventMask; - std::atomic<nsecs_t> mLastVsyncTime; + std::atomic<nsecs_t> mExpectedVSyncTime; public: explicit Handler(MessageQueue& queue) : mQueue(queue), mEventMask(0) {} virtual void handleMessage(const Message& message); void dispatchRefresh(); - void dispatchInvalidate(nsecs_t timestamp); + void dispatchInvalidate(nsecs_t expectedVSyncTimestamp); }; friend class Handler; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 3a84b67e93..86bb6eb7de 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -276,12 +276,12 @@ Scheduler::ConnectionHandle Scheduler::enableVSyncInjection(bool enable) { return mInjectorConnectionHandle; } -bool Scheduler::injectVSync(nsecs_t when) { +bool Scheduler::injectVSync(nsecs_t when, nsecs_t expectedVSyncTime) { if (!mInjectVSyncs || !mVSyncInjector) { return false; } - mVSyncInjector->onInjectSyncEvent(when); + mVSyncInjector->onInjectSyncEvent(when, expectedVSyncTime); return true; } diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 75c02f3134..4a0280fe1e 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -95,7 +95,7 @@ public: ConnectionHandle enableVSyncInjection(bool enable); // Returns false if injection is disabled. - bool injectVSync(nsecs_t when); + bool injectVSync(nsecs_t when, nsecs_t expectedVSyncTime); void enableHardwareVsync(); void disableHardwareVsync(bool makeUnavailable); diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp index 2f1faacba0..5f0c9ce40c 100644 --- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp @@ -128,7 +128,7 @@ private: mLastCallTime = vsynctime; } - mCallback->onDispSyncEvent(wakeupTime); + mCallback->onDispSyncEvent(wakeupTime, vsynctime); { std::lock_guard<std::mutex> lk(mMutex); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index d0e2668edd..c5ebc540ab 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1415,7 +1415,7 @@ status_t SurfaceFlinger::enableVSyncInjections(bool enable) { status_t SurfaceFlinger::injectVSync(nsecs_t when) { Mutex::Autolock lock(mStateLock); - return mScheduler->injectVSync(when) ? NO_ERROR : BAD_VALUE; + return mScheduler->injectVSync(when, calculateExpectedPresentTime(when)) ? NO_ERROR : BAD_VALUE; } status_t SurfaceFlinger::getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) const @@ -1454,7 +1454,9 @@ status_t SurfaceFlinger::addRegionSamplingListener(const Rect& samplingArea, if (!listener || samplingArea == Rect::INVALID_RECT) { return BAD_VALUE; } - mRegionSamplingThread->addListener(samplingArea, stopLayerHandle, listener); + + const wp<Layer> stopLayer = fromHandle(stopLayerHandle); + mRegionSamplingThread->addListener(samplingArea, stopLayer, listener); return NO_ERROR; } @@ -1824,16 +1826,16 @@ nsecs_t SurfaceFlinger::previousFramePresentTime() NO_THREAD_SAFETY_ANALYSIS { return fence->getSignalTime(); } -void SurfaceFlinger::populateExpectedPresentTime(nsecs_t wakeupTime) { +nsecs_t SurfaceFlinger::calculateExpectedPresentTime(nsecs_t now) const { DisplayStatInfo stats; mScheduler->getDisplayStatInfo(&stats); - const nsecs_t presentTime = mScheduler->getDispSyncExpectedPresentTime(wakeupTime); + const nsecs_t presentTime = mScheduler->getDispSyncExpectedPresentTime(now); // Inflate the expected present time if we're targetting the next vsync. - mExpectedPresentTime.store( - mVSyncModulator->getOffsets().sf > 0 ? presentTime : presentTime + stats.vsyncPeriod); + return mVSyncModulator->getOffsets().sf > 0 ? presentTime : presentTime + stats.vsyncPeriod; } -void SurfaceFlinger::onMessageReceived(int32_t what, nsecs_t when) NO_THREAD_SAFETY_ANALYSIS { +void SurfaceFlinger::onMessageReceived(int32_t what, + nsecs_t expectedVSyncTime) NO_THREAD_SAFETY_ANALYSIS { ATRACE_CALL(); switch (what) { case MessageQueue::INVALIDATE: { @@ -1842,7 +1844,7 @@ void SurfaceFlinger::onMessageReceived(int32_t what, nsecs_t when) NO_THREAD_SAF // value throughout this frame to make sure all layers are // seeing this same value. const nsecs_t lastExpectedPresentTime = mExpectedPresentTime.load(); - populateExpectedPresentTime(when); + mExpectedPresentTime = expectedVSyncTime; // When Backpressure propagation is enabled we want to give a small grace period // for the present fence to fire instead of just giving up on this frame to handle cases @@ -3160,7 +3162,7 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBind Mutex::Autolock _l(mStateLock); sp<Layer> parent; if (parentHandle != nullptr) { - parent = fromHandle(parentHandle); + parent = fromHandleLocked(parentHandle).promote(); if (parent == nullptr) { return NAME_NOT_FOUND; } @@ -3242,7 +3244,6 @@ bool SurfaceFlinger::flushTransactionQueues() { while (!transactionQueue.empty()) { const auto& transaction = transactionQueue.front(); if (!transactionIsReadyToBeApplied(transaction.desiredPresentTime, - true /* useCachedExpectedPresentTime */, transaction.states)) { setTransactionFlags(eTransactionFlushNeeded); break; @@ -3274,9 +3275,7 @@ bool SurfaceFlinger::transactionFlushNeeded() { bool SurfaceFlinger::transactionIsReadyToBeApplied(int64_t desiredPresentTime, - bool useCachedExpectedPresentTime, const Vector<ComposerState>& states) { - if (!useCachedExpectedPresentTime) populateExpectedPresentTime(systemTime()); const nsecs_t expectedPresentTime = mExpectedPresentTime.load(); // Do not present if the desiredPresentTime has not passed unless it is more than one second @@ -3328,9 +3327,13 @@ void SurfaceFlinger::setTransactionState( } } + const bool pendingTransactions = itr != mTransactionQueues.end(); // Expected present time is computed and cached on invalidate, so it may be stale. - if (itr != mTransactionQueues.end() || !transactionIsReadyToBeApplied( - desiredPresentTime, false /* useCachedExpectedPresentTime */, states)) { + if (!pendingTransactions) { + mExpectedPresentTime = calculateExpectedPresentTime(systemTime()); + } + + if (pendingTransactions || !transactionIsReadyToBeApplied(desiredPresentTime, states)) { mTransactionQueues[applyToken].emplace(states, displays, flags, desiredPresentTime, uncacheBuffer, postTime, privileged, hasListenerCallbacks, listenerCallbacks); @@ -3534,7 +3537,7 @@ uint32_t SurfaceFlinger::setClientStateLocked( sp<Layer> layer = nullptr; if (s.surface) { - layer = fromHandle(s.surface); + layer = fromHandleLocked(s.surface).promote(); } else { // The client may provide us a null handle. Treat it as if the layer was removed. ALOGW("Attempt to set client state with a null layer handle"); @@ -3850,7 +3853,7 @@ status_t SurfaceFlinger::mirrorLayer(const sp<Client>& client, const sp<IBinder> { Mutex::Autolock _l(mStateLock); - mirrorFrom = fromHandle(mirrorFromHandle); + mirrorFrom = fromHandleLocked(mirrorFromHandle).promote(); if (!mirrorFrom) { return NAME_NOT_FOUND; } @@ -5562,7 +5565,7 @@ status_t SurfaceFlinger::captureLayers( { Mutex::Autolock lock(mStateLock); - parent = fromHandle(layerHandleBinder); + parent = fromHandleLocked(layerHandleBinder).promote(); if (parent == nullptr || parent->isRemovedFromCurrentState()) { ALOGE("captureLayers called with an invalid or removed parent"); return NAME_NOT_FOUND; @@ -5595,7 +5598,7 @@ status_t SurfaceFlinger::captureLayers( reqHeight = crop.height() * frameScale; for (const auto& handle : excludeHandles) { - sp<Layer> excludeLayer = fromHandle(handle); + sp<Layer> excludeLayer = fromHandleLocked(handle).promote(); if (excludeLayer != nullptr) { excludeLayers.emplace(excludeLayer); } else { @@ -6066,7 +6069,12 @@ void SurfaceFlinger::SetInputWindowsListener::onSetInputWindowsFinished() { mFlinger->setInputWindowsFinished(); } -sp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) { +wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) { + Mutex::Autolock _l(mStateLock); + return fromHandleLocked(handle); +} + +wp<Layer> SurfaceFlinger::fromHandleLocked(const sp<IBinder>& handle) { BBinder* b = nullptr; if (handle) { b = handle->localBinder(); @@ -6076,7 +6084,7 @@ sp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) { } auto it = mLayersByLocalBinderToken.find(b); if (it != mLayersByLocalBinderToken.end()) { - return it->second.promote(); + return it->second; } return nullptr; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index c5e7d13008..8973bc92f5 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -315,7 +315,7 @@ public: // called on the main thread by MessageQueue when an internal message // is received // TODO: this should be made accessible only to MessageQueue - void onMessageReceived(int32_t what, nsecs_t when); + void onMessageReceived(int32_t what, nsecs_t expectedVSyncTime); renderengine::RenderEngine& getRenderEngine() const; @@ -331,7 +331,12 @@ public: return mTransactionCompletedThread; } - sp<Layer> fromHandle(const sp<IBinder>& handle) REQUIRES(mStateLock); + // Converts from a binder handle to a Layer + // Returns nullptr if the handle does not point to an existing layer. + // Otherwise, returns a weak reference so that callers off the main-thread + // won't accidentally hold onto the last strong reference. + wp<Layer> fromHandle(const sp<IBinder>& handle); + wp<Layer> fromHandleLocked(const sp<IBinder>& handle) REQUIRES(mStateLock); // Inherit from ClientCache::ErasedRecipient void bufferErased(const client_cache_t& clientCacheId) override; @@ -625,7 +630,6 @@ private: void commitTransaction() REQUIRES(mStateLock); void commitOffscreenLayers(); bool transactionIsReadyToBeApplied(int64_t desiredPresentTime, - bool useCachedExpectedPresentTime, const Vector<ComposerState>& states); uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock); uint32_t addInputWindowCommands(const InputWindowCommands& inputWindowCommands) @@ -848,9 +852,9 @@ private: // Must be called on the main thread. nsecs_t previousFramePresentTime(); - // Populates the expected present time for this frame. For negative offsets, performs a + // Calculates the expected present time for this frame. For negative offsets, performs a // correction using the predicted vsync for the next frame instead. - void populateExpectedPresentTime(nsecs_t now); + nsecs_t calculateExpectedPresentTime(nsecs_t now) const; /* * Display identification diff --git a/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp b/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp index 2e705dad6d..afebc40aa9 100644 --- a/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp +++ b/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp @@ -43,7 +43,7 @@ protected: void createDispSync(); void createDispSyncSource(); - void onVSyncEvent(nsecs_t when) override; + void onVSyncEvent(nsecs_t when, nsecs_t expectedVSyncTimestamp) override; std::unique_ptr<mock::DispSync> mDispSync; std::unique_ptr<DispSyncSource> mDispSyncSource; @@ -66,7 +66,7 @@ DispSyncSourceTest::~DispSyncSourceTest() { ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name()); } -void DispSyncSourceTest::onVSyncEvent(nsecs_t when) { +void DispSyncSourceTest::onVSyncEvent(nsecs_t when, nsecs_t /*expectedVSyncTimestamp*/) { ALOGD("onVSyncEvent: %" PRId64, when); mVSyncEventCallRecorder.recordCall(when); diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index ba5c0c2988..b90b566eee 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -258,14 +258,14 @@ TEST_F(EventThreadTest, requestNextVsyncPostsASingleVSyncEventToTheConnection) { // Use the received callback to signal a first vsync event. // The interceptor should receive the event, as well as the connection. - mCallback->onVSyncEvent(123); + mCallback->onVSyncEvent(123, 456); expectInterceptCallReceived(123); expectVsyncEventReceivedByConnection(123, 1u); // Use the received callback to signal a second vsync event. // The interceptor should receive the event, but the the connection should // not as it was only interested in the first. - mCallback->onVSyncEvent(456); + mCallback->onVSyncEvent(456, 123); expectInterceptCallReceived(456); EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); @@ -299,7 +299,7 @@ TEST_F(EventThreadTest, setVsyncRateZeroPostsNoVSyncEventsToThatConnection) { // Send a vsync event. EventThread should then make a call to the // interceptor, and the second connection. The first connection should not // get the event. - mCallback->onVSyncEvent(123); + mCallback->onVSyncEvent(123, 456); expectInterceptCallReceived(123); EXPECT_FALSE(firstConnectionEventRecorder.waitForUnexpectedCall().has_value()); expectVsyncEventReceivedByConnection("secondConnection", secondConnectionEventRecorder, 123, @@ -314,17 +314,17 @@ TEST_F(EventThreadTest, setVsyncRateOnePostsAllEventsToThatConnection) { // Send a vsync event. EventThread should then make a call to the // interceptor, and the connection. - mCallback->onVSyncEvent(123); + mCallback->onVSyncEvent(123, 456); expectInterceptCallReceived(123); expectVsyncEventReceivedByConnection(123, 1u); // A second event should go to the same places. - mCallback->onVSyncEvent(456); + mCallback->onVSyncEvent(456, 123); expectInterceptCallReceived(456); expectVsyncEventReceivedByConnection(456, 2u); // A third event should go to the same places. - mCallback->onVSyncEvent(789); + mCallback->onVSyncEvent(789, 777); expectInterceptCallReceived(789); expectVsyncEventReceivedByConnection(789, 3u); } @@ -336,22 +336,22 @@ TEST_F(EventThreadTest, setVsyncRateTwoPostsEveryOtherEventToThatConnection) { expectVSyncSetEnabledCallReceived(true); // The first event will be seen by the interceptor, and not the connection. - mCallback->onVSyncEvent(123); + mCallback->onVSyncEvent(123, 456); expectInterceptCallReceived(123); EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); // The second event will be seen by the interceptor and the connection. - mCallback->onVSyncEvent(456); + mCallback->onVSyncEvent(456, 123); expectInterceptCallReceived(456); expectVsyncEventReceivedByConnection(456, 2u); // The third event will be seen by the interceptor, and not the connection. - mCallback->onVSyncEvent(789); + mCallback->onVSyncEvent(789, 777); expectInterceptCallReceived(789); EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); // The fourth event will be seen by the interceptor and the connection. - mCallback->onVSyncEvent(101112); + mCallback->onVSyncEvent(101112, 7847); expectInterceptCallReceived(101112); expectVsyncEventReceivedByConnection(101112, 4u); } @@ -366,7 +366,7 @@ TEST_F(EventThreadTest, connectionsRemovedIfInstanceDestroyed) { mConnection = nullptr; // The first event will be seen by the interceptor, and not the connection. - mCallback->onVSyncEvent(123); + mCallback->onVSyncEvent(123, 456); expectInterceptCallReceived(123); EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); @@ -386,13 +386,13 @@ TEST_F(EventThreadTest, connectionsRemovedIfEventDeliveryError) { // The first event will be seen by the interceptor, and by the connection, // which then returns an error. - mCallback->onVSyncEvent(123); + mCallback->onVSyncEvent(123, 456); expectInterceptCallReceived(123); expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 123, 1u); // A subsequent event will be seen by the interceptor and not by the // connection. - mCallback->onVSyncEvent(456); + mCallback->onVSyncEvent(456, 123); expectInterceptCallReceived(456); EXPECT_FALSE(errorConnectionEventRecorder.waitForUnexpectedCall().has_value()); @@ -420,7 +420,7 @@ TEST_F(EventThreadTest, tracksEventConnections) { // The first event will be seen by the interceptor, and by the connection, // which then returns an error. - mCallback->onVSyncEvent(123); + mCallback->onVSyncEvent(123, 456); expectInterceptCallReceived(123); expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 123, 1u); expectVsyncEventReceivedByConnection("successConnection", secondConnectionEventRecorder, 123, @@ -440,13 +440,13 @@ TEST_F(EventThreadTest, eventsDroppedIfNonfatalEventDeliveryError) { // The first event will be seen by the interceptor, and by the connection, // which then returns an non-fatal error. - mCallback->onVSyncEvent(123); + mCallback->onVSyncEvent(123, 456); expectInterceptCallReceived(123); expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 123, 1u); // A subsequent event will be seen by the interceptor, and by the connection, // which still then returns an non-fatal error. - mCallback->onVSyncEvent(456); + mCallback->onVSyncEvent(456, 123); expectInterceptCallReceived(456); expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 456, 2u); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 389df86b4d..add33270f9 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -405,7 +405,6 @@ public: auto& mutableUseFrameRateApi() { return mFlinger->useFrameRateApi; } auto fromHandle(const sp<IBinder>& handle) { - Mutex::Autolock _l(mFlinger->mStateLock); return mFlinger->fromHandle(handle); } diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp index fbbb69ca53..2a48a22f48 100644 --- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp @@ -322,7 +322,7 @@ TEST_F(TransactionApplicationTest, BlockWithPriorTransaction_SyncInputWindows) { TEST_F(TransactionApplicationTest, FromHandle) { sp<IBinder> badHandle; auto ret = mFlinger.fromHandle(badHandle); - EXPECT_EQ(nullptr, ret.get()); + EXPECT_EQ(nullptr, ret.promote().get()); } } // namespace android diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp index 32c9045ade..3f14d652ce 100644 --- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp @@ -135,7 +135,7 @@ std::shared_ptr<FenceTime> generateSignalledFenceWithTime(nsecs_t time) { class StubCallback : public DispSync::Callback { public: - void onDispSyncEvent(nsecs_t when) final { + void onDispSyncEvent(nsecs_t when, nsecs_t /*expectedVSyncTimestamp*/) final { std::lock_guard<std::mutex> lk(mMutex); mLastCallTime = when; } @@ -544,7 +544,9 @@ TEST_F(VSyncReactorTest, selfRemovingEventListenerStopsCallbacks) { class SelfRemovingCallback : public DispSync::Callback { public: SelfRemovingCallback(VSyncReactor& vsr) : mVsr(vsr) {} - void onDispSyncEvent(nsecs_t when) final { mVsr.removeEventListener(this, &when); } + void onDispSyncEvent(nsecs_t when, nsecs_t /*expectedVSyncTimestamp*/) final { + mVsr.removeEventListener(this, &when); + } private: VSyncReactor& mVsr; diff --git a/services/surfaceflinger/tests/unittests/mock/MockDispSync.cpp b/services/surfaceflinger/tests/unittests/mock/MockDispSync.cpp index f6c4f62022..1c8c44dca0 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockDispSync.cpp +++ b/services/surfaceflinger/tests/unittests/mock/MockDispSync.cpp @@ -17,6 +17,7 @@ #include "mock/MockDispSync.h" #include <thread> +using namespace std::chrono_literals; namespace android { namespace mock { @@ -54,8 +55,9 @@ status_t DispSync::changePhaseOffset(Callback* callback, nsecs_t phase) { void DispSync::triggerCallback() { if (mCallback.callback == nullptr) return; - mCallback.callback->onDispSyncEvent( - std::chrono::steady_clock::now().time_since_epoch().count()); + const std::chrono::nanoseconds now = std::chrono::steady_clock::now().time_since_epoch(); + const auto expectedVSyncTime = now + 16ms; + mCallback.callback->onDispSyncEvent(now.count(), expectedVSyncTime.count()); } } // namespace mock |