From c5c474df8184f99dd420bd20ac375cfc29bcf819 Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Thu, 8 Aug 2019 11:57:53 -0700 Subject: [PATCH 01/43] SF: Present before signalling presentation done In doing the refactor, I had effectively swapped the calls to HWComposer::presentAndGetReleaseFences() and DisplaySurface::onFrameCommitted() so that onFrameCommitted was being called before the display was presented. If client composition was used for those frames, the result was that the GPU composed buffer on display could be written to (cleared) before it was actually replaced by a new buffer. Bug: 138914274 Test: Unable to reproduce black scanlines in Hangouts on Walleye Change-Id: I13625f124130826a5f80337b74e937f1291220e1 (cherry picked from commit 7d90ba5fdf9fe081ccb900e9032a965aaf00d978) --- services/surfaceflinger/CompositionEngine/src/Output.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index b411e0a04a..6878e99b72 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -300,10 +300,10 @@ void Output::postFramebuffer() { return; } - mRenderSurface->onPresentDisplayCompleted(); - auto frame = presentAndGetFrameFences(); + mRenderSurface->onPresentDisplayCompleted(); + for (auto& layer : getOutputLayersOrderedByZ()) { // The layer buffer from the previous frame (if any) is released // by HWC only when the release fence from this frame (if any) is From f4fab6473fc612cfcd93705942a9320501e75bed Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Wed, 16 Oct 2019 22:42:03 +0000 Subject: [PATCH 02/43] Revert "Make libbinder available to VNDK APEX" This reverts commit 03ab48771d7d385339e44301fc6c4751fe2d23a7. Reason for revert: some targets are broken Test: m Bug: 142773030 Change-Id: I2b39e399ddfbadb975ea87bb0ca74e3834742bb6 (cherry picked from commit 7148b355f3c8da8e209c4eb833a3639a27091d9c) --- libs/binder/Android.bp | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index e5646ef701..643a956b4a 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -73,7 +73,6 @@ cc_library { // or dessert updates. Instead, apex users should use libbinder_ndk. apex_available: [ "//apex_available:platform", - "com.android.vndk.current", // TODO(b/139016109) remove these three "com.android.media.swcodec", "test_com.android.media.swcodec", From be3dd44c62defb96b7464aaf07645d4fae220ffd Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Wed, 16 Oct 2019 22:40:26 +0000 Subject: [PATCH 03/43] Revert "libbinder: Consider VNDK_APEX as vendor stable" This reverts commit 6b92a6d3830a0f428adf0a2c7ecc53d52c37aa90. Reason for revert: some targets are broken Test: m Bug: 142773030 Change-Id: I6109ecf7a73ea77de1e64981d21329c8c3119bcf (cherry picked from commit e896c8e4053dbab5461b8dbd32c447572e504dd5) --- libs/binder/Parcel.cpp | 2 +- libs/binder/include/binder/Stability.h | 2 +- .../ndk/include_platform/android/binder_stability.h | 9 +++------ 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 7d9846a3cc..b70e57d49c 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -505,7 +505,7 @@ void Parcel::updateWorkSourceRequestHeaderPosition() const { } } -#if defined(__ANDROID_APEX_COM_ANDROID_VNDK_CURRENT__) || (defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__)) +#if defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__) constexpr int32_t kHeader = B_PACK_CHARS('V', 'N', 'D', 'R'); #else constexpr int32_t kHeader = B_PACK_CHARS('S', 'Y', 'S', 'T'); diff --git a/libs/binder/include/binder/Stability.h b/libs/binder/include/binder/Stability.h index b2f51d381c..2894482f55 100644 --- a/libs/binder/include/binder/Stability.h +++ b/libs/binder/include/binder/Stability.h @@ -81,7 +81,7 @@ class Stability final { VINTF = 0b111111, }; -#if defined(__ANDROID_APEX_COM_ANDROID_VNDK_CURRENT__) || (defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__)) +#if defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__) static constexpr Level kLocalStability = Level::VENDOR; #else static constexpr Level kLocalStability = Level::SYSTEM; diff --git a/libs/binder/ndk/include_platform/android/binder_stability.h b/libs/binder/ndk/include_platform/android/binder_stability.h index 2a4ded8691..e1a8cfd1f0 100644 --- a/libs/binder/ndk/include_platform/android/binder_stability.h +++ b/libs/binder/ndk/include_platform/android/binder_stability.h @@ -30,8 +30,7 @@ enum { FLAG_PRIVATE_VENDOR = 0x10000000, }; -#if defined(__ANDROID_APEX_COM_ANDROID_VNDK_CURRENT__) || \ - (defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__)) +#if (defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__)) enum { FLAG_PRIVATE_LOCAL = FLAG_PRIVATE_VENDOR, @@ -46,8 +45,7 @@ static inline void AIBinder_markCompilationUnitStability(AIBinder* binder) { AIBinder_markVendorStability(binder); } -#else // defined(__ANDROID_APEX_COM_ANDROID_VNDK_CURRENT__) || (defined(__ANDROID_VNDK__) && - // !defined(__ANDROID_APEX__)) +#else // defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__) enum { FLAG_PRIVATE_LOCAL = 0, @@ -62,8 +60,7 @@ static inline void AIBinder_markCompilationUnitStability(AIBinder* binder) { AIBinder_markSystemStability(binder); } -#endif // defined(__ANDROID_APEX_COM_ANDROID_VNDK_CURRENT__) || (defined(__ANDROID_VNDK__) && - // !defined(__ANDROID_APEX__)) +#endif // defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__) /** * This interface has system<->vendor stability From 72268f17fee0abf5369972c890902c6d64abfff8 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Tue, 12 Nov 2019 12:46:02 -0800 Subject: [PATCH 04/43] [SurfaceFlinger] Don't persist buffers to HWC when powered off. This fixes an extremely rare crash, where stale buffer handles were parceled over to HWC. The cause was that HWC's command queue is not flushed while the display is powered off, so buffers handles may become stale while they are sitting in the command queue. If a layer's buffer goes out of scope in SurfaceFlinger, e.g. an app continues renderng while the display is powered down, SurfaceFlinger latches the new buffers, and consequently releases old buffers, then those buffers will be deallocated while still sending the handles over to HWC the next time a frame needs to be presented. The fix prevents buffers from being queued while the display power mode is OFF, so that buffer handles should never become stale while in the command queue. Bug: 141290044 Test: Enabling HWSAN: covering the phone during Hangouts video calling with speaker-phone disabled to trigger display power down. Test: libcompositionengine_test Change-Id: I2592fecbbc17cf1ed70c348df8e53e9c59afb073 (cherry picked from commit 444c254ca0c920df3ad4ff19c262b2cd155caa7c) --- .../CompositionEngine/src/Output.cpp | 4 + .../CompositionEngine/tests/OutputTest.cpp | 25 ++++++ .../tests/unittests/CompositionTest.cpp | 80 +++++++++++-------- 3 files changed, 75 insertions(+), 34 deletions(-) diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index aa638b7d91..195300580e 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -551,6 +551,10 @@ void Output::updateAndWriteCompositionState( ATRACE_CALL(); ALOGV(__FUNCTION__); + if (!getState().isEnabled) { + return; + } + for (auto* layer : getOutputLayersOrderedByZ()) { layer->updateCompositionState(refreshArgs.updatingGeometryThisFrame, refreshArgs.devOptForceClientComposition); diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index 95ae888767..7fce520000 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -482,6 +482,31 @@ TEST_F(OutputTest, getOutputLayerForLayerWorks) { EXPECT_EQ(nullptr, mOutput->getOutputLayerForLayer(&layer)); } +/* + * Output::updateAndWriteCompositionState() + */ + +TEST_F(OutputTest, updateAndWriteCompositionState_takesEarlyOutIfNotEnabled) { + mOutput->editState().isEnabled = false; + + CompositionRefreshArgs args; + mOutput->updateAndWriteCompositionState(args); +} + +TEST_F(OutputTest, updateAndWriteCompositionState_updatesLayers) { + mOutput->editState().isEnabled = true; + mock::OutputLayer* outputLayer = new StrictMock(); + mOutput->injectOutputLayerForTest(std::unique_ptr(outputLayer)); + + EXPECT_CALL(*outputLayer, updateCompositionState(true, true)).Times(1); + EXPECT_CALL(*outputLayer, writeStateToHWC(true)).Times(1); + + CompositionRefreshArgs args; + args.updatingGeometryThisFrame = true; + args.devOptForceClientComposition = true; + mOutput->updateAndWriteCompositionState(args); +} + /* * Output::prepareFrame() */ diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 143a7a0fb5..8ca888bde6 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -168,6 +168,7 @@ class CompositionTest : public testing::Test { std::unordered_set mDefaultCapabilities = {HWC2::Capability::SidebandStream}; + bool mDisplayOff = false; TestableSurfaceFlinger mFlinger; sp mDisplay; sp mExternalDisplay; @@ -534,30 +535,33 @@ struct BaseLayerProperties { } static void setupHwcSetGeometryCallExpectations(CompositionTest* test) { - // TODO: Coverage of other values - EXPECT_CALL(*test->mComposer, - setLayerBlendMode(HWC_DISPLAY, HWC_LAYER, LayerProperties::BLENDMODE)) - .Times(1); - // TODO: Coverage of other values for origin - EXPECT_CALL(*test->mComposer, - setLayerDisplayFrame(HWC_DISPLAY, HWC_LAYER, - IComposerClient::Rect({0, 0, LayerProperties::WIDTH, - LayerProperties::HEIGHT}))) - .Times(1); - EXPECT_CALL(*test->mComposer, - setLayerPlaneAlpha(HWC_DISPLAY, HWC_LAYER, LayerProperties::COLOR[3])) - .Times(1); - // TODO: Coverage of other values - EXPECT_CALL(*test->mComposer, setLayerZOrder(HWC_DISPLAY, HWC_LAYER, 0u)).Times(1); - // TODO: Coverage of other values - EXPECT_CALL(*test->mComposer, setLayerInfo(HWC_DISPLAY, HWC_LAYER, 0u, 0u)).Times(1); - - // These expectations retire on saturation as the code path these - // expectations are for appears to make an extra call to them. - // TODO: Investigate this extra call - EXPECT_CALL(*test->mComposer, setLayerTransform(HWC_DISPLAY, HWC_LAYER, DEFAULT_TRANSFORM)) - .Times(AtLeast(1)) - .RetiresOnSaturation(); + if (!test->mDisplayOff) { + // TODO: Coverage of other values + EXPECT_CALL(*test->mComposer, + setLayerBlendMode(HWC_DISPLAY, HWC_LAYER, LayerProperties::BLENDMODE)) + .Times(1); + // TODO: Coverage of other values for origin + EXPECT_CALL(*test->mComposer, + setLayerDisplayFrame(HWC_DISPLAY, HWC_LAYER, + IComposerClient::Rect({0, 0, LayerProperties::WIDTH, + LayerProperties::HEIGHT}))) + .Times(1); + EXPECT_CALL(*test->mComposer, + setLayerPlaneAlpha(HWC_DISPLAY, HWC_LAYER, LayerProperties::COLOR[3])) + .Times(1); + // TODO: Coverage of other values + EXPECT_CALL(*test->mComposer, setLayerZOrder(HWC_DISPLAY, HWC_LAYER, 0u)).Times(1); + // TODO: Coverage of other values + EXPECT_CALL(*test->mComposer, setLayerInfo(HWC_DISPLAY, HWC_LAYER, 0u, 0u)).Times(1); + + // These expectations retire on saturation as the code path these + // expectations are for appears to make an extra call to them. + // TODO: Investigate this extra call + EXPECT_CALL(*test->mComposer, + setLayerTransform(HWC_DISPLAY, HWC_LAYER, DEFAULT_TRANSFORM)) + .Times(AtLeast(1)) + .RetiresOnSaturation(); + } } static void setupHwcSetSourceCropBufferCallExpectations(CompositionTest* test) { @@ -585,13 +589,16 @@ struct BaseLayerProperties { } static void setupHwcSetPerFrameColorCallExpectations(CompositionTest* test) { - EXPECT_CALL(*test->mComposer, setLayerSurfaceDamage(HWC_DISPLAY, HWC_LAYER, _)).Times(1); - - // TODO: use COLOR - EXPECT_CALL(*test->mComposer, - setLayerColor(HWC_DISPLAY, HWC_LAYER, - IComposerClient::Color({0xff, 0xff, 0xff, 0xff}))) - .Times(1); + if (!test->mDisplayOff) { + EXPECT_CALL(*test->mComposer, setLayerSurfaceDamage(HWC_DISPLAY, HWC_LAYER, _)) + .Times(1); + + // TODO: use COLOR + EXPECT_CALL(*test->mComposer, + setLayerColor(HWC_DISPLAY, HWC_LAYER, + IComposerClient::Color({0xff, 0xff, 0xff, 0xff}))) + .Times(1); + } } static void setupHwcSetPerFrameBufferCallExpectations(CompositionTest* test) { @@ -940,9 +947,11 @@ struct KeepCompositionTypeVariant { static constexpr HWC2::Composition TYPE = static_cast(CompositionType); static void setupHwcSetCallExpectations(CompositionTest* test) { - EXPECT_CALL(*test->mComposer, - setLayerCompositionType(HWC_DISPLAY, HWC_LAYER, CompositionType)) - .Times(1); + if (!test->mDisplayOff) { + EXPECT_CALL(*test->mComposer, + setLayerCompositionType(HWC_DISPLAY, HWC_LAYER, CompositionType)) + .Times(1); + } } static void setupHwcGetCallExpectations(CompositionTest* test) { @@ -1341,6 +1350,7 @@ TEST_F(CompositionTest, captureScreenCursorLayer) { */ TEST_F(CompositionTest, displayOffHWCComposedNormalBufferLayerWithDirtyGeometry) { + mDisplayOff = true; displayRefreshCompositionDirtyGeometry, KeepCompositionTypeVariant, @@ -1348,6 +1358,7 @@ TEST_F(CompositionTest, displayOffHWCComposedNormalBufferLayerWithDirtyGeometry) } TEST_F(CompositionTest, displayOffHWCComposedNormalBufferLayerWithDirtyFrame) { + mDisplayOff = true; displayRefreshCompositionDirtyFrame, KeepCompositionTypeVariant, @@ -1355,6 +1366,7 @@ TEST_F(CompositionTest, displayOffHWCComposedNormalBufferLayerWithDirtyFrame) { } TEST_F(CompositionTest, displayOffREComposedNormalBufferLayer) { + mDisplayOff = true; displayRefreshCompositionDirtyFrame, ChangeCompositionTypeVariant Date: Fri, 7 Feb 2020 19:29:00 +0000 Subject: [PATCH 05/43] Revert "SF: Remove refresh_rate_switching flag. With a fix." Revert "Removing refresh_rate_switching flag" Revert submission 10239313-b:148427603 Reason for revert: runtime errors. see b/149077559 Reverted Changes: I5bb964572:SF: Remove refresh_rate_switching flag. With a fix... I48c82de41:Removing refresh_rate_switching flag Change-Id: I0be6ed40dc2d07d3cdc1db06e77d037d3ad97b7d Bug: 149077559 (cherry picked from commit 67df70ab09465b21879ee84247df6a495a1b7a14) --- .../Scheduler/RefreshRateConfigs.cpp | 20 +++- .../Scheduler/RefreshRateConfigs.h | 10 +- .../surfaceflinger/Scheduler/Scheduler.cpp | 51 ++++------ services/surfaceflinger/SurfaceFlinger.cpp | 40 +++++--- .../SurfaceFlingerProperties.cpp | 5 - .../sysprop/SurfaceFlingerProperties.sysprop | 1 - .../api/SurfaceFlingerProperties-current.txt | 1 - .../tests/unittests/LayerHistoryTest.cpp | 3 +- .../tests/unittests/LayerHistoryTestV2.cpp | 3 +- .../unittests/RefreshRateConfigsTest.cpp | 93 +++++++++++++++---- .../tests/unittests/RefreshRateStatsTest.cpp | 4 +- .../tests/unittests/SchedulerTest.cpp | 3 +- .../tests/unittests/TestableSurfaceFlinger.h | 3 +- 13 files changed, 155 insertions(+), 82 deletions(-) diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp index c73e825d51..0f55615d50 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp +++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp @@ -216,12 +216,20 @@ const AllRefreshRatesMapType& RefreshRateConfigs::getAllRefreshRates() const { const RefreshRate& RefreshRateConfigs::getMinRefreshRateByPolicy() const { std::lock_guard lock(mLock); - return *mAvailableRefreshRates.front(); + if (!mRefreshRateSwitching) { + return *mCurrentRefreshRate; + } else { + return *mAvailableRefreshRates.front(); + } } const RefreshRate& RefreshRateConfigs::getMaxRefreshRateByPolicy() const { std::lock_guard lock(mLock); + if (!mRefreshRateSwitching) { + return *mCurrentRefreshRate; + } else { return *mAvailableRefreshRates.back(); + } } const RefreshRate& RefreshRateConfigs::getCurrentRefreshRate() const { @@ -234,14 +242,18 @@ void RefreshRateConfigs::setCurrentConfigId(HwcConfigIndexType configId) { mCurrentRefreshRate = &mRefreshRates.at(configId); } -RefreshRateConfigs::RefreshRateConfigs(const std::vector& configs, - HwcConfigIndexType currentHwcConfig) { +RefreshRateConfigs::RefreshRateConfigs(bool refreshRateSwitching, + const std::vector& configs, + HwcConfigIndexType currentHwcConfig) + : mRefreshRateSwitching(refreshRateSwitching) { init(configs, currentHwcConfig); } RefreshRateConfigs::RefreshRateConfigs( + bool refreshRateSwitching, const std::vector>& configs, - HwcConfigIndexType currentConfigId) { + HwcConfigIndexType currentConfigId) + : mRefreshRateSwitching(refreshRateSwitching) { std::vector inputConfigs; for (size_t configId = 0; configId < configs.size(); ++configId) { auto configGroup = HwcConfigGroupType(configs[configId]->getConfigGroup()); diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h index fc959597a3..c4dea0d298 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h +++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h @@ -94,6 +94,9 @@ class RefreshRateConfigs { // Returns true if config is allowed by the current policy. bool isConfigAllowed(HwcConfigIndexType config) const EXCLUDES(mLock); + // Returns true if this device is doing refresh rate switching. This won't change at runtime. + bool refreshRateSwitchingSupported() const { return mRefreshRateSwitching; } + // Describes the different options the layer voted for refresh rate enum class LayerVoteType { NoVote, // Doesn't care about the refresh rate @@ -164,9 +167,10 @@ class RefreshRateConfigs { nsecs_t vsyncPeriod = 0; }; - RefreshRateConfigs(const std::vector& configs, + RefreshRateConfigs(bool refreshRateSwitching, const std::vector& configs, HwcConfigIndexType currentHwcConfig); - RefreshRateConfigs(const std::vector>& configs, + RefreshRateConfigs(bool refreshRateSwitching, + const std::vector>& configs, HwcConfigIndexType currentConfigId); private: @@ -204,6 +208,8 @@ class RefreshRateConfigs { const RefreshRate* mMinSupportedRefreshRate; const RefreshRate* mMaxSupportedRefreshRate; + const bool mRefreshRateSwitching; + mutable std::mutex mLock; }; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 49a269fe5f..bc4f805d5e 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -530,6 +530,8 @@ void Scheduler::dump(std::string& result) const { using base::StringAppendF; const char* const states[] = {"off", "on"}; + const bool supported = mRefreshRateConfigs.refreshRateSwitchingSupported(); + StringAppendF(&result, "+ Refresh rate switching: %s\n", states[supported]); StringAppendF(&result, "+ Content detection: %s\n", states[mLayerHistory != nullptr]); StringAppendF(&result, "+ Idle timer: %s\n", @@ -573,44 +575,35 @@ bool Scheduler::layerHistoryHasClientSpecifiedFrameRate() { } HwcConfigIndexType Scheduler::calculateRefreshRateType() { - // This block of the code checks whether any layers used the SetFrameRate API. If they have, - // their request should be honored regardless of whether the device has refresh rate switching - // turned off. - if (layerHistoryHasClientSpecifiedFrameRate()) { - if (!mUseContentDetectionV2) { - return mRefreshRateConfigs.getRefreshRateForContent(mFeatures.contentRequirements) - .configId; - } else { - return mRefreshRateConfigs.getRefreshRateForContentV2(mFeatures.contentRequirements) - .configId; - } + if (!mRefreshRateConfigs.refreshRateSwitchingSupported()) { + return mRefreshRateConfigs.getCurrentRefreshRate().configId; } // If the layer history doesn't have the frame rate specified, use the old path. NOTE: // if we remove the kernel idle timer, and use our internal idle timer, this code will have to // be refactored. - // If Display Power is not in normal operation we want to be in performance mode. - // When coming back to normal mode, a grace period is given with DisplayPowerTimer - if (mDisplayPowerTimer && - (!mFeatures.isDisplayPowerStateNormal || - mFeatures.displayPowerTimer == TimerState::Reset)) { - return mRefreshRateConfigs.getMaxRefreshRateByPolicy().configId; - } + if (!layerHistoryHasClientSpecifiedFrameRate()) { + // If Display Power is not in normal operation we want to be in performance mode. + // When coming back to normal mode, a grace period is given with DisplayPowerTimer + if (!mFeatures.isDisplayPowerStateNormal || + mFeatures.displayPowerTimer == TimerState::Reset) { + return mRefreshRateConfigs.getMaxRefreshRateByPolicy().configId; + } - // As long as touch is active we want to be in performance mode - if (mTouchTimer && mFeatures.touch == TouchState::Active) { - return mRefreshRateConfigs.getMaxRefreshRateByPolicy().configId; - } + // As long as touch is active we want to be in performance mode + if (mFeatures.touch == TouchState::Active) { + return mRefreshRateConfigs.getMaxRefreshRateByPolicy().configId; + } - // If timer has expired as it means there is no new content on the screen - if (mIdleTimer && mFeatures.idleTimer == TimerState::Expired) { - return mRefreshRateConfigs.getMinRefreshRateByPolicy().configId; + // If timer has expired as it means there is no new content on the screen + if (mFeatures.idleTimer == TimerState::Expired) { + return mRefreshRateConfigs.getMinRefreshRateByPolicy().configId; + } } if (!mUseContentDetectionV2) { - // If content detection is off we choose performance as we don't know the content fps. + // If content detection is off we choose performance as we don't know the content fps if (mFeatures.contentDetection == ContentDetectionState::Off) { - // TODO(b/148428554): Be careful to not always call this. return mRefreshRateConfigs.getMaxRefreshRateByPolicy().configId; } @@ -630,10 +623,6 @@ HwcConfigIndexType Scheduler::calculateRefreshRateType() { std::optional Scheduler::getPreferredConfigId() { std::lock_guard lock(mFeatureStateLock); - // Make sure that the default config ID is first updated, before returned. - if (mFeatures.configId.has_value()) { - mFeatures.configId = calculateRefreshRateType(); - } return mFeatures.configId; } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index e8c7a55bc6..c125b2c75b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -556,6 +556,12 @@ void SurfaceFlinger::bootFinished() readPersistentProperties(); mBootStage = BootStage::FINISHED; + if (mRefreshRateConfigs->refreshRateSwitchingSupported()) { + // set the refresh rate according to the policy + const auto& performanceRefreshRate = mRefreshRateConfigs->getMaxRefreshRateByPolicy(); + changeRefreshRateLocked(performanceRefreshRate, Scheduler::ConfigEvent::None); + } + if (property_get_bool("sf.debug.show_refresh_rate_overlay", false)) { mRefreshRateOverlay = std::make_unique(*this); mRefreshRateOverlay->changeRefreshRate(mRefreshRateConfigs->getCurrentRefreshRate()); @@ -2712,7 +2718,8 @@ void SurfaceFlinger::initScheduler(DisplayId primaryDisplayId) { auto currentConfig = HwcConfigIndexType(getHwComposer().getActiveConfigIndex(primaryDisplayId)); mRefreshRateConfigs = - std::make_unique(getHwComposer().getConfigs( + std::make_unique(refresh_rate_switching(false), + getHwComposer().getConfigs( primaryDisplayId), currentConfig); mRefreshRateStats = @@ -5702,19 +5709,26 @@ status_t SurfaceFlinger::setDesiredDisplayConfigSpecsInternal(const sponConfigChanged(mAppConnectionHandle, display->getId()->value, display->getActiveConfig(), vsyncPeriod); - auto configId = mScheduler->getPreferredConfigId(); - auto preferredRefreshRate = configId - ? mRefreshRateConfigs->getRefreshRateFromConfigId(*configId) - // NOTE: Choose the default config ID, if Scheduler doesn't have one in mind. - : mRefreshRateConfigs->getRefreshRateFromConfigId(defaultConfig); - ALOGV("trying to switch to Scheduler preferred config %d (%s)", - preferredRefreshRate.configId.value(), preferredRefreshRate.name.c_str()); - - if (isDisplayConfigAllowed(preferredRefreshRate.configId)) { - ALOGV("switching to Scheduler preferred config %d", preferredRefreshRate.configId.value()); - setDesiredActiveConfig({preferredRefreshRate.configId, Scheduler::ConfigEvent::Changed}); + if (mRefreshRateConfigs->refreshRateSwitchingSupported()) { + auto configId = mScheduler->getPreferredConfigId(); + auto preferredRefreshRate = configId + ? mRefreshRateConfigs->getRefreshRateFromConfigId(*configId) + : mRefreshRateConfigs->getMinRefreshRateByPolicy(); + ALOGV("trying to switch to Scheduler preferred config %d (%s)", + preferredRefreshRate.configId.value(), preferredRefreshRate.name.c_str()); + if (isDisplayConfigAllowed(preferredRefreshRate.configId)) { + ALOGV("switching to Scheduler preferred config %d", + preferredRefreshRate.configId.value()); + setDesiredActiveConfig( + {preferredRefreshRate.configId, Scheduler::ConfigEvent::Changed}); + } else { + // Set the highest allowed config + setDesiredActiveConfig({mRefreshRateConfigs->getMaxRefreshRateByPolicy().configId, + Scheduler::ConfigEvent::Changed}); + } } else { - LOG_ALWAYS_FATAL("Desired config not allowed: %d", preferredRefreshRate.configId.value()); + ALOGV("switching to config %d", defaultConfig.value()); + setDesiredActiveConfig({defaultConfig, Scheduler::ConfigEvent::Changed}); } return NO_ERROR; diff --git a/services/surfaceflinger/SurfaceFlingerProperties.cpp b/services/surfaceflinger/SurfaceFlingerProperties.cpp index e199b53dce..b4716eb61e 100644 --- a/services/surfaceflinger/SurfaceFlingerProperties.cpp +++ b/services/surfaceflinger/SurfaceFlingerProperties.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include @@ -228,12 +227,8 @@ int64_t color_space_agnostic_dataspace(Dataspace defaultValue) { } bool refresh_rate_switching(bool defaultValue) { -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" auto temp = SurfaceFlingerProperties::refresh_rate_switching(); -#pragma clang diagnostic pop if (temp.has_value()) { - ALOGW("Using deprecated refresh_rate_switching sysprop. Value: %d", *temp); return *temp; } return defaultValue; diff --git a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop index 764181abbf..ed2b220e86 100644 --- a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop +++ b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop @@ -311,7 +311,6 @@ prop { scope: System access: Readonly prop_name: "ro.surface_flinger.refresh_rate_switching" - deprecated: true } prop { diff --git a/services/surfaceflinger/sysprop/api/SurfaceFlingerProperties-current.txt b/services/surfaceflinger/sysprop/api/SurfaceFlingerProperties-current.txt index bee99f40e3..d24ad18eaf 100644 --- a/services/surfaceflinger/sysprop/api/SurfaceFlingerProperties-current.txt +++ b/services/surfaceflinger/sysprop/api/SurfaceFlingerProperties-current.txt @@ -75,7 +75,6 @@ props { prop { api_name: "refresh_rate_switching" prop_name: "ro.surface_flinger.refresh_rate_switching" - deprecated: true } prop { api_name: "running_without_sync_framework" diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp index 18e9941385..d9481be1a1 100644 --- a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp @@ -63,7 +63,8 @@ class LayerHistoryTest : public testing::Test { auto createLayer() { return sp(new mock::MockLayer(mFlinger.flinger())); } - RefreshRateConfigs mConfigs{{ + RefreshRateConfigs mConfigs{true, + { RefreshRateConfigs::InputConfig{HwcConfigIndexType(0), HwcConfigGroupType(0), LO_FPS_PERIOD}, diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp index 959c256262..bb3bbad6c2 100644 --- a/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp +++ b/services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp @@ -71,7 +71,8 @@ class LayerHistoryTestV2 : public testing::Test { auto createLayer() { return sp(new mock::MockLayer(mFlinger.flinger())); } - RefreshRateConfigs mConfigs{{ + RefreshRateConfigs mConfigs{true, + { RefreshRateConfigs::InputConfig{HwcConfigIndexType(0), HwcConfigGroupType(0), LO_FPS_PERIOD}, diff --git a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp index 841c624a3f..7c1ecea81f 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp @@ -74,14 +74,26 @@ TEST_F(RefreshRateConfigsTest, oneDeviceConfig_SwitchingSupported) { std::vector configs{ {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); +} + +TEST_F(RefreshRateConfigsTest, oneDeviceConfig_SwitchingNotSupported) { + std::vector configs{ + {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}}}; + auto refreshRateConfigs = + std::make_unique(/*refreshRateSwitching=*/false, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + ASSERT_FALSE(refreshRateConfigs->refreshRateSwitchingSupported()); } TEST_F(RefreshRateConfigsTest, invalidPolicy) { std::vector configs{ {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); ASSERT_LT(refreshRateConfigs->setPolicy(HwcConfigIndexType(10), 60, 60, nullptr), 0); ASSERT_LT(refreshRateConfigs->setPolicy(HWC_CONFIG_ID_60, 20, 40, nullptr), 0); } @@ -91,7 +103,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_storesFullRefreshRateMap) { {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); const auto minRate = refreshRateConfigs->getMinRefreshRate(); const auto performanceRate = refreshRateConfigs->getMaxRefreshRate(); @@ -113,8 +128,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_storesFullRefreshRateMap_differe {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_1, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); const auto minRate = refreshRateConfigs->getMinRefreshRateByPolicy(); const auto performanceRate = refreshRateConfigs->getMaxRefreshRate(); const auto minRate60 = refreshRateConfigs->getMinRefreshRateByPolicy(); @@ -128,6 +145,7 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_storesFullRefreshRateMap_differe ASSERT_GE(refreshRateConfigs->setPolicy(HWC_CONFIG_ID_90, 60, 90, nullptr), 0); refreshRateConfigs->setCurrentConfigId(HWC_CONFIG_ID_90); + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); const auto minRate90 = refreshRateConfigs->getMinRefreshRateByPolicy(); const auto performanceRate90 = refreshRateConfigs->getMaxRefreshRateByPolicy(); @@ -143,8 +161,9 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_policyChange) { {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); - + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); auto minRate = refreshRateConfigs->getMinRefreshRateByPolicy(); auto performanceRate = refreshRateConfigs->getMaxRefreshRateByPolicy(); @@ -155,6 +174,7 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_policyChange) { ASSERT_EQ(expectedPerformanceConfig, performanceRate); ASSERT_GE(refreshRateConfigs->setPolicy(HWC_CONFIG_ID_60, 60, 60, nullptr), 0); + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); auto minRate60 = refreshRateConfigs->getMinRefreshRateByPolicy(); auto performanceRate60 = refreshRateConfigs->getMaxRefreshRateByPolicy(); @@ -167,7 +187,8 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getCurrentRefreshRate) { {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); { auto current = refreshRateConfigs->getCurrentRefreshRate(); EXPECT_EQ(current.configId, HWC_CONFIG_ID_60); @@ -191,7 +212,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContent) { {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; RefreshRate expected90Config = {HWC_CONFIG_ID_90, VSYNC_90, HWC_GROUP_ID_0, "90fps", 90}; @@ -252,7 +276,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_60_90 {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; RefreshRate expected90Config = {HWC_CONFIG_ID_90, VSYNC_90, HWC_GROUP_ID_0, "90fps", 90}; @@ -360,7 +387,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_60_72 {HWC_CONFIG_ID_72, HWC_GROUP_ID_0, VSYNC_72}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; RefreshRate expected72Config = {HWC_CONFIG_ID_72, VSYNC_72, HWC_GROUP_ID_0, "72fps", 70}; @@ -400,7 +430,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_30_60 {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}, {HWC_CONFIG_ID_120, HWC_GROUP_ID_0, VSYNC_120}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected30Config = {HWC_CONFIG_ID_30, VSYNC_30, HWC_GROUP_ID_0, "30fps", 30}; RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; @@ -441,7 +474,8 @@ TEST_F(RefreshRateConfigsTest, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}, {HWC_CONFIG_ID_120, HWC_GROUP_ID_0, VSYNC_120}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); RefreshRate expected30Config = {HWC_CONFIG_ID_30, VSYNC_30, HWC_GROUP_ID_0, "30fps", 30}; RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; @@ -508,7 +542,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_30_60 {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_30, HWC_GROUP_ID_0, VSYNC_30}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; RefreshRate expected30Config = {HWC_CONFIG_ID_30, VSYNC_30, HWC_GROUP_ID_0, "30fps", 30}; @@ -546,7 +583,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_30_60 {HWC_CONFIG_ID_72, HWC_GROUP_ID_0, VSYNC_72}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected30Config = {HWC_CONFIG_ID_30, VSYNC_30, HWC_GROUP_ID_0, "30fps", 30}; RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; @@ -585,7 +625,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_Prior {HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected30Config = {HWC_CONFIG_ID_30, VSYNC_30, HWC_GROUP_ID_0, "30fps", 30}; RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; @@ -638,7 +681,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_24Fps {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected30Config = {HWC_CONFIG_ID_30, VSYNC_30, HWC_GROUP_ID_0, "30fps", 30}; RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; @@ -661,7 +707,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContent_Explici {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; RefreshRate expected90Config = {HWC_CONFIG_ID_90, VSYNC_90, HWC_GROUP_ID_0, "90fps", 90}; @@ -689,7 +738,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_Expli {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; RefreshRate expected90Config = {HWC_CONFIG_ID_90, VSYNC_90, HWC_GROUP_ID_0, "90fps", 90}; @@ -727,7 +779,10 @@ TEST_F(RefreshRateConfigsTest, twoDeviceConfigs_getRefreshRateForContentV2_75HzC {{HWC_CONFIG_ID_60, HWC_GROUP_ID_0, VSYNC_60}, {HWC_CONFIG_ID_90, HWC_GROUP_ID_0, VSYNC_90}}}; auto refreshRateConfigs = - std::make_unique(configs, /*currentConfigId=*/HWC_CONFIG_ID_60); + std::make_unique(/*refreshRateSwitching=*/true, configs, + /*currentConfigId=*/HWC_CONFIG_ID_60); + + ASSERT_TRUE(refreshRateConfigs->refreshRateSwitchingSupported()); RefreshRate expected30Config = {HWC_CONFIG_ID_30, VSYNC_30, HWC_GROUP_ID_0, "30fps", 30}; RefreshRate expected60Config = {HWC_CONFIG_ID_60, VSYNC_60, HWC_GROUP_ID_0, "60fps", 60}; diff --git a/services/surfaceflinger/tests/unittests/RefreshRateStatsTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateStatsTest.cpp index 18d6bd21e6..8e07c79656 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateStatsTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateStatsTest.cpp @@ -47,8 +47,8 @@ class RefreshRateStatsTest : public testing::Test { ~RefreshRateStatsTest(); void init(const std::vector& configs) { - mRefreshRateConfigs = - std::make_unique(configs, /*currentConfig=*/CONFIG_ID_0); + mRefreshRateConfigs = std::make_unique( + /*refreshRateSwitching=*/true, configs, /*currentConfig=*/CONFIG_ID_0); mRefreshRateStats = std::make_unique(*mRefreshRateConfigs, mTimeStats, /*currentConfigId=*/CONFIG_ID_0, diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp index 89002a8f3e..82a00ee734 100644 --- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp +++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp @@ -73,7 +73,8 @@ SchedulerTest::SchedulerTest() { std::vector configs{ {{HwcConfigIndexType(0), HwcConfigGroupType(0), 16666667}}}; mRefreshRateConfigs = std::make_unique< - scheduler::RefreshRateConfigs>(configs, /*currentConfig=*/HwcConfigIndexType(0)); + scheduler::RefreshRateConfigs>(/*refreshRateSwitching=*/false, configs, + /*currentConfig=*/HwcConfigIndexType(0)); mScheduler = std::make_unique(*mRefreshRateConfigs, false); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 798ba766fc..64838cab7d 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -202,7 +202,8 @@ class TestableSurfaceFlinger { std::vector configs{ {{HwcConfigIndexType(0), HwcConfigGroupType(0), 16666667}}}; mFlinger->mRefreshRateConfigs = std::make_unique< - scheduler::RefreshRateConfigs>(configs, /*currentConfig=*/HwcConfigIndexType(0)); + scheduler::RefreshRateConfigs>(/*refreshRateSwitching=*/false, configs, + /*currentConfig=*/HwcConfigIndexType(0)); mFlinger->mRefreshRateStats = std::make_unique< scheduler::RefreshRateStats>(*mFlinger->mRefreshRateConfigs, *mFlinger->mTimeStats, /*currentConfig=*/HwcConfigIndexType(0), From 6db519dff9a4c4d8e84bcf34bdcbd41f36b111ef Mon Sep 17 00:00:00 2001 From: Daniel Chapin Date: Tue, 11 Feb 2020 23:28:24 +0000 Subject: [PATCH 06/43] Revert "SurfaceFlinger: use vsyncPeriod from HWC" This reverts commit 5dee2f130ef20dd21e413bc97556e2284610cd7a. Reason for revert: b/149273033 Change-Id: I8c8f4da27efc94d2b3b3f7f82201951612c3d525 (cherry picked from commit 7a386fcc1cbbde753bd893c9fc95045f4a3ae079) --- .../DisplayHardware/HWComposer.cpp | 1 + .../surfaceflinger/Scheduler/DispSync.cpp | 3 +- services/surfaceflinger/Scheduler/DispSync.h | 6 +- .../surfaceflinger/Scheduler/Scheduler.cpp | 6 +- services/surfaceflinger/Scheduler/Scheduler.h | 3 +- .../surfaceflinger/Scheduler/VSyncReactor.cpp | 19 ++---- .../surfaceflinger/Scheduler/VSyncReactor.h | 6 +- services/surfaceflinger/SurfaceFlinger.cpp | 6 +- .../tests/unittests/VSyncReactorTest.cpp | 66 +++++++------------ .../tests/unittests/mock/MockDispSync.h | 2 +- 10 files changed, 43 insertions(+), 75 deletions(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index f8d45c0e57..7124c6ae69 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -109,6 +109,7 @@ class ComposerCallbackBridge : public android::Hwc2::IComposerCallback { android::Hwc2::Display display, int64_t timestamp, android::Hwc2::VsyncPeriodNanos vsyncPeriodNanos) override { if (mVsyncSwitchingSupported) { + // TODO(b/140201379): use vsyncPeriodNanos in the new DispSync mCallback->onVsyncReceived(mSequenceId, display, timestamp, std::make_optional(vsyncPeriodNanos)); } else { diff --git a/services/surfaceflinger/Scheduler/DispSync.cpp b/services/surfaceflinger/Scheduler/DispSync.cpp index 809a0e52fa..ca41608c1c 100644 --- a/services/surfaceflinger/Scheduler/DispSync.cpp +++ b/services/surfaceflinger/Scheduler/DispSync.cpp @@ -542,8 +542,7 @@ void DispSync::beginResync() { resetLocked(); } -bool DispSync::addResyncSample(nsecs_t timestamp, std::optional /*hwcVsyncPeriod*/, - bool* periodFlushed) { +bool DispSync::addResyncSample(nsecs_t timestamp, bool* periodFlushed) { Mutex::Autolock lock(mMutex); ALOGV("[%s] addResyncSample(%" PRId64 ")", mName, ns2us(timestamp)); diff --git a/services/surfaceflinger/Scheduler/DispSync.h b/services/surfaceflinger/Scheduler/DispSync.h index 2d9afc9544..c6aadbb928 100644 --- a/services/surfaceflinger/Scheduler/DispSync.h +++ b/services/surfaceflinger/Scheduler/DispSync.h @@ -49,8 +49,7 @@ class DispSync { virtual void reset() = 0; virtual bool addPresentFence(const std::shared_ptr&) = 0; virtual void beginResync() = 0; - virtual bool addResyncSample(nsecs_t timestamp, std::optional hwcVsyncPeriod, - bool* periodFlushed) = 0; + virtual bool addResyncSample(nsecs_t timestamp, bool* periodFlushed) = 0; virtual void endResync() = 0; virtual void setPeriod(nsecs_t period) = 0; virtual nsecs_t getPeriod() = 0; @@ -126,8 +125,7 @@ class DispSync : public android::DispSync { // down the DispSync model, and false otherwise. // periodFlushed will be set to true if mPendingPeriod is flushed to // mIntendedPeriod, and false otherwise. - bool addResyncSample(nsecs_t timestamp, std::optional hwcVsyncPeriod, - bool* periodFlushed) override; + bool addResyncSample(nsecs_t timestamp, bool* periodFlushed) override; void endResync() override; // The setPeriod method sets the vsync event model's period to a specific diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 331c8a8cc7..2b70d65588 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -338,15 +338,13 @@ void Scheduler::setVsyncPeriod(nsecs_t period) { } } -void Scheduler::addResyncSample(nsecs_t timestamp, std::optional hwcVsyncPeriod, - bool* periodFlushed) { +void Scheduler::addResyncSample(nsecs_t timestamp, bool* periodFlushed) { bool needsHwVsync = false; *periodFlushed = false; { // Scope for the lock std::lock_guard lock(mHWVsyncLock); if (mPrimaryHWVsyncEnabled) { - needsHwVsync = - mPrimaryDispSync->addResyncSample(timestamp, hwcVsyncPeriod, periodFlushed); + needsHwVsync = mPrimaryDispSync->addResyncSample(timestamp, periodFlushed); } } diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 82b78e320a..c2345ccc14 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -109,8 +109,7 @@ class Scheduler { // Passes a vsync sample to DispSync. periodFlushed will be true if // DispSync detected that the vsync period changed, and false otherwise. - void addResyncSample(nsecs_t timestamp, std::optional hwcVsyncPeriod, - bool* periodFlushed); + void addResyncSample(nsecs_t timestamp, bool* periodFlushed); void addPresentFence(const std::shared_ptr&); void setIgnorePresentFences(bool ignore); nsecs_t getDispSyncExpectedPresentTime(); diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp index da73e4e194..70e4760676 100644 --- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp @@ -229,33 +229,24 @@ void VSyncReactor::beginResync() { void VSyncReactor::endResync() {} -bool VSyncReactor::periodConfirmed(nsecs_t vsync_timestamp, std::optional HwcVsyncPeriod) { - if (!mPeriodConfirmationInProgress) { +bool VSyncReactor::periodConfirmed(nsecs_t vsync_timestamp) { + if (!mLastHwVsync || !mPeriodConfirmationInProgress) { return false; } - - if (!mLastHwVsync && !HwcVsyncPeriod) { - return false; - } - auto const period = mPeriodTransitioningTo ? *mPeriodTransitioningTo : getPeriod(); + static constexpr int allowancePercent = 10; static constexpr std::ratio allowancePercentRatio; auto const allowance = period * allowancePercentRatio.num / allowancePercentRatio.den; - if (HwcVsyncPeriod) { - return std::abs(*HwcVsyncPeriod - period) < allowance; - } - auto const distance = vsync_timestamp - *mLastHwVsync; return std::abs(distance - period) < allowance; } -bool VSyncReactor::addResyncSample(nsecs_t timestamp, std::optional hwcVsyncPeriod, - bool* periodFlushed) { +bool VSyncReactor::addResyncSample(nsecs_t timestamp, bool* periodFlushed) { assert(periodFlushed); std::lock_guard lk(mMutex); - if (periodConfirmed(timestamp, hwcVsyncPeriod)) { + if (periodConfirmed(timestamp)) { if (mPeriodTransitioningTo) { mTracker->setPeriod(*mPeriodTransitioningTo); for (auto& entry : mCallbacks) { diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.h b/services/surfaceflinger/Scheduler/VSyncReactor.h index aa8a38d871..5b79f35c23 100644 --- a/services/surfaceflinger/Scheduler/VSyncReactor.h +++ b/services/surfaceflinger/Scheduler/VSyncReactor.h @@ -49,8 +49,7 @@ class VSyncReactor : public android::DispSync { // TODO: (b/145626181) remove begin,endResync functions from DispSync i/f when possible. void beginResync() final; - bool addResyncSample(nsecs_t timestamp, std::optional hwcVsyncPeriod, - bool* periodFlushed) final; + bool addResyncSample(nsecs_t timestamp, bool* periodFlushed) final; void endResync() final; status_t addEventListener(const char* name, nsecs_t phase, DispSync::Callback* callback, @@ -66,8 +65,7 @@ class VSyncReactor : public android::DispSync { void updateIgnorePresentFencesInternal() REQUIRES(mMutex); void startPeriodTransition(nsecs_t newPeriod) REQUIRES(mMutex); void endPeriodTransition() REQUIRES(mMutex); - bool periodConfirmed(nsecs_t vsync_timestamp, std::optional hwcVsyncPeriod) - REQUIRES(mMutex); + bool periodConfirmed(nsecs_t vsync_timestamp) REQUIRES(mMutex); std::unique_ptr const mClock; std::unique_ptr const mTracker; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f81179a3e2..1a4d316c5d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1502,9 +1502,11 @@ nsecs_t SurfaceFlinger::getVsyncPeriod() const { void SurfaceFlinger::onVsyncReceived(int32_t sequenceId, hwc2_display_t hwcDisplayId, int64_t timestamp, - std::optional vsyncPeriod) { + std::optional /*vsyncPeriod*/) { ATRACE_NAME("SF onVsync"); + // TODO(b/140201379): use vsyncPeriod in the new DispSync + Mutex::Autolock lock(mStateLock); // Ignore any vsyncs from a previous hardware composer. if (sequenceId != getBE().mComposerSequenceId) { @@ -1521,7 +1523,7 @@ void SurfaceFlinger::onVsyncReceived(int32_t sequenceId, hwc2_display_t hwcDispl } bool periodFlushed = false; - mScheduler->addResyncSample(timestamp, vsyncPeriod, &periodFlushed); + mScheduler->addResyncSample(timestamp, &periodFlushed); if (periodFlushed) { mVSyncModulator->onRefreshRateChangeCompleted(); } diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp index 2f36bb2941..1de72b9599 100644 --- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp @@ -250,9 +250,9 @@ TEST_F(VSyncReactorTest, ignoresProperlyAfterAPeriodConfirmation) { nsecs_t const newPeriod = 5000; mReactor.setPeriod(newPeriod); - EXPECT_TRUE(mReactor.addResyncSample(0, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(0, &periodFlushed)); EXPECT_FALSE(periodFlushed); - EXPECT_FALSE(mReactor.addResyncSample(newPeriod, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(newPeriod, &periodFlushed)); EXPECT_TRUE(periodFlushed); EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0))); @@ -302,16 +302,16 @@ TEST_F(VSyncReactorTest, setPeriodCalledOnceConfirmedChange) { mReactor.setPeriod(newPeriod); bool periodFlushed = true; - EXPECT_TRUE(mReactor.addResyncSample(10000, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(10000, &periodFlushed)); EXPECT_FALSE(periodFlushed); - EXPECT_TRUE(mReactor.addResyncSample(20000, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(20000, &periodFlushed)); EXPECT_FALSE(periodFlushed); Mock::VerifyAndClearExpectations(mMockTracker.get()); EXPECT_CALL(*mMockTracker, setPeriod(newPeriod)).Times(1); - EXPECT_FALSE(mReactor.addResyncSample(25000, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(25000, &periodFlushed)); EXPECT_TRUE(periodFlushed); } @@ -320,14 +320,14 @@ TEST_F(VSyncReactorTest, changingPeriodBackAbortsConfirmationProcess) { nsecs_t const newPeriod = 5000; mReactor.setPeriod(newPeriod); bool periodFlushed = true; - EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, &periodFlushed)); EXPECT_FALSE(periodFlushed); - EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, &periodFlushed)); EXPECT_FALSE(periodFlushed); mReactor.setPeriod(period); - EXPECT_FALSE(mReactor.addResyncSample(sampleTime += period, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(sampleTime += period, &periodFlushed)); EXPECT_FALSE(periodFlushed); } @@ -338,14 +338,14 @@ TEST_F(VSyncReactorTest, changingToAThirdPeriodWillWaitForLastPeriod) { mReactor.setPeriod(secondPeriod); bool periodFlushed = true; - EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, &periodFlushed)); EXPECT_FALSE(periodFlushed); - EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, &periodFlushed)); EXPECT_FALSE(periodFlushed); mReactor.setPeriod(thirdPeriod); - EXPECT_TRUE(mReactor.addResyncSample(sampleTime += secondPeriod, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(sampleTime += secondPeriod, &periodFlushed)); EXPECT_FALSE(periodFlushed); - EXPECT_FALSE(mReactor.addResyncSample(sampleTime += thirdPeriod, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(sampleTime += thirdPeriod, &periodFlushed)); EXPECT_TRUE(periodFlushed); } @@ -360,9 +360,9 @@ TEST_F(VSyncReactorTest, reportedBadTimestampFromPredictorWillReactivateHwVSync) nsecs_t skewyPeriod = period >> 1; bool periodFlushed = false; nsecs_t sampleTime = 0; - EXPECT_TRUE(mReactor.addResyncSample(sampleTime += skewyPeriod, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(sampleTime += skewyPeriod, &periodFlushed)); EXPECT_FALSE(periodFlushed); - EXPECT_FALSE(mReactor.addResyncSample(sampleTime += period, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(sampleTime += period, &periodFlushed)); EXPECT_FALSE(periodFlushed); } @@ -390,12 +390,12 @@ TEST_F(VSyncReactorTest, setPeriodCalledFirstTwoEventsNewPeriod) { mReactor.setPeriod(newPeriod); bool periodFlushed = true; - EXPECT_TRUE(mReactor.addResyncSample(5000, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(5000, &periodFlushed)); EXPECT_FALSE(periodFlushed); Mock::VerifyAndClearExpectations(mMockTracker.get()); EXPECT_CALL(*mMockTracker, setPeriod(newPeriod)).Times(1); - EXPECT_FALSE(mReactor.addResyncSample(10000, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(10000, &periodFlushed)); EXPECT_TRUE(periodFlushed); } @@ -404,7 +404,7 @@ TEST_F(VSyncReactorTest, addResyncSampleTypical) { bool periodFlushed = false; EXPECT_CALL(*mMockTracker, addVsyncTimestamp(fakeTimestamp)); - EXPECT_FALSE(mReactor.addResyncSample(fakeTimestamp, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(fakeTimestamp, &periodFlushed)); EXPECT_FALSE(periodFlushed); } @@ -418,17 +418,17 @@ TEST_F(VSyncReactorTest, addResyncSamplePeriodChanges) { auto constexpr numTimestampSubmissions = 10; for (auto i = 0; i < numTimestampSubmissions; i++) { time += period; - EXPECT_TRUE(mReactor.addResyncSample(time, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(time, &periodFlushed)); EXPECT_FALSE(periodFlushed); } time += newPeriod; - EXPECT_FALSE(mReactor.addResyncSample(time, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(time, &periodFlushed)); EXPECT_TRUE(periodFlushed); for (auto i = 0; i < numTimestampSubmissions; i++) { time += newPeriod; - EXPECT_FALSE(mReactor.addResyncSample(time, std::nullopt, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(time, &periodFlushed)); EXPECT_FALSE(periodFlushed); } } @@ -440,11 +440,11 @@ TEST_F(VSyncReactorTest, addPresentFenceWhileAwaitingPeriodConfirmationRequestsH mReactor.setPeriod(newPeriod); time += period; - mReactor.addResyncSample(time, std::nullopt, &periodFlushed); + mReactor.addResyncSample(time, &periodFlushed); EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0))); time += newPeriod; - mReactor.addResyncSample(time, std::nullopt, &periodFlushed); + mReactor.addResyncSample(time, &periodFlushed); EXPECT_FALSE(mReactor.addPresentFence(generateSignalledFenceWithTime(0))); } @@ -568,8 +568,8 @@ TEST_F(VSyncReactorTest, changingPeriodChangesOffsetsOnNextCb) { bool periodFlushed = false; mReactor.setPeriod(anotherPeriod); - EXPECT_TRUE(mReactor.addResyncSample(anotherPeriod, std::nullopt, &periodFlushed)); - EXPECT_FALSE(mReactor.addResyncSample(anotherPeriod * 2, std::nullopt, &periodFlushed)); + EXPECT_TRUE(mReactor.addResyncSample(anotherPeriod, &periodFlushed)); + EXPECT_FALSE(mReactor.addResyncSample(anotherPeriod * 2, &periodFlushed)); mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime); } @@ -614,24 +614,6 @@ TEST_F(VSyncReactorTest, beginResyncResetsModel) { mReactor.beginResync(); } -TEST_F(VSyncReactorTest, periodChangeWithGivenVsyncPeriod) { - bool periodFlushed = true; - EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(3); - mReactor.setIgnorePresentFences(true); - - nsecs_t const newPeriod = 5000; - mReactor.setPeriod(newPeriod); - - EXPECT_TRUE(mReactor.addResyncSample(0, 0, &periodFlushed)); - EXPECT_FALSE(periodFlushed); - EXPECT_TRUE(mReactor.addResyncSample(newPeriod, 0, &periodFlushed)); - EXPECT_FALSE(periodFlushed); - EXPECT_FALSE(mReactor.addResyncSample(newPeriod, newPeriod, &periodFlushed)); - EXPECT_TRUE(periodFlushed); - - EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0))); -} - using VSyncReactorDeathTest = VSyncReactorTest; TEST_F(VSyncReactorDeathTest, invalidRemoval) { mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime); diff --git a/services/surfaceflinger/tests/unittests/mock/MockDispSync.h b/services/surfaceflinger/tests/unittests/mock/MockDispSync.h index a2ae6c9b8d..9ca116d735 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockDispSync.h +++ b/services/surfaceflinger/tests/unittests/mock/MockDispSync.h @@ -31,7 +31,7 @@ class DispSync : public android::DispSync { MOCK_METHOD0(reset, void()); MOCK_METHOD1(addPresentFence, bool(const std::shared_ptr&)); MOCK_METHOD0(beginResync, void()); - MOCK_METHOD3(addResyncSample, bool(nsecs_t, std::optional, bool*)); + MOCK_METHOD2(addResyncSample, bool(nsecs_t, bool*)); MOCK_METHOD0(endResync, void()); MOCK_METHOD1(setPeriod, void(nsecs_t)); MOCK_METHOD0(getPeriod, nsecs_t()); From b5183a10419ad948a80ffcea772ab67edc56339c Mon Sep 17 00:00:00 2001 From: Anthony Stange Date: Mon, 20 Apr 2020 22:37:08 +0000 Subject: [PATCH 07/43] Revert "Use resolution to round sensor event values" This reverts commit 0ff158f8f2e5749737c51444b9aeb153bd0b325d. Reason for revert: Causing b/154494468 Change-Id: Ib119719a35c6487ceb086978f97b17857aad12ea (cherry picked from commit 7ff9e0f6025eeb5dbe08e9d3d584e6b7652bae1a) --- services/sensorservice/SensorDevice.cpp | 31 +------ services/sensorservice/SensorDevice.h | 4 +- services/sensorservice/SensorDeviceUtils.cpp | 96 -------------------- services/sensorservice/SensorDeviceUtils.h | 7 -- 4 files changed, 3 insertions(+), 135 deletions(-) diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp index aa6f1b8f5a..3b68e0e097 100644 --- a/services/sensorservice/SensorDevice.cpp +++ b/services/sensorservice/SensorDevice.cpp @@ -143,14 +143,6 @@ void SensorDevice::initializeSensorList() { for (size_t i=0 ; i < count; i++) { sensor_t sensor; convertToSensor(convertToOldSensorInfo(list[i]), &sensor); - - if (sensor.resolution == 0) { - // Don't crash here or the device will go into a crashloop. - ALOGE("%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); - } - // 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", @@ -516,7 +508,7 @@ ssize_t SensorDevice::pollHal(sensors_event_t* buffer, size_t count) { const auto &events, const auto &dynamicSensorsAdded) { if (result == Result::OK) { - convertToSensorEventsAndQuantize(convertToNewEvents(events), + convertToSensorEvents(convertToNewEvents(events), convertToNewSensorInfos(dynamicSensorsAdded), buffer); err = (ssize_t)events.size(); } else { @@ -579,8 +571,6 @@ 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 { @@ -1087,7 +1077,7 @@ void SensorDevice::convertToSensorEvent( } } -void SensorDevice::convertToSensorEventsAndQuantize( +void SensorDevice::convertToSensorEvents( const hidl_vec &src, const hidl_vec &dynamicSensorsAdded, sensors_event_t *dst) { @@ -1098,24 +1088,7 @@ void SensorDevice::convertToSensorEventsAndQuantize( 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 04e60315b4..24d03c63c2 100644 --- a/services/sensorservice/SensorDevice.h +++ b/services/sensorservice/SensorDevice.h @@ -233,13 +233,11 @@ class SensorDevice : public Singleton, void convertToSensorEvent(const Event &src, sensors_event_t *dst); - void convertToSensorEventsAndQuantize( + void convertToSensorEvents( const hardware::hidl_vec &src, const hardware::hidl_vec &dynamicSensorsAdded, sensors_event_t *dst); - float getResolutionForSensor(int sensorHandle); - bool mIsDirectReportSupported; typedef hardware::MessageQueue WakeLockQueue; diff --git a/services/sensorservice/SensorDeviceUtils.cpp b/services/sensorservice/SensorDeviceUtils.cpp index 6bf62e4f82..dbafffe303 100644 --- a/services/sensorservice/SensorDeviceUtils.cpp +++ b/services/sensorservice/SensorDeviceUtils.cpp @@ -17,112 +17,16 @@ #include "SensorDeviceUtils.h" #include -#include #include #include -#include #include using ::android::hardware::Void; -using SensorTypeV2_1 = android::hardware::sensors::V2_1::SensorType; using namespace android::hardware::sensors::V1_0; namespace android { namespace SensorDeviceUtils { -namespace { - -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(*value) / incRes) * incRes; -} - -} // namespace - -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 b66542cd09..e2eb606b9a 100644 --- a/services/sensorservice/SensorDeviceUtils.h +++ b/services/sensorservice/SensorDeviceUtils.h @@ -18,7 +18,6 @@ #define ANDROID_SENSOR_DEVICE_UTIL #include -#include #include #include @@ -30,12 +29,6 @@ using ::android::hidl::manager::V1_0::IServiceNotification; namespace android { namespace SensorDeviceUtils { -// 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: From 4368557f2ee1bdd6c09b5ee48bc647d4a1ac5fac Mon Sep 17 00:00:00 2001 From: Anthony Stange Date: Mon, 20 Apr 2020 22:37:08 +0000 Subject: [PATCH 08/43] Revert "Use resolution to round sensor event values" This reverts commit 0ff158f8f2e5749737c51444b9aeb153bd0b325d. Reason for revert: Causing b/154494468 Bug: 154494468 Change-Id: Ib119719a35c6487ceb086978f97b17857aad12ea (cherry picked from commit 9b466b78fac9c607ab524515c451a26080ff1496) --- services/sensorservice/SensorDevice.cpp | 31 +------ services/sensorservice/SensorDevice.h | 4 +- services/sensorservice/SensorDeviceUtils.cpp | 96 -------------------- services/sensorservice/SensorDeviceUtils.h | 7 -- 4 files changed, 3 insertions(+), 135 deletions(-) diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp index aa6f1b8f5a..3b68e0e097 100644 --- a/services/sensorservice/SensorDevice.cpp +++ b/services/sensorservice/SensorDevice.cpp @@ -143,14 +143,6 @@ void SensorDevice::initializeSensorList() { for (size_t i=0 ; i < count; i++) { sensor_t sensor; convertToSensor(convertToOldSensorInfo(list[i]), &sensor); - - if (sensor.resolution == 0) { - // Don't crash here or the device will go into a crashloop. - ALOGE("%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); - } - // 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", @@ -516,7 +508,7 @@ ssize_t SensorDevice::pollHal(sensors_event_t* buffer, size_t count) { const auto &events, const auto &dynamicSensorsAdded) { if (result == Result::OK) { - convertToSensorEventsAndQuantize(convertToNewEvents(events), + convertToSensorEvents(convertToNewEvents(events), convertToNewSensorInfos(dynamicSensorsAdded), buffer); err = (ssize_t)events.size(); } else { @@ -579,8 +571,6 @@ 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 { @@ -1087,7 +1077,7 @@ void SensorDevice::convertToSensorEvent( } } -void SensorDevice::convertToSensorEventsAndQuantize( +void SensorDevice::convertToSensorEvents( const hidl_vec &src, const hidl_vec &dynamicSensorsAdded, sensors_event_t *dst) { @@ -1098,24 +1088,7 @@ void SensorDevice::convertToSensorEventsAndQuantize( 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 04e60315b4..24d03c63c2 100644 --- a/services/sensorservice/SensorDevice.h +++ b/services/sensorservice/SensorDevice.h @@ -233,13 +233,11 @@ class SensorDevice : public Singleton, void convertToSensorEvent(const Event &src, sensors_event_t *dst); - void convertToSensorEventsAndQuantize( + void convertToSensorEvents( const hardware::hidl_vec &src, const hardware::hidl_vec &dynamicSensorsAdded, sensors_event_t *dst); - float getResolutionForSensor(int sensorHandle); - bool mIsDirectReportSupported; typedef hardware::MessageQueue WakeLockQueue; diff --git a/services/sensorservice/SensorDeviceUtils.cpp b/services/sensorservice/SensorDeviceUtils.cpp index 6bf62e4f82..dbafffe303 100644 --- a/services/sensorservice/SensorDeviceUtils.cpp +++ b/services/sensorservice/SensorDeviceUtils.cpp @@ -17,112 +17,16 @@ #include "SensorDeviceUtils.h" #include -#include #include #include -#include #include using ::android::hardware::Void; -using SensorTypeV2_1 = android::hardware::sensors::V2_1::SensorType; using namespace android::hardware::sensors::V1_0; namespace android { namespace SensorDeviceUtils { -namespace { - -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(*value) / incRes) * incRes; -} - -} // namespace - -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 b66542cd09..e2eb606b9a 100644 --- a/services/sensorservice/SensorDeviceUtils.h +++ b/services/sensorservice/SensorDeviceUtils.h @@ -18,7 +18,6 @@ #define ANDROID_SENSOR_DEVICE_UTIL #include -#include #include #include @@ -30,12 +29,6 @@ using ::android::hidl::manager::V1_0::IServiceNotification; namespace android { namespace SensorDeviceUtils { -// 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: From b437df06d063cf24b82340b4573b151c5b60cf5c Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Tue, 28 Jul 2020 23:11:21 -0700 Subject: [PATCH 09/43] GpuService: secure setUpdatableDriverPath setUpdatableDriverPath should only be called by system_server and developer driver path needs to be protected by a lock. Bug: 162383705 Bug: 159240322 Test: ./gapit validate_gpu_profiling --os android Change-Id: I48896325598acab89079dbc658ddf9b92d303244 Merged-In: I48896325598acab89079dbc658ddf9b92d303244 (cherry picked from commit 2b65d6ca48773901c396344c5fdc851ec14a4bdf) --- services/gpuservice/GpuService.cpp | 16 ++++++++++++++-- services/gpuservice/GpuService.h | 3 ++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/services/gpuservice/GpuService.cpp b/services/gpuservice/GpuService.cpp index 304f1d059e..81b0a46e0c 100644 --- a/services/gpuservice/GpuService.cpp +++ b/services/gpuservice/GpuService.cpp @@ -63,11 +63,23 @@ void GpuService::setTargetStats(const std::string& appPackageName, const uint64_ } void GpuService::setUpdatableDriverPath(const std::string& driverPath) { - developerDriverPath = driverPath; + IPCThreadState* ipc = IPCThreadState::self(); + const int pid = ipc->getCallingPid(); + const int uid = ipc->getCallingUid(); + + // only system_server is allowed to set updatable driver path + if (uid != AID_SYSTEM) { + ALOGE("Permission Denial: can't set updatable driver path from pid=%d, uid=%d\n", pid, uid); + return; + } + + std::lock_guard lock(mLock); + mDeveloperDriverPath = driverPath; } std::string GpuService::getUpdatableDriverPath() { - return developerDriverPath; + std::lock_guard lock(mLock); + return mDeveloperDriverPath; } status_t GpuService::shellCommand(int /*in*/, int out, int err, std::vector& args) { diff --git a/services/gpuservice/GpuService.h b/services/gpuservice/GpuService.h index ba44fe04d4..d1c3aabcce 100644 --- a/services/gpuservice/GpuService.h +++ b/services/gpuservice/GpuService.h @@ -75,7 +75,8 @@ class GpuService : public BnGpuService, public PriorityDumper { * Attributes */ std::unique_ptr mGpuStats; - std::string developerDriverPath; + std::mutex mLock; + std::string mDeveloperDriverPath; }; } // namespace android From 1f8eaf998cc3f48ff2c15f18c6979bac62bbbe52 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 6 Aug 2020 19:32:45 +0000 Subject: [PATCH 10/43] libbinder_ndk: fix failure when dump/shell are unset People directly using libbinder_ndk functions who didn't create a debug dump function function would fail to initialize that pointer, and potentially crash. Those who didn't create a shell function were guaranteed to crash. This wasn't noticed because the C++ wrappers which are the recommended way to use libbinder_ndk always set these functions. Bug: 161812320 Test: unit tests Merged-In: I1f6909531bc640097f3f48c4a558fd03f2fa62cb Change-Id: I1f6909531bc640097f3f48c4a558fd03f2fa62cb (cherry picked from commit deb5346761308d9cda3a249283a482a1ce08549e) --- libs/binder/ndk/ibinder.cpp | 2 +- libs/binder/ndk/ibinder_internal.h | 10 +++++----- libs/binder/ndk/test/iface.cpp | 12 ++++++++++- libs/binder/ndk/test/include/iface/iface.h | 3 +++ .../ndk/test/libbinder_ndk_unit_test.cpp | 20 +++++++++++++++++++ 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp index 649faa1c76..919150d740 100644 --- a/libs/binder/ndk/ibinder.cpp +++ b/libs/binder/ndk/ibinder.cpp @@ -161,7 +161,7 @@ status_t ABBinder::onTransact(transaction_code_t code, const Parcel& data, Parce binder_status_t status = getClass()->onTransact(this, code, &in, &out); return PruneStatusT(status); - } else if (code == SHELL_COMMAND_TRANSACTION) { + } else if (code == SHELL_COMMAND_TRANSACTION && getClass()->handleShellCommand != nullptr) { int in = data.readFileDescriptor(); int out = data.readFileDescriptor(); int err = data.readFileDescriptor(); diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h index 57794279f2..902fe7934d 100644 --- a/libs/binder/ndk/ibinder_internal.h +++ b/libs/binder/ndk/ibinder_internal.h @@ -110,13 +110,13 @@ struct AIBinder_Class { const ::android::String16& getInterfaceDescriptor() const { return mInterfaceDescriptor; } // required to be non-null, implemented for every class - const AIBinder_Class_onCreate onCreate; - const AIBinder_Class_onDestroy onDestroy; - const AIBinder_Class_onTransact onTransact; + const AIBinder_Class_onCreate onCreate = nullptr; + const AIBinder_Class_onDestroy onDestroy = nullptr; + const AIBinder_Class_onTransact onTransact = nullptr; // optional methods for a class - AIBinder_onDump onDump; - AIBinder_handleShellCommand handleShellCommand; + AIBinder_onDump onDump = nullptr; + AIBinder_handleShellCommand handleShellCommand = nullptr; private: // This must be a String16 since BBinder virtual getInterfaceDescriptor returns a reference to diff --git a/libs/binder/ndk/test/iface.cpp b/libs/binder/ndk/test/iface.cpp index 64832f3081..a5889856fc 100644 --- a/libs/binder/ndk/test/iface.cpp +++ b/libs/binder/ndk/test/iface.cpp @@ -118,7 +118,7 @@ IFoo::~IFoo() { AIBinder_Weak_delete(mWeakBinder); } -binder_status_t IFoo::addService(const char* instance) { +AIBinder* IFoo::getBinder() { AIBinder* binder = nullptr; if (mWeakBinder != nullptr) { @@ -132,8 +132,18 @@ binder_status_t IFoo::addService(const char* instance) { AIBinder_Weak_delete(mWeakBinder); } mWeakBinder = AIBinder_Weak_new(binder); + + // WARNING: it is important that this class does not implement debug or + // shell functions because it does not use special C++ wrapper + // functions, and so this is how we test those functions. } + return binder; +} + +binder_status_t IFoo::addService(const char* instance) { + AIBinder* binder = getBinder(); + binder_status_t status = AServiceManager_addService(binder, instance); // Strong references we care about kept by remote process AIBinder_decStrong(binder); diff --git a/libs/binder/ndk/test/include/iface/iface.h b/libs/binder/ndk/test/include/iface/iface.h index cdf5493216..d9dd64b8a6 100644 --- a/libs/binder/ndk/test/include/iface/iface.h +++ b/libs/binder/ndk/test/include/iface/iface.h @@ -30,6 +30,9 @@ class IFoo : public virtual ::android::RefBase { static AIBinder_Class* kClass; + // binder representing this interface with one reference count + AIBinder* getBinder(); + // Takes ownership of IFoo binder_status_t addService(const char* instance); static ::android::sp getService(const char* instance, AIBinder** outBinder = nullptr); diff --git a/libs/binder/ndk/test/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/test/libbinder_ndk_unit_test.cpp index fd30d87c76..aaf36b97a4 100644 --- a/libs/binder/ndk/test/libbinder_ndk_unit_test.cpp +++ b/libs/binder/ndk/test/libbinder_ndk_unit_test.cpp @@ -126,6 +126,26 @@ TEST(NdkBinder, CheckServiceThatDoesExist) { AIBinder_decStrong(binder); } +TEST(NdkBinder, UnimplementedDump) { + sp foo = IFoo::getService(IFoo::kSomeInstanceName); + ASSERT_NE(foo, nullptr); + AIBinder* binder = foo->getBinder(); + EXPECT_EQ(OK, AIBinder_dump(binder, STDOUT_FILENO, nullptr, 0)); + AIBinder_decStrong(binder); +} + +TEST(NdkBinder, UnimplementedShell) { + // libbinder_ndk doesn't support calling shell, so we are calling from the + // libbinder across processes to the NDK service which doesn't implement + // shell + static const sp sm(android::defaultServiceManager()); + sp testService = sm->getService(String16(IFoo::kSomeInstanceName)); + + Vector argsVec; + EXPECT_EQ(OK, IBinder::shellCommand(testService, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO, + argsVec, nullptr, nullptr)); +} + TEST(NdkBinder, DoubleNumber) { sp foo = IFoo::getService(IFoo::kSomeInstanceName); ASSERT_NE(foo, nullptr); From 26824396562b080adee861beb8332a2c1e3f7a08 Mon Sep 17 00:00:00 2001 From: Arthur Ishiguro Date: Tue, 22 Sep 2020 13:05:15 -0700 Subject: [PATCH 11/43] Prevent mEventCache UAF in SensorEventConnection Since there is no check to see if SensorEventConnection has been destroyed, the mEventCache pointer can still be used even after it was freed. Bug: 168211968 Test: Run test code that attempts to enable a sensor after destroying the SensorEventConnection, and verify no system_server crash occurs. Change-Id: Ia9275b7cc574df371cdb2e1b80c6699df193b580 Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580 (cherry picked from commit 3e9afc163256db661b9039120d07501b3a8a7d99) (cherry picked from commit f1bf7dd095ac2f632442663cb16aeef4691b93e7) --- .../sensorservice/SensorEventConnection.cpp | 28 ++++++++++++------- .../sensorservice/SensorEventConnection.h | 5 ++-- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index b4b5f98609..6c8671289d 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -47,20 +48,13 @@ SensorService::SensorEventConnection::SensorEventConnection( SensorService::SensorEventConnection::~SensorEventConnection() { ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this); destroy(); -} - -void SensorService::SensorEventConnection::destroy() { - Mutex::Autolock _l(mDestroyLock); - - // destroy once only - if (mDestroyed) { - return; - } - mService->cleanupConnection(this); if (mEventCache != nullptr) { delete[] mEventCache; } +} + +void SensorService::SensorEventConnection::destroy() { mDestroyed = true; } @@ -665,6 +659,11 @@ status_t SensorService::SensorEventConnection::enableDisable( int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + status_t err; if (enabled) { err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs, @@ -679,10 +678,19 @@ status_t SensorService::SensorEventConnection::enableDisable( status_t SensorService::SensorEventConnection::setEventRate( int handle, nsecs_t samplingPeriodNs) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName); } status_t SensorService::SensorEventConnection::flush() { + if (mDestroyed) { + return DEAD_OBJECT; + } + return mService->flushSensor(this, mOpPackageName); } diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index 8f2d5db28f..9487a39a92 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -17,6 +17,7 @@ #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H #define ANDROID_SENSOR_EVENT_CONNECTION_H +#include #include #include #include @@ -182,8 +183,8 @@ class SensorService::SensorEventConnection: int mTotalAcksNeeded, mTotalAcksReceived; #endif - mutable Mutex mDestroyLock; - bool mDestroyed; + // Used to track if this object was inappropriately used after destroy(). + std::atomic_bool mDestroyed; // 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. From e0ef447437455c5de1e04b71018c4d146ca46944 Mon Sep 17 00:00:00 2001 From: Jon Spivack Date: Tue, 27 Oct 2020 19:29:14 -0700 Subject: [PATCH 12/43] libbinder: Add ClientCounterCallbackImpl to LazyServiceRegistrar This extra layer of indirection below ClientCounterCallback fixes a shared pointer ownership issue between LazyServiceRegistrar and ServiceManager. It also allows for implementation changes (like this one) without changing headers and breaking VNDK. Bug: 170212632 Test: Manual (Went through reproduction steps in bug on cf_x86_phone-userdebug) Test: atest aidl_lazy_test Change-Id: I4164a6d44e567c752726953e85aee0e91c6b525e Merged-In: I4164a6d44e567c752726953e85aee0e91c6b525e (cherry picked from commit 7c227cc333b85938a1ad0f860655bb83567ca755) --- libs/binder/LazyServiceRegistrar.cpp | 46 +++++++++++++++++++++------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp index 6f49aa1607..f2c5139b56 100644 --- a/libs/binder/LazyServiceRegistrar.cpp +++ b/libs/binder/LazyServiceRegistrar.cpp @@ -29,16 +29,12 @@ namespace internal { using AidlServiceManager = android::os::IServiceManager; -class ClientCounterCallback : public ::android::os::BnClientCallback { +class ClientCounterCallbackImpl : public ::android::os::BnClientCallback { public: - ClientCounterCallback() : mNumConnectedServices(0), mForcePersist(false) {} + ClientCounterCallbackImpl() : mNumConnectedServices(0), mForcePersist(false) {} bool registerService(const sp& service, const std::string& name, bool allowIsolated, int dumpFlags); - - /** - * Set a flag to prevent services from automatically shutting down - */ void forcePersist(bool persist); protected: @@ -69,7 +65,23 @@ class ClientCounterCallback : public ::android::os::BnClientCallback { bool mForcePersist; }; -bool ClientCounterCallback::registerService(const sp& service, const std::string& name, +class ClientCounterCallback { +public: + ClientCounterCallback(); + + bool registerService(const sp& service, const std::string& name, + bool allowIsolated, int dumpFlags); + + /** + * Set a flag to prevent services from automatically shutting down + */ + void forcePersist(bool persist); + +private: + sp mImpl; +}; + +bool ClientCounterCallbackImpl::registerService(const sp& service, const std::string& name, bool allowIsolated, int dumpFlags) { auto manager = interface_cast(asBinder(defaultServiceManager())); @@ -95,7 +107,7 @@ bool ClientCounterCallback::registerService(const sp& service, const st return true; } -void ClientCounterCallback::forcePersist(bool persist) { +void ClientCounterCallbackImpl::forcePersist(bool persist) { mForcePersist = persist; if(!mForcePersist) { // Attempt a shutdown in case the number of clients hit 0 while the flag was on @@ -107,7 +119,7 @@ void ClientCounterCallback::forcePersist(bool persist) { * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple * invocations could occur on different threads however. */ -Status ClientCounterCallback::onClients(const sp& service, bool clients) { +Status ClientCounterCallbackImpl::onClients(const sp& service, bool clients) { if (clients) { mNumConnectedServices++; } else { @@ -122,7 +134,7 @@ Status ClientCounterCallback::onClients(const sp& service, bool clients return Status::ok(); } -void ClientCounterCallback::tryShutdown() { +void ClientCounterCallbackImpl::tryShutdown() { if(mNumConnectedServices > 0) { // Should only shut down if there are no clients return; @@ -143,7 +155,6 @@ void ClientCounterCallback::tryShutdown() { bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk(); - if (!success) { ALOGI("Failed to unregister service %s", entry.first.c_str()); break; @@ -168,6 +179,19 @@ void ClientCounterCallback::tryShutdown() { } } +ClientCounterCallback::ClientCounterCallback() { + mImpl = new ClientCounterCallbackImpl(); +} + +bool ClientCounterCallback::registerService(const sp& service, const std::string& name, + bool allowIsolated, int dumpFlags) { + return mImpl->registerService(service, name, allowIsolated, dumpFlags); +} + +void ClientCounterCallback::forcePersist(bool persist) { + mImpl->forcePersist(persist); +} + } // namespace internal LazyServiceRegistrar::LazyServiceRegistrar() { From d12cbfc998606ecaad3eaef1099a7a5ea4d9d851 Mon Sep 17 00:00:00 2001 From: Chavi Weingarten Date: Wed, 9 Dec 2020 00:36:11 +0000 Subject: [PATCH 13/43] resolve merge conflicts of cae2ee036040fc0dce9fc82af9bf8d85240d566b to rvc-dev Change-Id: I906ae5bed4b340817a52f7968276dea035f2e65a Fixes: 169256435 Merged-In: I4d87e90793a88a646aaa1ae5806f118f1ae51e30 (cherry picked from commit 60f3ab275ef3ddf3afcdfdce4eb09b59024fec51) --- services/surfaceflinger/SurfaceFlinger.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 07690cbf32..4a60d5c6f2 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -600,13 +600,6 @@ void SurfaceFlinger::bootFinished() if (mWindowManager != 0) { mWindowManager->linkToDeath(static_cast(this)); } - sp input(defaultServiceManager()->getService( - String16("inputflinger"))); - if (input == nullptr) { - ALOGE("Failed to link to input service"); - } else { - mInputFlinger = interface_cast(input); - } if (mVrFlinger) { mVrFlinger->OnBootFinished(); @@ -621,7 +614,15 @@ void SurfaceFlinger::bootFinished() LOG_EVENT_LONG(LOGTAG_SF_STOP_BOOTANIM, ns2ms(systemTime(SYSTEM_TIME_MONOTONIC))); - static_cast(schedule([this] { + sp input(defaultServiceManager()->getService(String16("inputflinger"))); + + static_cast(schedule([=] { + if (input == nullptr) { + ALOGE("Failed to link to input service"); + } else { + mInputFlinger = interface_cast(input); + } + readPersistentProperties(); mPowerAdvisor.onBootFinished(); mBootStage = BootStage::FINISHED; From 6fa3f0d4219c1c99df17635c9fa510364e7a223a Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Wed, 18 Nov 2020 00:32:42 +0000 Subject: [PATCH 14/43] libbinder: check null bytes in readString*Inplace This is entirely defensive, since the only real guarantee we have here from these APIs is that a buffer of a given length is available. However, since we write 0's here, presumably to guard against people assuming these are null-terminated strings, we might as well enforce that they are actually null terminated. Bug: 172655291 Test: binderParcelTest (added in newer CL) Change-Id: Ie879112540155f6a93b97aeaf3d41ed8ba4ae79f Merged-In: Ie879112540155f6a93b97aeaf3d41ed8ba4ae79f (cherry picked from commit 51e02b16c397c44ddf81a0736cf6045cd4c44128) (cherry picked from commit 58f5cfa56d5282e69a7580dc4bb97603c409f003) --- libs/binder/Parcel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 9642a87f4e..1f7d27e0e9 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -1869,7 +1869,7 @@ const char* Parcel::readString8Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char* str = (const char*)readInplace(size+1); - if (str != nullptr) { + if (str != nullptr && str[size] == '\0') { return str; } } @@ -1929,7 +1929,7 @@ const char16_t* Parcel::readString16Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char16_t* str = (const char16_t*)readInplace((size+1)*sizeof(char16_t)); - if (str != nullptr) { + if (str != nullptr && str[size] == u'\0') { return str; } } From ffa62ed93f06ad9acba14400615cb533671ebc09 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 4 Dec 2020 21:13:03 +0000 Subject: [PATCH 15/43] libbinder: readString*Inplace SafetyNet (II) SafetyNet logs (this time for failure case, instead of success case). Bug: 172655291 Test: adb logcat -b events | grep snet # exactly one occurance w/ repro (c/p'd from 34af0637666f43ae62040ad1bad76468423feba2) Merged-In: I75ace071693c0a4579ed9477f7b9212a6e27c36d Change-Id: I75ace071693c0a4579ed9477f7b9212a6e27c36d (cherry picked from commit 61d0f84881cfc1bbac513ccd156c56603a48cc90) --- libs/binder/Parcel.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 1f7d27e0e9..b7ad660317 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -1869,8 +1869,11 @@ const char* Parcel::readString8Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char* str = (const char*)readInplace(size+1); - if (str != nullptr && str[size] == '\0') { - return str; + if (str != nullptr) { + if (str[size] == '\0') { + return str; + } + android_errorWriteLog(0x534e4554, "172655291"); } } *outLen = 0; @@ -1929,8 +1932,11 @@ const char16_t* Parcel::readString16Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char16_t* str = (const char16_t*)readInplace((size+1)*sizeof(char16_t)); - if (str != nullptr && str[size] == u'\0') { - return str; + if (str != nullptr) { + if (str[size] == u'\0') { + return str; + } + android_errorWriteLog(0x534e4554, "172655291"); } } *outLen = 0; From c02140b857db4dab853a596accf9c96b85ee3198 Mon Sep 17 00:00:00 2001 From: KaiChieh Chuang Date: Tue, 13 Oct 2020 10:38:48 +0800 Subject: [PATCH 16/43] SF: handle long waiting Layer sync point When Layer transaction deferred it wait for barrier layer's specific frame. When the target frame is delayed for a long time, e.g. 5 min the deferred transaction layer may queue up in mPendingStates, may up to 20000 in real case. When the target frame come, it will loop through mPendingStates and call popPendingState with mPendingStates.removeAt(0); which is an inefficient operator for Vector, which cause SWT. Change to use std::deque for mPendingStates & mPendingStatesSnapshot Add sync point timeout debug log. Bug: 170690571 Test: boot up Test: LayerUpdateTest Test: setting pages in/out Change-Id: I17f3751836574c3691c7e5a1e6d2ea6c3fcd3903 Merged-In: I17f3751836574c3691c7e5a1e6d2ea6c3fcd3903 --- services/surfaceflinger/Layer.cpp | 7 +++-- services/surfaceflinger/Layer.h | 43 +++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index e9965d4717..55af849b7f 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -793,7 +793,9 @@ void Layer::pushPendingState() { // to be applied as per normal (no synchronization). mCurrentState.barrierLayer_legacy = nullptr; } else { - auto syncPoint = std::make_shared(mCurrentState.frameNumber_legacy, this); + auto syncPoint = std::make_shared(mCurrentState.frameNumber_legacy, + this, + barrierLayer); if (barrierLayer->addSyncPoint(syncPoint)) { std::stringstream ss; ss << "Adding sync point " << mCurrentState.frameNumber_legacy; @@ -818,7 +820,7 @@ void Layer::popPendingState(State* stateToCommit) { ATRACE_CALL(); *stateToCommit = mPendingStates[0]; - mPendingStates.removeAt(0); + mPendingStates.pop_front(); ATRACE_INT(mTransactionName.c_str(), mPendingStates.size()); } @@ -857,6 +859,7 @@ bool Layer::applyPendingStates(State* stateToCommit) { mRemoteSyncPoints.pop_front(); } else { ATRACE_NAME("!frameIsAvailable"); + mRemoteSyncPoints.front()->checkTimeoutAndLog(); break; } } else { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 2c90c92f6c..d071710542 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -35,6 +35,7 @@ #include #include +#include #include #include #include @@ -881,12 +882,14 @@ class Layer : public virtual RefBase, compositionengine::LayerFE { class SyncPoint { public: - explicit SyncPoint(uint64_t frameNumber, wp requestedSyncLayer) + explicit SyncPoint(uint64_t frameNumber, + wp requestedSyncLayer, + wp barrierLayer_legacy) : mFrameNumber(frameNumber), mFrameIsAvailable(false), mTransactionIsApplied(false), - mRequestedSyncLayer(requestedSyncLayer) {} - + mRequestedSyncLayer(requestedSyncLayer), + mBarrierLayer_legacy(barrierLayer_legacy) {} uint64_t getFrameNumber() const { return mFrameNumber; } bool frameIsAvailable() const { return mFrameIsAvailable; } @@ -899,11 +902,41 @@ class Layer : public virtual RefBase, compositionengine::LayerFE { sp getRequestedSyncLayer() { return mRequestedSyncLayer.promote(); } + sp getBarrierLayer() const { return mBarrierLayer_legacy.promote(); } + + bool isTimeout() const { + using namespace std::chrono_literals; + static constexpr std::chrono::nanoseconds TIMEOUT_THRESHOLD = 1s; + + return std::chrono::steady_clock::now() - mCreateTimeStamp > TIMEOUT_THRESHOLD; + } + + void checkTimeoutAndLog() { + using namespace std::chrono_literals; + static constexpr std::chrono::nanoseconds LOG_PERIOD = 1s; + + if (!frameIsAvailable() && isTimeout()) { + const auto now = std::chrono::steady_clock::now(); + if (now - mLastLogTime > LOG_PERIOD) { + mLastLogTime = now; + sp requestedSyncLayer = getRequestedSyncLayer(); + sp barrierLayer = getBarrierLayer(); + ALOGW("[%s] sync point %" PRIu64 " wait timeout %lld for %s", + requestedSyncLayer ? requestedSyncLayer->getDebugName() : "Removed", + mFrameNumber, (now - mCreateTimeStamp).count(), + barrierLayer ? barrierLayer->getDebugName() : "Removed"); + } + } + } private: const uint64_t mFrameNumber; std::atomic mFrameIsAvailable; std::atomic mTransactionIsApplied; wp mRequestedSyncLayer; + wp mBarrierLayer_legacy; + const std::chrono::time_point mCreateTimeStamp = + std::chrono::steady_clock::now(); + std::chrono::time_point mLastLogTime; }; // SyncPoints which will be signaled when the correct frame is at the head @@ -984,12 +1017,12 @@ class Layer : public virtual RefBase, compositionengine::LayerFE { State mDrawingState; // Store a copy of the pending state so that the drawing thread can access the // states without a lock. - Vector mPendingStatesSnapshot; + std::deque mPendingStatesSnapshot; // these are protected by an external lock (mStateLock) State mCurrentState; std::atomic mTransactionFlags{0}; - Vector mPendingStates; + std::deque mPendingStates; // Timestamp history for UIAutomation. Thread safe. FrameTracker mFrameTracker; From 9e07dbd59ac7178daa320160b33746477155319c Mon Sep 17 00:00:00 2001 From: Snild Dolkow Date: Fri, 22 Jan 2021 14:42:08 +0100 Subject: [PATCH 17/43] Ensure that expected present time is in the future onMessageInvalidate() sets mExpectedPresentTime to a value originating from the vsync event that scheduled it. If handling of the invalidate is delayed, this value will be in the past, which is clearly not a plausible present time. Because of this, queued frames may incorrectly be ignored as "too early". This has caused problems in screen-on animations: even though the first frame is drawn and ready, SurfaceFlinger shows an old frame. This looks particularly bad with a fade-from-black animation: the old frame flashes by before the first frame of the animation shows. Here's an example timeline of how this problem could manifest: 1000 ms INVALIDATE queued with expectedVSyncTimestamp = 1010ms 1013 ms SF calls setPowerMode() 1014 ms SystemUI queues new NotificationShade frame 1255 ms setPowerMode() returns; invalidate can now be handled 1256 ms invalidate runs; mExpectedPresentTime set to 1010ms 1257 ms handlePageFlip() skips NotificationShade frame; "too early" 1259 ms refresh runs, composits prehistoric NotificationShade frame It's a bit of a race: if SystemUI manages to queue its new frame before the INVALIDATE message's timestamp, there won't be a problem. To solve this, let's check that the expected present time is in the future, and pick the nearest future vsync point if it's not. In order to not break frame miss detection, mScheduledPresentTime is introduced and used instead of mExpectedPresentTime for jank calculations. Fixes: 178415552 Test: manual on Xperia device; flash of old frame is gone Test: compiles on aosp/redfin Change-Id: I095f1dd08374fd1d14552cd1af90d95e9718b4dd Merged-In: I095f1dd08374fd1d14552cd1af90d95e9718b4dd --- services/surfaceflinger/SurfaceFlinger.cpp | 12 +++++++++--- services/surfaceflinger/SurfaceFlinger.h | 1 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 493d2dea7b..e01c8c9dd5 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1861,8 +1861,14 @@ void SurfaceFlinger::onMessageInvalidate(nsecs_t expectedVSyncTime) { // calculate the expected present time once and use the cached // value throughout this frame to make sure all layers are // seeing this same value. - const nsecs_t lastExpectedPresentTime = mExpectedPresentTime.load(); - mExpectedPresentTime = expectedVSyncTime; + if (expectedVSyncTime >= frameStart) { + mExpectedPresentTime = expectedVSyncTime; + } else { + mExpectedPresentTime = mScheduler->getDispSyncExpectedPresentTime(frameStart); + } + + const nsecs_t lastScheduledPresentTime = mScheduledPresentTime; + mScheduledPresentTime = 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 @@ -1892,7 +1898,7 @@ void SurfaceFlinger::onMessageInvalidate(nsecs_t expectedVSyncTime) { const TracedOrdinal frameMissed = {"PrevFrameMissed", framePending || (previousPresentTime >= 0 && - (lastExpectedPresentTime < + (lastScheduledPresentTime < previousPresentTime - frameMissedSlop))}; const TracedOrdinal hwcFrameMissed = {"PrevHwcFrameMissed", mHadDeviceComposition && frameMissed}; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 222d6c7511..64d1a4c8f3 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1217,6 +1217,7 @@ class SurfaceFlinger : public BnSurfaceComposer, std::unique_ptr mRefreshRateStats; std::atomic mExpectedPresentTime = 0; + nsecs_t mScheduledPresentTime = 0; hal::Vsync mHWCVsyncPendingState = hal::Vsync::DISABLE; /* ------------------------------------------------------------------------ From 1e7f9e6b6c5b9cf38ada03ad2a6a66ab3d3f7428 Mon Sep 17 00:00:00 2001 From: SeYeong Byeon Date: Tue, 15 Sep 2020 15:02:40 +0900 Subject: [PATCH 18/43] gralloc4: fix PlaneLayout encode typecasting Fixes a bug where planeLayout members were being downcasted to int32_t during the encoding step. Bug: 168564125 Signed-off-by: SeYeong Byeon Change-Id: I8d5139dbd253278193775380ca387d45bfe2589d (cherry picked from commit 389ee53332904b49c8f9fb35ef4e9e624e1ee3d2) --- libs/gralloc/types/Gralloc4.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libs/gralloc/types/Gralloc4.cpp b/libs/gralloc/types/Gralloc4.cpp index 53c68b7230..e2f072a7ab 100644 --- a/libs/gralloc/types/Gralloc4.cpp +++ b/libs/gralloc/types/Gralloc4.cpp @@ -706,35 +706,35 @@ status_t encodePlaneLayout(const PlaneLayout& input, OutputHidlVec* output) { return err; } - err = encodeInteger(static_cast(input.offsetInBytes), output); + err = encodeInteger(static_cast(input.offsetInBytes), output); if (err) { return err; } - err = encodeInteger(static_cast(input.sampleIncrementInBits), output); + err = encodeInteger(static_cast(input.sampleIncrementInBits), output); if (err) { return err; } - err = encodeInteger(static_cast(input.strideInBytes), output); + err = encodeInteger(static_cast(input.strideInBytes), output); if (err) { return err; } - err = encodeInteger(static_cast(input.widthInSamples), output); + err = encodeInteger(static_cast(input.widthInSamples), output); if (err) { return err; } - err = encodeInteger(static_cast(input.heightInSamples), output); + err = encodeInteger(static_cast(input.heightInSamples), output); if (err) { return err; } - err = encodeInteger(static_cast(input.totalSizeInBytes), output); + err = encodeInteger(static_cast(input.totalSizeInBytes), output); if (err) { return err; } - err = encodeInteger(static_cast(input.horizontalSubsampling), output); + err = encodeInteger(static_cast(input.horizontalSubsampling), output); if (err) { return err; } - return encodeInteger(static_cast(input.verticalSubsampling), output); + return encodeInteger(static_cast(input.verticalSubsampling), output); } status_t decodePlaneLayout(InputHidlVec* input, PlaneLayout* output) { From 92978652979f86ef90474c309f4f695100a8c72f Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 3 Mar 2021 01:32:21 +0000 Subject: [PATCH 19/43] Do not modify vector after getting references We used to obtain a reference to a specific element inside a vector. We would then modify the vector, invalidating the reference. But we then used the reference, and passed it to 'assignPointerIds'. Refactor the code to modify the collection first, and then to proceed with modifying / reading the elements. Bug: 179839665 Test: atest inputflinger_tests (on a hwasan build) Merged-In: I9204b954884e9c83a50babdad5e08a0f6d18ad78 Change-Id: I9204b954884e9c83a50babdad5e08a0f6d18ad78 (cherry picked from commit 8cf78f9553981600f57e9c829886848172114484) --- .../reader/mapper/TouchInputMapper.cpp | 75 ++++++++++--------- .../reader/mapper/TouchInputMapper.h | 2 +- 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp index 99a572a5fd..decbea4c3e 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -1413,27 +1413,28 @@ void TouchInputMapper::process(const RawEvent* rawEvent) { } void TouchInputMapper::sync(nsecs_t when) { - const RawState* last = - mRawStatesPending.empty() ? &mCurrentRawState : &mRawStatesPending.back(); - // Push a new state. mRawStatesPending.emplace_back(); - RawState* next = &mRawStatesPending.back(); - next->clear(); - next->when = when; + RawState& next = mRawStatesPending.back(); + next.clear(); + next.when = when; // Sync button state. - next->buttonState = + next.buttonState = mTouchButtonAccumulator.getButtonState() | mCursorButtonAccumulator.getButtonState(); // Sync scroll - next->rawVScroll = mCursorScrollAccumulator.getRelativeVWheel(); - next->rawHScroll = mCursorScrollAccumulator.getRelativeHWheel(); + next.rawVScroll = mCursorScrollAccumulator.getRelativeVWheel(); + next.rawHScroll = mCursorScrollAccumulator.getRelativeHWheel(); mCursorScrollAccumulator.finishSync(); // Sync touch - syncTouch(when, next); + syncTouch(when, &next); + + // The last RawState is the actually second to last, since we just added a new state + const RawState& last = + mRawStatesPending.size() == 1 ? mCurrentRawState : mRawStatesPending.rbegin()[1]; // Assign pointer ids. if (!mHavePointerIds) { @@ -1443,9 +1444,9 @@ void TouchInputMapper::sync(nsecs_t when) { #if DEBUG_RAW_EVENTS ALOGD("syncTouch: pointerCount %d -> %d, touching ids 0x%08x -> 0x%08x, " "hovering ids 0x%08x -> 0x%08x", - last->rawPointerData.pointerCount, next->rawPointerData.pointerCount, - last->rawPointerData.touchingIdBits.value, next->rawPointerData.touchingIdBits.value, - last->rawPointerData.hoveringIdBits.value, next->rawPointerData.hoveringIdBits.value); + last.rawPointerData.pointerCount, next.rawPointerData.pointerCount, + last.rawPointerData.touchingIdBits.value, next.rawPointerData.touchingIdBits.value, + last.rawPointerData.hoveringIdBits.value, next.rawPointerData.hoveringIdBits.value); #endif processRawTouches(false /*timeout*/); @@ -3679,11 +3680,11 @@ const TouchInputMapper::VirtualKey* TouchInputMapper::findVirtualKeyHit(int32_t return nullptr; } -void TouchInputMapper::assignPointerIds(const RawState* last, RawState* current) { - uint32_t currentPointerCount = current->rawPointerData.pointerCount; - uint32_t lastPointerCount = last->rawPointerData.pointerCount; +void TouchInputMapper::assignPointerIds(const RawState& last, RawState& current) { + uint32_t currentPointerCount = current.rawPointerData.pointerCount; + uint32_t lastPointerCount = last.rawPointerData.pointerCount; - current->rawPointerData.clearIdBits(); + current.rawPointerData.clearIdBits(); if (currentPointerCount == 0) { // No pointers to assign. @@ -3694,20 +3695,20 @@ void TouchInputMapper::assignPointerIds(const RawState* last, RawState* current) // All pointers are new. for (uint32_t i = 0; i < currentPointerCount; i++) { uint32_t id = i; - current->rawPointerData.pointers[i].id = id; - current->rawPointerData.idToIndex[id] = i; - current->rawPointerData.markIdBit(id, current->rawPointerData.isHovering(i)); + current.rawPointerData.pointers[i].id = id; + current.rawPointerData.idToIndex[id] = i; + current.rawPointerData.markIdBit(id, current.rawPointerData.isHovering(i)); } return; } if (currentPointerCount == 1 && lastPointerCount == 1 && - current->rawPointerData.pointers[0].toolType == last->rawPointerData.pointers[0].toolType) { + current.rawPointerData.pointers[0].toolType == last.rawPointerData.pointers[0].toolType) { // Only one pointer and no change in count so it must have the same id as before. - uint32_t id = last->rawPointerData.pointers[0].id; - current->rawPointerData.pointers[0].id = id; - current->rawPointerData.idToIndex[id] = 0; - current->rawPointerData.markIdBit(id, current->rawPointerData.isHovering(0)); + uint32_t id = last.rawPointerData.pointers[0].id; + current.rawPointerData.pointers[0].id = id; + current.rawPointerData.idToIndex[id] = 0; + current.rawPointerData.markIdBit(id, current.rawPointerData.isHovering(0)); return; } @@ -3725,9 +3726,9 @@ void TouchInputMapper::assignPointerIds(const RawState* last, RawState* current) for (uint32_t lastPointerIndex = 0; lastPointerIndex < lastPointerCount; lastPointerIndex++) { const RawPointerData::Pointer& currentPointer = - current->rawPointerData.pointers[currentPointerIndex]; + current.rawPointerData.pointers[currentPointerIndex]; const RawPointerData::Pointer& lastPointer = - last->rawPointerData.pointers[lastPointerIndex]; + last.rawPointerData.pointers[lastPointerIndex]; if (currentPointer.toolType == lastPointer.toolType) { int64_t deltaX = currentPointer.x - lastPointer.x; int64_t deltaY = currentPointer.y - lastPointer.y; @@ -3831,12 +3832,12 @@ void TouchInputMapper::assignPointerIds(const RawState* last, RawState* current) matchedCurrentBits.markBit(currentPointerIndex); matchedLastBits.markBit(lastPointerIndex); - uint32_t id = last->rawPointerData.pointers[lastPointerIndex].id; - current->rawPointerData.pointers[currentPointerIndex].id = id; - current->rawPointerData.idToIndex[id] = currentPointerIndex; - current->rawPointerData.markIdBit(id, - current->rawPointerData.isHovering( - currentPointerIndex)); + uint32_t id = last.rawPointerData.pointers[lastPointerIndex].id; + current.rawPointerData.pointers[currentPointerIndex].id = id; + current.rawPointerData.idToIndex[id] = currentPointerIndex; + current.rawPointerData.markIdBit(id, + current.rawPointerData.isHovering( + currentPointerIndex)); usedIdBits.markBit(id); #if DEBUG_POINTER_ASSIGNMENT @@ -3853,10 +3854,10 @@ void TouchInputMapper::assignPointerIds(const RawState* last, RawState* current) uint32_t currentPointerIndex = matchedCurrentBits.markFirstUnmarkedBit(); uint32_t id = usedIdBits.markFirstUnmarkedBit(); - current->rawPointerData.pointers[currentPointerIndex].id = id; - current->rawPointerData.idToIndex[id] = currentPointerIndex; - current->rawPointerData.markIdBit(id, - current->rawPointerData.isHovering(currentPointerIndex)); + current.rawPointerData.pointers[currentPointerIndex].id = id; + current.rawPointerData.idToIndex[id] = currentPointerIndex; + current.rawPointerData.markIdBit(id, + current.rawPointerData.isHovering(currentPointerIndex)); #if DEBUG_POINTER_ASSIGNMENT ALOGD("assignPointerIds - assigned: cur=%" PRIu32 ", id=%" PRIu32, currentPointerIndex, id); diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.h b/services/inputflinger/reader/mapper/TouchInputMapper.h index 58bfc5c596..1c2cc18f91 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.h +++ b/services/inputflinger/reader/mapper/TouchInputMapper.h @@ -758,7 +758,7 @@ class TouchInputMapper : public InputMapper { bool isPointInsideSurface(int32_t x, int32_t y); const VirtualKey* findVirtualKeyHit(int32_t x, int32_t y); - static void assignPointerIds(const RawState* last, RawState* current); + static void assignPointerIds(const RawState& last, RawState& current); const char* modeToString(DeviceMode deviceMode); void rotateAndScale(float& x, float& y); From ed2228209cffc4c51ac8adf6271d53f0908a0d50 Mon Sep 17 00:00:00 2001 From: Aaron Kling Date: Sun, 21 Feb 2021 22:34:09 -0600 Subject: [PATCH 20/43] Properly scale touch input window through resolution changes The previous code would limit the touch window to the original resolution. Credit: https://stackoverflow.com/a/61284787 Change-Id: I6e93a00ac3009ab1c7b89135dacad302b020dc9f --- services/inputflinger/reader/mapper/TouchInputMapper.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp index 99a572a5fd..9a9e31b24a 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -3653,13 +3653,8 @@ void TouchInputMapper::rotateAndScale(float& x, float& y) { } bool TouchInputMapper::isPointInsideSurface(int32_t x, int32_t y) { - const float xScaled = (x - mRawPointerAxes.x.minValue) * mXScale; - const float yScaled = (y - mRawPointerAxes.y.minValue) * mYScale; - return x >= mRawPointerAxes.x.minValue && x <= mRawPointerAxes.x.maxValue && - xScaled >= mSurfaceLeft && xScaled <= mSurfaceRight && - y >= mRawPointerAxes.y.minValue && y <= mRawPointerAxes.y.maxValue && - yScaled >= mSurfaceTop && yScaled <= mSurfaceBottom; + y >= mRawPointerAxes.y.minValue && y <= mRawPointerAxes.y.maxValue; } const TouchInputMapper::VirtualKey* TouchInputMapper::findVirtualKeyHit(int32_t x, int32_t y) { From 240b8119bc45fc604ff163038d5a3d4ce95cdae1 Mon Sep 17 00:00:00 2001 From: Sean McCreary Date: Thu, 17 Jun 2021 10:39:54 -0600 Subject: [PATCH 21/43] Use the original device ID when the injected event is unchanged * This is a partial revert of 0d8ed6e8ade20652cea20f579f40b1d698ce8fc0 a.k.a. Change-Id I9a61a99cf5f8ca1a27e4526dd6feedf2c1beec0f which forced all injected events to appear to be from virtual keyboard ID. * Fixes some cases described in https://issuetracker.google.com/issues/163120692 There may be accessibility apps that modify events in such a way that they do not appear equivalent to the original event and therefore continue to have their originated device ID being overridden to the virtual keyboard ID. Fixing this fully needs API enhancements between system server and accessibility filters to allow conveying the changes the filter wishes to make. * Add native MotionEvent::equals helper that's used by fw/b InputManagerService to determine whether or not an input filter modified a MotionEvent. All fields except mDownTime are checked for precise equality. There are two issues with mDowntime: - native uses ns, system service uses ms so there is a loss of precision during inputflinger -> system server -> inputflinger round trip. - the magnify accessibility helper modifies this field with MotionEvent.setDownTime() Change-Id: Ib980ead9005934ebab65eece1a20cba0e2866b9a --- include/input/Input.h | 6 +++ libs/input/Input.cpp | 42 +++++++++++++++++++ .../dispatcher/InputDispatcher.cpp | 18 +++++--- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/include/input/Input.h b/include/input/Input.h index 54b4e5a737..57a05ada90 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -227,6 +227,10 @@ enum { // Disables automatic key repeating behavior. POLICY_FLAG_DISABLE_KEY_REPEAT = 0x08000000, + // Indicates whether an injected event is an unmodified copy of what was originally + // a real input event. + POLICY_FLAGS_INJECTED_IS_UNCHANGED = 0x10000000, + /* These flags are set by the input reader policy as it intercepts each event. */ // Indicates that the device was in an interactive state when the @@ -693,6 +697,8 @@ class MotionEvent : public InputEvent { void copyFrom(const MotionEvent* other, bool keepHistory); + bool equals(const MotionEvent* other) const; + void addSample( nsecs_t eventTime, const PointerCoords* pointerCoords); diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index 31aa685391..36428a3727 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -17,6 +17,7 @@ #define LOG_TAG "Input" //#define LOG_NDEBUG 0 +#include #include #include #include @@ -385,6 +386,47 @@ void MotionEvent::copyFrom(const MotionEvent* other, bool keepHistory) { } } +bool MotionEvent::equals(const MotionEvent* other) const { + /* + * All fields are checked for precise equality except for mDownTime + * which is not checked because: + * 1) magnify accessibility service uses MotionEvent.setDownTime() + * 2) fw/b InputManagerService converts from ns to ms so precision is + * different after inputflinger -> system server -> inputflinger + * round trip. + */ + return mId == other->mId + && mDeviceId == other->mDeviceId + && mSource == other->mSource + && mDisplayId == other->mDisplayId + && mAction == other->mAction + && mActionButton == other->mActionButton + && mFlags == other->mFlags + && mEdgeFlags == other->mEdgeFlags + && mMetaState == other->mMetaState + && mButtonState == other->mButtonState + && mClassification == other->mClassification + && mXScale == other->mXScale + && mYScale == other->mYScale + && mXOffset == other->mXOffset + && mYOffset == other->mYOffset + && mXPrecision == other->mXPrecision + && mYPrecision == other->mYPrecision + && ((std::isnan(mRawXCursorPosition) && std::isnan(other->mRawXCursorPosition)) + || mRawXCursorPosition == other->mRawXCursorPosition) + && ((std::isnan(mRawYCursorPosition) && std::isnan(other->mRawYCursorPosition)) + || mRawYCursorPosition == other->mRawYCursorPosition) + && mPointerProperties.size() == other->mPointerProperties.size() + && std::equal(mPointerProperties.begin(), mPointerProperties.end(), + other->mPointerProperties.begin()) + && mSampleEventTimes.size() == other->mSampleEventTimes.size() + && std::equal(mSampleEventTimes.begin(), mSampleEventTimes.end(), + other->mSampleEventTimes.begin()) + && mSamplePointerCoords.size() == other->mSamplePointerCoords.size() + && std::equal(mSamplePointerCoords.begin(), mSamplePointerCoords.end(), + other->mSamplePointerCoords.begin()); +} + void MotionEvent::addSample( int64_t eventTime, const PointerCoords* pointerCoords) { diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index dd0082c09e..95704f2527 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -3291,6 +3291,8 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec if (hasInjectionPermission(injectorPid, injectorUid)) { policyFlags |= POLICY_FLAG_TRUSTED; } + // Override device ID if the injected event is not an unmodified real input. + bool forceVirtualKeyboardId = !(policyFlags & POLICY_FLAGS_INJECTED_IS_UNCHANGED); std::queue injectedEntries; switch (event->getType()) { @@ -3303,11 +3305,14 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec int32_t flags = incomingKey.getFlags(); int32_t keyCode = incomingKey.getKeyCode(); + int32_t deviceId = forceVirtualKeyboardId + ? VIRTUAL_KEYBOARD_ID + : incomingKey.getDeviceId(); int32_t metaState = incomingKey.getMetaState(); - accelerateMetaShortcuts(VIRTUAL_KEYBOARD_ID, action, + accelerateMetaShortcuts(deviceId, action, /*byref*/ keyCode, /*byref*/ metaState); KeyEvent keyEvent; - keyEvent.initialize(incomingKey.getId(), VIRTUAL_KEYBOARD_ID, incomingKey.getSource(), + keyEvent.initialize(incomingKey.getId(), deviceId, incomingKey.getSource(), incomingKey.getDisplayId(), INVALID_HMAC, action, flags, keyCode, incomingKey.getScanCode(), metaState, incomingKey.getRepeatCount(), incomingKey.getDownTime(), incomingKey.getEventTime()); @@ -3328,7 +3333,7 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec mLock.lock(); KeyEntry* injectedEntry = new KeyEntry(incomingKey.getId(), incomingKey.getEventTime(), - VIRTUAL_KEYBOARD_ID, incomingKey.getSource(), + deviceId, incomingKey.getSource(), incomingKey.getDisplayId(), policyFlags, action, flags, keyCode, incomingKey.getScanCode(), metaState, incomingKey.getRepeatCount(), incomingKey.getDownTime()); @@ -3346,6 +3351,9 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec if (!validateMotionEvent(action, actionButton, pointerCount, pointerProperties)) { return INPUT_EVENT_INJECTION_FAILED; } + int32_t deviceId = forceVirtualKeyboardId + ? VIRTUAL_KEYBOARD_ID + : motionEvent->getDeviceId(); if (!(policyFlags & POLICY_FLAG_FILTERED)) { nsecs_t eventTime = motionEvent->getEventTime(); @@ -3361,7 +3369,7 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec const nsecs_t* sampleEventTimes = motionEvent->getSampleEventTimes(); const PointerCoords* samplePointerCoords = motionEvent->getSamplePointerCoords(); MotionEntry* injectedEntry = - new MotionEntry(motionEvent->getId(), *sampleEventTimes, VIRTUAL_KEYBOARD_ID, + new MotionEntry(motionEvent->getId(), *sampleEventTimes, deviceId, motionEvent->getSource(), motionEvent->getDisplayId(), policyFlags, action, actionButton, motionEvent->getFlags(), motionEvent->getMetaState(), motionEvent->getButtonState(), @@ -3378,7 +3386,7 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec samplePointerCoords += pointerCount; MotionEntry* nextInjectedEntry = new MotionEntry(motionEvent->getId(), *sampleEventTimes, - VIRTUAL_KEYBOARD_ID, motionEvent->getSource(), + deviceId, motionEvent->getSource(), motionEvent->getDisplayId(), policyFlags, action, actionButton, motionEvent->getFlags(), motionEvent->getMetaState(), motionEvent->getButtonState(), From 23c9f4198a13c0817f521214fa6174d84ad3c76f Mon Sep 17 00:00:00 2001 From: Sam Mortimer Date: Wed, 28 Jul 2021 16:35:46 -0700 Subject: [PATCH 22/43] Revert "Use the original device ID when the injected event is unchanged" This reverts commit 240b8119bc45fc604ff163038d5a3d4ce95cdae1. Change-Id: If3b0297cfdb05fcf1d7dbc6a305733d1a7184cdd --- include/input/Input.h | 6 --- libs/input/Input.cpp | 42 ------------------- .../dispatcher/InputDispatcher.cpp | 18 +++----- 3 files changed, 5 insertions(+), 61 deletions(-) diff --git a/include/input/Input.h b/include/input/Input.h index 57a05ada90..54b4e5a737 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -227,10 +227,6 @@ enum { // Disables automatic key repeating behavior. POLICY_FLAG_DISABLE_KEY_REPEAT = 0x08000000, - // Indicates whether an injected event is an unmodified copy of what was originally - // a real input event. - POLICY_FLAGS_INJECTED_IS_UNCHANGED = 0x10000000, - /* These flags are set by the input reader policy as it intercepts each event. */ // Indicates that the device was in an interactive state when the @@ -697,8 +693,6 @@ class MotionEvent : public InputEvent { void copyFrom(const MotionEvent* other, bool keepHistory); - bool equals(const MotionEvent* other) const; - void addSample( nsecs_t eventTime, const PointerCoords* pointerCoords); diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index 36428a3727..31aa685391 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -17,7 +17,6 @@ #define LOG_TAG "Input" //#define LOG_NDEBUG 0 -#include #include #include #include @@ -386,47 +385,6 @@ void MotionEvent::copyFrom(const MotionEvent* other, bool keepHistory) { } } -bool MotionEvent::equals(const MotionEvent* other) const { - /* - * All fields are checked for precise equality except for mDownTime - * which is not checked because: - * 1) magnify accessibility service uses MotionEvent.setDownTime() - * 2) fw/b InputManagerService converts from ns to ms so precision is - * different after inputflinger -> system server -> inputflinger - * round trip. - */ - return mId == other->mId - && mDeviceId == other->mDeviceId - && mSource == other->mSource - && mDisplayId == other->mDisplayId - && mAction == other->mAction - && mActionButton == other->mActionButton - && mFlags == other->mFlags - && mEdgeFlags == other->mEdgeFlags - && mMetaState == other->mMetaState - && mButtonState == other->mButtonState - && mClassification == other->mClassification - && mXScale == other->mXScale - && mYScale == other->mYScale - && mXOffset == other->mXOffset - && mYOffset == other->mYOffset - && mXPrecision == other->mXPrecision - && mYPrecision == other->mYPrecision - && ((std::isnan(mRawXCursorPosition) && std::isnan(other->mRawXCursorPosition)) - || mRawXCursorPosition == other->mRawXCursorPosition) - && ((std::isnan(mRawYCursorPosition) && std::isnan(other->mRawYCursorPosition)) - || mRawYCursorPosition == other->mRawYCursorPosition) - && mPointerProperties.size() == other->mPointerProperties.size() - && std::equal(mPointerProperties.begin(), mPointerProperties.end(), - other->mPointerProperties.begin()) - && mSampleEventTimes.size() == other->mSampleEventTimes.size() - && std::equal(mSampleEventTimes.begin(), mSampleEventTimes.end(), - other->mSampleEventTimes.begin()) - && mSamplePointerCoords.size() == other->mSamplePointerCoords.size() - && std::equal(mSamplePointerCoords.begin(), mSamplePointerCoords.end(), - other->mSamplePointerCoords.begin()); -} - void MotionEvent::addSample( int64_t eventTime, const PointerCoords* pointerCoords) { diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 95704f2527..dd0082c09e 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -3291,8 +3291,6 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec if (hasInjectionPermission(injectorPid, injectorUid)) { policyFlags |= POLICY_FLAG_TRUSTED; } - // Override device ID if the injected event is not an unmodified real input. - bool forceVirtualKeyboardId = !(policyFlags & POLICY_FLAGS_INJECTED_IS_UNCHANGED); std::queue injectedEntries; switch (event->getType()) { @@ -3305,14 +3303,11 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec int32_t flags = incomingKey.getFlags(); int32_t keyCode = incomingKey.getKeyCode(); - int32_t deviceId = forceVirtualKeyboardId - ? VIRTUAL_KEYBOARD_ID - : incomingKey.getDeviceId(); int32_t metaState = incomingKey.getMetaState(); - accelerateMetaShortcuts(deviceId, action, + accelerateMetaShortcuts(VIRTUAL_KEYBOARD_ID, action, /*byref*/ keyCode, /*byref*/ metaState); KeyEvent keyEvent; - keyEvent.initialize(incomingKey.getId(), deviceId, incomingKey.getSource(), + keyEvent.initialize(incomingKey.getId(), VIRTUAL_KEYBOARD_ID, incomingKey.getSource(), incomingKey.getDisplayId(), INVALID_HMAC, action, flags, keyCode, incomingKey.getScanCode(), metaState, incomingKey.getRepeatCount(), incomingKey.getDownTime(), incomingKey.getEventTime()); @@ -3333,7 +3328,7 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec mLock.lock(); KeyEntry* injectedEntry = new KeyEntry(incomingKey.getId(), incomingKey.getEventTime(), - deviceId, incomingKey.getSource(), + VIRTUAL_KEYBOARD_ID, incomingKey.getSource(), incomingKey.getDisplayId(), policyFlags, action, flags, keyCode, incomingKey.getScanCode(), metaState, incomingKey.getRepeatCount(), incomingKey.getDownTime()); @@ -3351,9 +3346,6 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec if (!validateMotionEvent(action, actionButton, pointerCount, pointerProperties)) { return INPUT_EVENT_INJECTION_FAILED; } - int32_t deviceId = forceVirtualKeyboardId - ? VIRTUAL_KEYBOARD_ID - : motionEvent->getDeviceId(); if (!(policyFlags & POLICY_FLAG_FILTERED)) { nsecs_t eventTime = motionEvent->getEventTime(); @@ -3369,7 +3361,7 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec const nsecs_t* sampleEventTimes = motionEvent->getSampleEventTimes(); const PointerCoords* samplePointerCoords = motionEvent->getSamplePointerCoords(); MotionEntry* injectedEntry = - new MotionEntry(motionEvent->getId(), *sampleEventTimes, deviceId, + new MotionEntry(motionEvent->getId(), *sampleEventTimes, VIRTUAL_KEYBOARD_ID, motionEvent->getSource(), motionEvent->getDisplayId(), policyFlags, action, actionButton, motionEvent->getFlags(), motionEvent->getMetaState(), motionEvent->getButtonState(), @@ -3386,7 +3378,7 @@ int32_t InputDispatcher::injectInputEvent(const InputEvent* event, int32_t injec samplePointerCoords += pointerCount; MotionEntry* nextInjectedEntry = new MotionEntry(motionEvent->getId(), *sampleEventTimes, - deviceId, motionEvent->getSource(), + VIRTUAL_KEYBOARD_ID, motionEvent->getSource(), motionEvent->getDisplayId(), policyFlags, action, actionButton, motionEvent->getFlags(), motionEvent->getMetaState(), motionEvent->getButtonState(), From 571b407460a7c0f6c2d90835b12ec6c50845fd79 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 3 Sep 2021 22:39:10 +0000 Subject: [PATCH 23/43] libbinder: uptimeMillis returns int64_t! am: 3ba4963f5b am: 17aa765fd3 am: d666af6990 Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/native/+/15720346 Bug: 197336441 Change-Id: I703760ecbfa007d27caf76556ed21bbf0558df80 (cherry picked from commit 62eaabc3242b5e4ae5da7d39928123fda570f0b2) --- libs/binder/IServiceManager.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index a9f2d73951..218970a1b2 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -215,7 +215,8 @@ sp ServiceManagerShim::getService(const String16& name) const const bool isVendorService = strcmp(ProcessState::self()->getDriverName().c_str(), "/dev/vndbinder") == 0; - const long timeout = uptimeMillis() + 5000; + const long timeout = 5000; + int64_t startTime = uptimeMillis(); // Vendor code can't access system properties if (!gSystemBootCompleted && !isVendorService) { #ifdef __ANDROID__ @@ -230,7 +231,7 @@ sp ServiceManagerShim::getService(const String16& name) const const long sleepTime = gSystemBootCompleted ? 1000 : 100; int n = 0; - while (uptimeMillis() < timeout) { + while (uptimeMillis() - startTime < timeout) { n++; ALOGI("Waiting for service '%s' on '%s'...", String8(name).string(), ProcessState::self()->getDriverName().c_str()); From a9d41ef7a9a7e4056cf3729146ecc94a3a9ce038 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Mon, 1 Nov 2021 18:17:23 -0700 Subject: [PATCH 24/43] avoid extra release of unowned objects in Parcel error path Another bug due to a huge amount of complexity in the Parcel implementation. Bug: 203847542 Test: added testcase fails on device w/o Parcel.cpp fix, and it passes on a device with the fix Merged-In: I34411675687cb3d18bffa082984ebdf308e1c1a6 Change-Id: I34411675687cb3d18bffa082984ebdf308e1c1a6 (cherry picked from commit 04390376b043bf6a15ff2943a9ed63d9d8173842) (cherry picked from commit d668098e4714025b41052207c9332de86dc3936a) Merged-In:I34411675687cb3d18bffa082984ebdf308e1c1a6 --- libs/binder/Parcel.cpp | 8 +++-- libs/binder/tests/binderLibTest.cpp | 45 +++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index b7ad660317..efcc042b3c 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -2322,12 +2322,14 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, type == BINDER_TYPE_FD)) { // We should never receive other types (eg BINDER_TYPE_FDA) as long as we don't support // them in libbinder. If we do receive them, it probably means a kernel bug; try to - // recover gracefully by clearing out the objects, and releasing the objects we do - // know about. + // recover gracefully by clearing out the objects. android_errorWriteLog(0x534e4554, "135930648"); + android_errorWriteLog(0x534e4554, "203847542"); ALOGE("%s: unsupported type object (%" PRIu32 ") at offset %" PRIu64 "\n", __func__, type, (uint64_t)offset); - releaseObjects(); + + // WARNING: callers of ipcSetDataReference need to make sure they + // don't rely on mObjectsSize in their release_func. mObjectsSize = 0; break; } diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 40de2db2cb..b37b688a5a 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -76,7 +76,7 @@ enum BinderLibTestTranscationCode { BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION, BINDER_LIB_TEST_GET_WORK_SOURCE_TRANSACTION, BINDER_LIB_TEST_ECHO_VECTOR, - BINDER_LIB_TEST_REJECT_BUF, + BINDER_LIB_TEST_REJECT_OBJECTS, }; pid_t start_server_process(int arg2, bool usePoll = false) @@ -1050,14 +1050,53 @@ TEST_F(BinderLibTest, BufRejected) { // And now, overwrite it with the buffer object memcpy(parcelData, &obj, sizeof(obj)); data.setDataSize(sizeof(obj)); + EXPECT_EQ(data.objectsCount(), 1); + + status_t ret = server->transact(BINDER_LIB_TEST_REJECT_OBJECTS, data, &reply); - status_t ret = server->transact(BINDER_LIB_TEST_REJECT_BUF, data, &reply); // Either the kernel should reject this transaction (if it's correct), but // if it's not, the server implementation should return an error if it // finds an object in the received Parcel. EXPECT_NE(NO_ERROR, ret); } +TEST_F(BinderLibTest, WeakRejected) { + Parcel data, reply; + sp server = addServer(); + ASSERT_TRUE(server != nullptr); + + sp binder = new BBinder(); + wp wpBinder(binder); + flat_binder_object obj{ + .hdr = {.type = BINDER_TYPE_WEAK_BINDER}, + .flags = 0, + .binder = reinterpret_cast(wpBinder.get_refs()), + .cookie = reinterpret_cast(wpBinder.unsafe_get()), + }; + data.setDataCapacity(1024); + // Write a bogus object at offset 0 to get an entry in the offset table + data.writeFileDescriptor(0); + EXPECT_EQ(data.objectsCount(), 1); + uint8_t *parcelData = const_cast(data.data()); + // And now, overwrite it with the weak binder + memcpy(parcelData, &obj, sizeof(obj)); + data.setDataSize(sizeof(obj)); + + // a previous bug caused other objects to be released an extra time, so we + // test with an object that libbinder will actually try to release + EXPECT_EQ(OK, data.writeStrongBinder(new BBinder())); + + EXPECT_EQ(data.objectsCount(), 2); + + // send it many times, since previous error was memory corruption, make it + // more likely that the server crashes + for (size_t i = 0; i < 100; i++) { + EXPECT_EQ(BAD_VALUE, server->transact(BINDER_LIB_TEST_REJECT_OBJECTS, data, &reply)); + } + + EXPECT_EQ(NO_ERROR, server->pingBinder()); +} + class BinderLibTestService : public BBinder { public: @@ -1340,7 +1379,7 @@ class BinderLibTestService : public BBinder reply->writeUint64Vector(vector); return NO_ERROR; } - case BINDER_LIB_TEST_REJECT_BUF: { + case BINDER_LIB_TEST_REJECT_OBJECTS: { return data.objectsCount() == 0 ? BAD_VALUE : NO_ERROR; } default: From 53414caeb632ea809feed02b370065342c8fd007 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 9 Dec 2020 08:07:46 -1000 Subject: [PATCH 25/43] Check if the window is partially obscured for slippery enters Currently, we only check whether a window is partially obscured during the initial tap down. However, there is another use case: slippery enter. During a slippery enter, the touch down is generated into the slipped-into window, and touch cancel is generated for the slipped-from window. The window receiving the slippery enter does not need to have any flags. Until we figure out whether we can restrict the usage of this flag to system components, add this check as an intermediate fix. Bug: 157929241 Test: atest FlagSlipperyTest Test: atest inputflinger_tests Change-Id: I93d9681479f41244ffed4b1f88cceb69be71adf2 Merged-In: I93d9681479f41244ffed4b1f88cceb69be71adf2 (cherry picked from commit d8c6ef21387db53930d728272db24cca1cd38a38) Merged-In:I93d9681479f41244ffed4b1f88cceb69be71adf2 --- .../dispatcher/InputDispatcher.cpp | 2 + .../tests/InputDispatcher_test.cpp | 205 ++++++++++++------ 2 files changed, 145 insertions(+), 62 deletions(-) diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index fe016af01f..8907b3576d 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -1782,6 +1782,8 @@ int32_t InputDispatcher::findTouchedWindowTargetsLocked(nsecs_t currentTime, } if (isWindowObscuredAtPointLocked(newTouchedWindowHandle, x, y)) { targetFlags |= InputTarget::FLAG_WINDOW_IS_OBSCURED; + } else if (isWindowObscuredLocked(newTouchedWindowHandle)) { + targetFlags |= InputTarget::FLAG_WINDOW_IS_PARTIALLY_OBSCURED; } BitSet32 pointerIds; diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 1a133dc01c..86c0503662 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -19,10 +19,10 @@ #include #include #include -#include - #include +#include #include + #include #include #include @@ -68,12 +68,10 @@ class FakeInputDispatcherPolicy : public InputDispatcherPolicyInterface { InputDispatcherConfiguration mConfig; protected: - virtual ~FakeInputDispatcherPolicy() { - } + virtual ~FakeInputDispatcherPolicy() {} public: - FakeInputDispatcherPolicy() { - } + FakeInputDispatcherPolicy() {} void assertFilterInputEventWasCalled(const NotifyKeyArgs& args) { assertFilterInputEventWasCalled(AINPUT_EVENT_TYPE_KEY, args.eventTime, args.action, @@ -366,7 +364,7 @@ class InputDispatcherTest : public testing::Test { mFakePolicy = new FakeInputDispatcherPolicy(); mDispatcher = new InputDispatcher(mFakePolicy); mDispatcher->setInputDispatchMode(/*enabled*/ true, /*frozen*/ false); - //Start InputDispatcher thread + // Start InputDispatcher thread ASSERT_EQ(OK, mDispatcher->start()); } @@ -391,7 +389,6 @@ class InputDispatcherTest : public testing::Test { } }; - TEST_F(InputDispatcherTest, InjectInputEvent_ValidatesKeyEvents) { KeyEvent event; @@ -589,9 +586,7 @@ class FakeApplicationHandle : public InputApplicationHandle { } virtual ~FakeApplicationHandle() {} - virtual bool updateInfo() override { - return true; - } + virtual bool updateInfo() override { return true; } void setDispatchingTimeout(std::chrono::nanoseconds timeout) { mInfo.dispatchingTimeout = timeout.count(); @@ -822,39 +817,40 @@ class FakeWindowHandle : public InputWindowHandle { } void consumeMotionCancel(int32_t expectedDisplayId = ADISPLAY_ID_DEFAULT, - int32_t expectedFlags = 0) { + int32_t expectedFlags = 0) { consumeEvent(AINPUT_EVENT_TYPE_MOTION, AMOTION_EVENT_ACTION_CANCEL, expectedDisplayId, expectedFlags); } void consumeMotionMove(int32_t expectedDisplayId = ADISPLAY_ID_DEFAULT, - int32_t expectedFlags = 0) { + int32_t expectedFlags = 0) { consumeEvent(AINPUT_EVENT_TYPE_MOTION, AMOTION_EVENT_ACTION_MOVE, expectedDisplayId, expectedFlags); } void consumeMotionDown(int32_t expectedDisplayId = ADISPLAY_ID_DEFAULT, - int32_t expectedFlags = 0) { + int32_t expectedFlags = 0) { consumeEvent(AINPUT_EVENT_TYPE_MOTION, AMOTION_EVENT_ACTION_DOWN, expectedDisplayId, expectedFlags); } void consumeMotionPointerDown(int32_t pointerIdx, - int32_t expectedDisplayId = ADISPLAY_ID_DEFAULT, int32_t expectedFlags = 0) { - int32_t action = AMOTION_EVENT_ACTION_POINTER_DOWN - | (pointerIdx << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT); + int32_t expectedDisplayId = ADISPLAY_ID_DEFAULT, + int32_t expectedFlags = 0) { + int32_t action = AMOTION_EVENT_ACTION_POINTER_DOWN | + (pointerIdx << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT); consumeEvent(AINPUT_EVENT_TYPE_MOTION, action, expectedDisplayId, expectedFlags); } void consumeMotionPointerUp(int32_t pointerIdx, int32_t expectedDisplayId = ADISPLAY_ID_DEFAULT, - int32_t expectedFlags = 0) { - int32_t action = AMOTION_EVENT_ACTION_POINTER_UP - | (pointerIdx << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT); + int32_t expectedFlags = 0) { + int32_t action = AMOTION_EVENT_ACTION_POINTER_UP | + (pointerIdx << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT); consumeEvent(AINPUT_EVENT_TYPE_MOTION, action, expectedDisplayId, expectedFlags); } void consumeMotionUp(int32_t expectedDisplayId = ADISPLAY_ID_DEFAULT, - int32_t expectedFlags = 0) { + int32_t expectedFlags = 0) { consumeEvent(AINPUT_EVENT_TYPE_MOTION, AMOTION_EVENT_ACTION_UP, expectedDisplayId, expectedFlags); } @@ -902,6 +898,11 @@ class FakeWindowHandle : public InputWindowHandle { const std::string& getName() { return mName; } + void setOwnerInfo(int32_t ownerPid, int32_t ownerUid) { + mInfo.ownerPid = ownerPid; + mInfo.ownerUid = ownerUid; + } + private: const std::string mName; std::unique_ptr mInputReceiver; @@ -1261,17 +1262,18 @@ TEST_F(InputDispatcherTest, TransferTouchFocus_OnePointer) { sp application = new FakeApplicationHandle(); // Create a couple of windows - sp firstWindow = new FakeWindowHandle(application, mDispatcher, - "First Window", ADISPLAY_ID_DEFAULT); - sp secondWindow = new FakeWindowHandle(application, mDispatcher, - "Second Window", ADISPLAY_ID_DEFAULT); + sp firstWindow = + new FakeWindowHandle(application, mDispatcher, "First Window", ADISPLAY_ID_DEFAULT); + sp secondWindow = + new FakeWindowHandle(application, mDispatcher, "Second Window", ADISPLAY_ID_DEFAULT); // Add the windows to the dispatcher mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {firstWindow, secondWindow}}}); // Send down to the first window - NotifyMotionArgs downMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_DOWN, - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT); + NotifyMotionArgs downMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_DOWN, AINPUT_SOURCE_TOUCHSCREEN, + ADISPLAY_ID_DEFAULT); mDispatcher->notifyMotion(&downMotionArgs); // Only the first window should get the down event firstWindow->consumeMotionDown(); @@ -1284,8 +1286,9 @@ TEST_F(InputDispatcherTest, TransferTouchFocus_OnePointer) { secondWindow->consumeMotionDown(); // Send up event to the second window - NotifyMotionArgs upMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_UP, - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT); + NotifyMotionArgs upMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_UP, AINPUT_SOURCE_TOUCHSCREEN, + ADISPLAY_ID_DEFAULT); mDispatcher->notifyMotion(&upMotionArgs); // The first window gets no events and the second gets up firstWindow->assertNoEvents(); @@ -1298,26 +1301,29 @@ TEST_F(InputDispatcherTest, TransferTouchFocus_TwoPointerNoSplitTouch) { PointF touchPoint = {10, 10}; // Create a couple of windows - sp firstWindow = new FakeWindowHandle(application, mDispatcher, - "First Window", ADISPLAY_ID_DEFAULT); - sp secondWindow = new FakeWindowHandle(application, mDispatcher, - "Second Window", ADISPLAY_ID_DEFAULT); + sp firstWindow = + new FakeWindowHandle(application, mDispatcher, "First Window", ADISPLAY_ID_DEFAULT); + sp secondWindow = + new FakeWindowHandle(application, mDispatcher, "Second Window", ADISPLAY_ID_DEFAULT); // Add the windows to the dispatcher mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {firstWindow, secondWindow}}}); // Send down to the first window - NotifyMotionArgs downMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_DOWN, - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, {touchPoint}); + NotifyMotionArgs downMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_DOWN, AINPUT_SOURCE_TOUCHSCREEN, + ADISPLAY_ID_DEFAULT, {touchPoint}); mDispatcher->notifyMotion(&downMotionArgs); // Only the first window should get the down event firstWindow->consumeMotionDown(); secondWindow->assertNoEvents(); // Send pointer down to the first window - NotifyMotionArgs pointerDownMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_POINTER_DOWN - | (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT), - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, {touchPoint, touchPoint}); + NotifyMotionArgs pointerDownMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_POINTER_DOWN | + (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT), + AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, + {touchPoint, touchPoint}); mDispatcher->notifyMotion(&pointerDownMotionArgs); // Only the first window should get the pointer down event firstWindow->consumeMotionPointerDown(1); @@ -1331,17 +1337,20 @@ TEST_F(InputDispatcherTest, TransferTouchFocus_TwoPointerNoSplitTouch) { secondWindow->consumeMotionPointerDown(1); // Send pointer up to the second window - NotifyMotionArgs pointerUpMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_POINTER_UP - | (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT), - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, {touchPoint, touchPoint}); + NotifyMotionArgs pointerUpMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_POINTER_UP | + (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT), + AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, + {touchPoint, touchPoint}); mDispatcher->notifyMotion(&pointerUpMotionArgs); // The first window gets nothing and the second gets pointer up firstWindow->assertNoEvents(); secondWindow->consumeMotionPointerUp(1); // Send up event to the second window - NotifyMotionArgs upMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_UP, - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT); + NotifyMotionArgs upMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_UP, AINPUT_SOURCE_TOUCHSCREEN, + ADISPLAY_ID_DEFAULT); mDispatcher->notifyMotion(&upMotionArgs); // The first window gets nothing and the second gets up firstWindow->assertNoEvents(); @@ -1352,15 +1361,15 @@ TEST_F(InputDispatcherTest, TransferTouchFocus_TwoPointersSplitTouch) { sp application = new FakeApplicationHandle(); // Create a non touch modal window that supports split touch - sp firstWindow = new FakeWindowHandle(application, mDispatcher, - "First Window", ADISPLAY_ID_DEFAULT); + sp firstWindow = + new FakeWindowHandle(application, mDispatcher, "First Window", ADISPLAY_ID_DEFAULT); firstWindow->setFrame(Rect(0, 0, 600, 400)); firstWindow->setLayoutParamFlags(InputWindowInfo::FLAG_NOT_TOUCH_MODAL | InputWindowInfo::FLAG_SPLIT_TOUCH); // Create a non touch modal window that supports split touch - sp secondWindow = new FakeWindowHandle(application, mDispatcher, - "Second Window", ADISPLAY_ID_DEFAULT); + sp secondWindow = + new FakeWindowHandle(application, mDispatcher, "Second Window", ADISPLAY_ID_DEFAULT); secondWindow->setFrame(Rect(0, 400, 600, 800)); secondWindow->setLayoutParamFlags(InputWindowInfo::FLAG_NOT_TOUCH_MODAL | InputWindowInfo::FLAG_SPLIT_TOUCH); @@ -1372,17 +1381,20 @@ TEST_F(InputDispatcherTest, TransferTouchFocus_TwoPointersSplitTouch) { PointF pointInSecond = {300, 600}; // Send down to the first window - NotifyMotionArgs firstDownMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_DOWN, - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, {pointInFirst}); + NotifyMotionArgs firstDownMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_DOWN, AINPUT_SOURCE_TOUCHSCREEN, + ADISPLAY_ID_DEFAULT, {pointInFirst}); mDispatcher->notifyMotion(&firstDownMotionArgs); // Only the first window should get the down event firstWindow->consumeMotionDown(); secondWindow->assertNoEvents(); // Send down to the second window - NotifyMotionArgs secondDownMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_POINTER_DOWN - | (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT), - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, {pointInFirst, pointInSecond}); + NotifyMotionArgs secondDownMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_POINTER_DOWN | + (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT), + AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, + {pointInFirst, pointInSecond}); mDispatcher->notifyMotion(&secondDownMotionArgs); // The first window gets a move and the second a down firstWindow->consumeMotionMove(); @@ -1395,17 +1407,20 @@ TEST_F(InputDispatcherTest, TransferTouchFocus_TwoPointersSplitTouch) { secondWindow->consumeMotionPointerDown(1); // Send pointer up to the second window - NotifyMotionArgs pointerUpMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_POINTER_UP - | (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT), - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, {pointInFirst, pointInSecond}); + NotifyMotionArgs pointerUpMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_POINTER_UP | + (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT), + AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT, + {pointInFirst, pointInSecond}); mDispatcher->notifyMotion(&pointerUpMotionArgs); // The first window gets nothing and the second gets pointer up firstWindow->assertNoEvents(); secondWindow->consumeMotionPointerUp(1); // Send up event to the second window - NotifyMotionArgs upMotionArgs = generateMotionArgs(AMOTION_EVENT_ACTION_UP, - AINPUT_SOURCE_TOUCHSCREEN, ADISPLAY_ID_DEFAULT); + NotifyMotionArgs upMotionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_UP, AINPUT_SOURCE_TOUCHSCREEN, + ADISPLAY_ID_DEFAULT); mDispatcher->notifyMotion(&upMotionArgs); // The first window gets nothing and the second gets up firstWindow->assertNoEvents(); @@ -1722,6 +1737,72 @@ TEST_F(InputDispatcherTest, VerifyInputEvent_MotionEvent) { EXPECT_EQ(motionArgs.buttonState, verifiedMotion.buttonState); } +/** + * Launch two windows, with different owners. One window (slipperyExitWindow) has Flag::SLIPPERY, + * and overlaps the other window, slipperyEnterWindow. The window 'slipperyExitWindow' is on top + * of the 'slipperyEnterWindow'. + * + * Inject touch down into the top window. Upon receipt of the DOWN event, move the window in such + * a way so that the touched location is no longer covered by the top window. + * + * Next, inject a MOVE event. Because the top window already moved earlier, this event is now + * positioned over the bottom (slipperyEnterWindow) only. And because the top window had + * Flag::SLIPPERY, this will cause the top window to lose the touch event (it will receive + * ACTION_CANCEL instead), and the bottom window will receive a newly generated gesture (starting + * with ACTION_DOWN). + * Thus, the touch has been transferred from the top window into the bottom window, because the top + * window moved itself away from the touched location and had Flag::SLIPPERY. + * + * Even though the top window moved away from the touched location, it is still obscuring the bottom + * window. It's just not obscuring it at the touched location. That means, FLAG_WINDOW_IS_PARTIALLY_ + * OBSCURED should be set for the MotionEvent that reaches the bottom window. + * + * In this test, we ensure that the event received by the bottom window has + * FLAG_WINDOW_IS_PARTIALLY_OBSCURED. + */ +TEST_F(InputDispatcherTest, SlipperyWindow_SetsFlagPartiallyObscured) { + constexpr int32_t SLIPPERY_PID = INJECTOR_PID + 1; + constexpr int32_t SLIPPERY_UID = INJECTOR_UID + 1; + + sp application = new FakeApplicationHandle(); + mDispatcher->setFocusedApplication(ADISPLAY_ID_DEFAULT, application); + + sp slipperyExitWindow = + new FakeWindowHandle(application, mDispatcher, "Top", ADISPLAY_ID_DEFAULT); + slipperyExitWindow->setLayoutParamFlags(InputWindowInfo::FLAG_NOT_TOUCH_MODAL | + InputWindowInfo::FLAG_SLIPPERY); + // Make sure this one overlaps the bottom window + slipperyExitWindow->setFrame(Rect(25, 25, 75, 75)); + // Change the owner uid/pid of the window so that it is considered to be occluding the bottom + // one. Windows with the same owner are not considered to be occluding each other. + slipperyExitWindow->setOwnerInfo(SLIPPERY_PID, SLIPPERY_UID); + + sp slipperyEnterWindow = + new FakeWindowHandle(application, mDispatcher, "Second", ADISPLAY_ID_DEFAULT); + slipperyExitWindow->setFrame(Rect(0, 0, 100, 100)); + + mDispatcher->setInputWindows( + {{ADISPLAY_ID_DEFAULT, {slipperyExitWindow, slipperyEnterWindow}}}); + + // Use notifyMotion instead of injecting to avoid dealing with injection permissions + NotifyMotionArgs args = generateMotionArgs(AMOTION_EVENT_ACTION_DOWN, AINPUT_SOURCE_TOUCHSCREEN, + ADISPLAY_ID_DEFAULT, {{50, 50}}); + mDispatcher->notifyMotion(&args); + slipperyExitWindow->consumeMotionDown(); + slipperyExitWindow->setFrame(Rect(70, 70, 100, 100)); + mDispatcher->setInputWindows( + {{ADISPLAY_ID_DEFAULT, {slipperyExitWindow, slipperyEnterWindow}}}); + + args = generateMotionArgs(AMOTION_EVENT_ACTION_MOVE, AINPUT_SOURCE_TOUCHSCREEN, + ADISPLAY_ID_DEFAULT, {{51, 51}}); + mDispatcher->notifyMotion(&args); + + slipperyExitWindow->consumeMotionCancel(); + + slipperyEnterWindow->consumeMotionDown(ADISPLAY_ID_DEFAULT, + AMOTION_EVENT_FLAG_WINDOW_IS_PARTIALLY_OBSCURED); +} + class InputDispatcherKeyRepeatTest : public InputDispatcherTest { protected: static constexpr nsecs_t KEY_REPEAT_TIMEOUT = 40 * 1000000; // 40 ms @@ -1950,7 +2031,7 @@ TEST_F(InputDispatcherFocusOnTwoDisplaysTest, MonitorMotionEvent_MultiDisplay) { // Test per-display input monitors for key event. TEST_F(InputDispatcherFocusOnTwoDisplaysTest, MonitorKeyEvent_MultiDisplay) { - //Input monitor per display. + // Input monitor per display. FakeMonitorReceiver monitorInPrimary = FakeMonitorReceiver(mDispatcher, "M_1", ADISPLAY_ID_DEFAULT); FakeMonitorReceiver monitorInSecondary = @@ -1972,11 +2053,11 @@ class InputFilterTest : public InputDispatcherTest { void testNotifyMotion(int32_t displayId, bool expectToBeFiltered) { NotifyMotionArgs motionArgs; - motionArgs = generateMotionArgs( - AMOTION_EVENT_ACTION_DOWN, AINPUT_SOURCE_TOUCHSCREEN, displayId); + motionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_DOWN, AINPUT_SOURCE_TOUCHSCREEN, displayId); mDispatcher->notifyMotion(&motionArgs); - motionArgs = generateMotionArgs( - AMOTION_EVENT_ACTION_UP, AINPUT_SOURCE_TOUCHSCREEN, displayId); + motionArgs = + generateMotionArgs(AMOTION_EVENT_ACTION_UP, AINPUT_SOURCE_TOUCHSCREEN, displayId); mDispatcher->notifyMotion(&motionArgs); ASSERT_TRUE(mDispatcher->waitForIdle()); if (expectToBeFiltered) { From e26f83863167833ac5cf3a0e80112a8cfbe0b03d Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Mon, 16 Mar 2020 22:35:21 +0000 Subject: [PATCH 26/43] EGL: Loader: Enable desktop openGL on Nvidia platforms There is some specific code I reversed from the EGL library that sets the openGL bit in a data struct when getProcAddr is called with eglSentinelNvFrameworks. Change-Id: I9c1eaba65c65dfc9b3e500ad627a492ec6ce2f36 --- opengl/libs/Android.bp | 5 ++++- opengl/libs/EGL/Loader.cpp | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/opengl/libs/Android.bp b/opengl/libs/Android.bp index e8d3684e4e..abe254ab62 100644 --- a/opengl/libs/Android.bp +++ b/opengl/libs/Android.bp @@ -132,7 +132,10 @@ cc_library_static { cc_library_shared { name: "libEGL", - defaults: ["egl_libs_defaults"], + defaults: [ + "egl_libs_defaults", + "nvidia_enhancements_defaults" + ], srcs: [ "EGL/egl_tls.cpp", "EGL/egl_cache.cpp", diff --git a/opengl/libs/EGL/Loader.cpp b/opengl/libs/EGL/Loader.cpp index d66ef2b969..0576a05948 100644 --- a/opengl/libs/EGL/Loader.cpp +++ b/opengl/libs/EGL/Loader.cpp @@ -642,6 +642,11 @@ void Loader::initialize_api(void* dso, egl_connection_t* cnx, uint32_t mask) { ALOGE_IF(!getProcAddress, "can't find eglGetProcAddress() in EGL driver library"); +#ifdef NV_ANDROID_FRAMEWORK_ENHANCEMENTS + // This internally sets a bit in the main Nvidia EGL driver to enable desktop openGL + getProcAddress("eglSentinelForNVFrameworks"); +#endif + egl_t* egl = &cnx->egl; __eglMustCastToProperFunctionPointerType* curr = (__eglMustCastToProperFunctionPointerType*)egl; From c41fa07834bb7a50d8901fd16cff23ddf1e517c3 Mon Sep 17 00:00:00 2001 From: Aaron Kling Date: Fri, 25 Mar 2022 17:05:00 -0500 Subject: [PATCH 27/43] EGL: nvidia: Don't use extension map The nvidia egl implementation exposes desktop opengl, which has far too many extentions for the map to handle. Change-Id: Id925c66fdf98108af126e9e02a38c696157576f2 --- opengl/libs/EGL/egl_platform_entries.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/opengl/libs/EGL/egl_platform_entries.cpp b/opengl/libs/EGL/egl_platform_entries.cpp index aa24e8ee68..b64d200366 100644 --- a/opengl/libs/EGL/egl_platform_entries.cpp +++ b/opengl/libs/EGL/egl_platform_entries.cpp @@ -1218,6 +1218,10 @@ __eglMustCastToProperFunctionPointerType eglGetProcAddressImpl(const char *procn addr = findBuiltinWrapper(procname); if (addr) return addr; +#ifdef NV_ANDROID_FRAMEWORK_ENHANCEMENTS + if (gEGLImpl.dso && gEGLImpl.egl.eglGetProcAddress) + addr = gEGLImpl.egl.eglGetProcAddress(procname); +#else // this protects accesses to sGLExtensionMap, sGLExtensionSlot, and sGLExtensionSlotMap pthread_mutex_lock(&sExtensionMapMutex); @@ -1305,6 +1309,7 @@ __eglMustCastToProperFunctionPointerType eglGetProcAddressImpl(const char *procn } pthread_mutex_unlock(&sExtensionMapMutex); +#endif return addr; } From 06852ffa04d8487520eedb86daa7f77ebbbf58d1 Mon Sep 17 00:00:00 2001 From: Aaron Kling Date: Fri, 25 Mar 2022 17:14:26 -0500 Subject: [PATCH 28/43] EGL: nvidia: For desktop gl contexts, use gles v2 hooks list Change-Id: Icd1e8e0c46c72bd87adb35faa2952d56b9ff2541 --- opengl/libs/EGL/egl_platform_entries.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opengl/libs/EGL/egl_platform_entries.cpp b/opengl/libs/EGL/egl_platform_entries.cpp index b64d200366..b68a7cd8a3 100644 --- a/opengl/libs/EGL/egl_platform_entries.cpp +++ b/opengl/libs/EGL/egl_platform_entries.cpp @@ -972,6 +972,12 @@ EGLContext eglCreateContextImpl(EGLDisplay dpy, EGLConfig config, if (context != EGL_NO_CONTEXT) { // figure out if it's a GLESv1 or GLESv2 int version = egl_connection_t::GLESv1_INDEX; + +#ifdef NV_ANDROID_FRAMEWORK_ENHANCEMENTS + if (cnx->egl.eglQueryAPI() == EGL_OPENGL_API) + version = egl_connection_t::GLESv2_INDEX; +#endif + if (attrib_list) { while (*attrib_list != EGL_NONE) { GLint attr = *attrib_list++; From 101d8a1f390bee3bcb12f697f1b3f554fadeb156 Mon Sep 17 00:00:00 2001 From: Aaron Kling Date: Fri, 20 Apr 2018 22:12:29 -0500 Subject: [PATCH 29/43] Fix eglMakeCurrent crash when in opengl contexts Nvidia shield devices support using egl to switch to a full desktop opengl context. In opengl 3.0+, GL_EXTENSIONS have to be retrieved with glGetStringi and is invalid for glGetString. Thus, eglMakecurrent can crash with a NPE if this case is not handled. The logic here is building a wrapper for glGetStringi, thus the error can be ignored. This patch allows GL_EXT_debug_marker to still pushed into the tokenized extension string used by the glGetStringi wrapper. Change-Id: I9c599e10c62aabf684bde4e81719aa248327ac80 --- opengl/libs/EGL/egl_object.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/opengl/libs/EGL/egl_object.cpp b/opengl/libs/EGL/egl_object.cpp index ff4fe2dd9c..a17b899eed 100644 --- a/opengl/libs/EGL/egl_object.cpp +++ b/opengl/libs/EGL/egl_object.cpp @@ -302,6 +302,14 @@ void egl_context_t::onMakeCurrent(EGLSurface draw, EGLSurface read) { // call the implementation's glGetString(GL_EXTENSIONS) const char* exts = (const char *)gEGLImpl.hooks[version]->gl.glGetString(GL_EXTENSIONS); +#ifdef NV_ANDROID_FRAMEWORK_ENHANCEMENTS + // In an opengl 3.0+ context, GL_EXTENSIONS is not valid for glGetString + if (exts == NULL) { + gEGLImpl.hooks[version]->gl.glGetError(); + exts = ""; + } +#endif + // If this context is sharing with another context, and the other context was reset // e.g. due to robustness failure, this context might also be reset and glGetString can // return NULL. From 4415ef3170127ac2269298f11cb3522298f6a106 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Mon, 6 Jun 2022 15:16:06 -0700 Subject: [PATCH 30/43] RESTRICT AUTOMERGE SurfaceFlinger: fix a potential race condition in stealReceiveChannel Add a mutex to prevent a potential race condition. Bug: 232541124 Test: See bug for details Change-Id: Ia338f124c786bf12d6adba10a67b9048fe9c34a5 (cherry picked from commit a820057ae00dba322b10d47b3711b04519324690) Merged-In: Ia338f124c786bf12d6adba10a67b9048fe9c34a5 --- services/surfaceflinger/Scheduler/EventThread.cpp | 5 +++++ services/surfaceflinger/Scheduler/EventThread.h | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index cee36a121f..cf57664cd5 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -138,6 +138,11 @@ void EventThreadConnection::onFirstRef() { } status_t EventThreadConnection::stealReceiveChannel(gui::BitTube* outChannel) { + std::scoped_lock lock(mLock); + if (mChannel.initCheck() != NO_ERROR) { + return NAME_NOT_FOUND; + } + outChannel->setReceiveFd(mChannel.moveReceiveFd()); return NO_ERROR; } diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index 64acbd72d0..831bf7baa9 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -98,7 +98,8 @@ class EventThreadConnection : public BnDisplayEventConnection { private: virtual void onFirstRef(); EventThread* const mEventThread; - gui::BitTube mChannel; + std::mutex mLock; + gui::BitTube mChannel GUARDED_BY(mLock); }; class EventThread { From 3a3c097fb70c513463fa4b249dc1ed1fe1b3ae85 Mon Sep 17 00:00:00 2001 From: Chris Ye Date: Sun, 10 May 2020 15:16:04 -0700 Subject: [PATCH 31/43] Change InputWindowInfo::isTrustedOverlay() to be permission and flag based. Add private flag to WindowManager.LayoutParams. If the flag is set, check if caller has INTERNAL_SYSTEM_WINDOW permission. Bug: 155781676 Bug: 196389741 Test: atest WindowManagerServiceTests Change-Id: I58cf9f38c496e0ae8b2193dca45c0805e831bc9e Merged-In: I58cf9f38c496e0ae8b2193dca45c0805e831bc9e (cherry picked from commit 39bc6117dda8cf5f6f43846f18fc0fed87692efa) Merged-In: I58cf9f38c496e0ae8b2193dca45c0805e831bc9e --- include/input/InputWindow.h | 16 ++++++++-------- libs/input/InputWindow.cpp | 16 ++-------------- .../inputflinger/dispatcher/InputDispatcher.cpp | 2 +- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/include/input/InputWindow.h b/include/input/InputWindow.h index 2dac5b62a7..271fdb3a2f 100644 --- a/include/input/InputWindow.h +++ b/include/input/InputWindow.h @@ -108,7 +108,6 @@ struct InputWindowInfo { TYPE_ACCESSIBILITY_OVERLAY = FIRST_SYSTEM_WINDOW + 32, TYPE_DOCK_DIVIDER = FIRST_SYSTEM_WINDOW + 34, TYPE_NOTIFICATION_SHADE = FIRST_SYSTEM_WINDOW + 40, - TYPE_TRUSTED_APPLICATION_OVERLAY = FIRST_SYSTEM_WINDOW + 42, LAST_SYSTEM_WINDOW = 2999, }; @@ -163,6 +162,12 @@ struct InputWindowInfo { bool hasFocus = false; bool hasWallpaper = false; bool paused = false; + /* This flag is set when the window is of a trusted type that is allowed to silently + * overlay other windows for the purpose of implementing the secure views feature. + * Trusted overlays, such as IME windows, can partly obscure other windows without causing + * motion events to be delivered to them with AMOTION_EVENT_FLAG_WINDOW_IS_OBSCURED. + */ + bool trustedOverlay = false; int32_t ownerPid = -1; int32_t ownerUid = -1; int32_t inputFeatures = 0; @@ -175,20 +180,15 @@ struct InputWindowInfo { void addTouchableRegion(const Rect& region); bool touchableRegionContainsPoint(int32_t x, int32_t y) const; - bool frameContainsPoint(int32_t x, int32_t y) const; - /* Returns true if the window is of a trusted type that is allowed to silently - * overlay other windows for the purpose of implementing the secure views feature. - * Trusted overlays, such as IME windows, can partly obscure other windows without causing - * motion events to be delivered to them with AMOTION_EVENT_FLAG_WINDOW_IS_OBSCURED. - */ - bool isTrustedOverlay() const; + bool frameContainsPoint(int32_t x, int32_t y) const; bool supportsSplitTouch() const; bool overlaps(const InputWindowInfo* other) const; status_t write(Parcel& output) const; + static InputWindowInfo read(const Parcel& from); }; diff --git a/libs/input/InputWindow.cpp b/libs/input/InputWindow.cpp index 85a2015e43..301c3f56f2 100644 --- a/libs/input/InputWindow.cpp +++ b/libs/input/InputWindow.cpp @@ -42,20 +42,6 @@ bool InputWindowInfo::frameContainsPoint(int32_t x, int32_t y) const { && y >= frameTop && y < frameBottom; } -// TODO(b/155781676): Remove and replace call points with trustedOverlay when that is ready. -bool InputWindowInfo::isTrustedOverlay() const { - return layoutParamsType == TYPE_INPUT_METHOD || layoutParamsType == TYPE_INPUT_METHOD_DIALOG || - layoutParamsType == TYPE_MAGNIFICATION_OVERLAY || layoutParamsType == TYPE_STATUS_BAR || - layoutParamsType == TYPE_NOTIFICATION_SHADE || - layoutParamsType == TYPE_NAVIGATION_BAR || - layoutParamsType == TYPE_NAVIGATION_BAR_PANEL || - layoutParamsType == TYPE_SECURE_SYSTEM_OVERLAY || - layoutParamsType == TYPE_DOCK_DIVIDER || - layoutParamsType == TYPE_ACCESSIBILITY_OVERLAY || - layoutParamsType == TYPE_INPUT_CONSUMER || - layoutParamsType == TYPE_TRUSTED_APPLICATION_OVERLAY; -} - bool InputWindowInfo::supportsSplitTouch() const { return layoutParamsFlags & FLAG_SPLIT_TOUCH; } @@ -92,6 +78,7 @@ status_t InputWindowInfo::write(Parcel& output) const { output.writeBool(hasFocus); output.writeBool(hasWallpaper); output.writeBool(paused); + output.writeBool(trustedOverlay); output.writeInt32(ownerPid); output.writeInt32(ownerUid); output.writeInt32(inputFeatures); @@ -130,6 +117,7 @@ InputWindowInfo InputWindowInfo::read(const Parcel& from) { ret.hasFocus = from.readBool(); ret.hasWallpaper = from.readBool(); ret.paused = from.readBool(); + ret.trustedOverlay = from.readBool(); ret.ownerPid = from.readInt32(); ret.ownerUid = from.readInt32(); ret.inputFeatures = from.readInt32(); diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 8907b3576d..d2183c5c34 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -2094,7 +2094,7 @@ static bool canBeObscuredBy(const sp& windowHandle, // If ownerPid is the same we don't generate occlusion events as there // is no in-process security boundary. return false; - } else if (otherInfo->isTrustedOverlay()) { + } else if (otherInfo->trustedOverlay) { return false; } else if (otherInfo->displayId != info->displayId) { return false; From 2495fc8c2eacc60e61b81f014e546adeab6c9bf2 Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Tue, 29 Jun 2021 15:42:56 -0700 Subject: [PATCH 32/43] Add mechanism for a task's windows to be trusted overlays (SF) - Add a layer state to indicate that this layer and its children in the hierarchy are trusted. This can only be set by callers holding ACCESS_SURFACE_FLINGER, and will be used for the PIP task layer to indicate that activities in PIP are trusted (as they are controlled only by the user and SystemUI) Bug: 191529039 Bug: 196389741 Test: TBD Change-Id: Id92ccb087bd0d8dbaeeef3ba50b67fe015e53db8 Merged-In: Id92ccb087bd0d8dbaeeef3ba50b67fe015e53db8 (cherry picked from commit 7605fb4273cfdf922a041f201dbcc1e10fae1fe2) Merged-In: Id92ccb087bd0d8dbaeeef3ba50b67fe015e53db8 --- cmds/surfacereplayer/proto/src/trace.proto | 5 ++++ libs/gui/LayerState.cpp | 8 ++++++ libs/gui/SurfaceComposerClient.cpp | 13 ++++++++++ libs/gui/include/gui/LayerState.h | 8 +++++- libs/gui/include/gui/SurfaceComposerClient.h | 3 +++ services/surfaceflinger/Layer.cpp | 22 ++++++++++++++++ services/surfaceflinger/Layer.h | 5 ++++ services/surfaceflinger/SurfaceFlinger.cpp | 9 +++++++ .../surfaceflinger/SurfaceInterceptor.cpp | 11 ++++++++ services/surfaceflinger/SurfaceInterceptor.h | 1 + .../layerproto/LayerProtoParser.cpp | 2 ++ .../include/layerproto/LayerProtoParser.h | 1 + .../surfaceflinger/layerproto/layers.proto | 2 ++ .../tests/SurfaceInterceptor_test.cpp | 26 +++++++++++++++++++ 14 files changed, 115 insertions(+), 1 deletion(-) diff --git a/cmds/surfacereplayer/proto/src/trace.proto b/cmds/surfacereplayer/proto/src/trace.proto index b57409867f..372cecd63c 100644 --- a/cmds/surfacereplayer/proto/src/trace.proto +++ b/cmds/surfacereplayer/proto/src/trace.proto @@ -53,6 +53,7 @@ message SurfaceChange { ReparentChildrenChange reparent_children = 20; BackgroundBlurRadiusChange background_blur_radius = 21; ShadowRadiusChange shadow_radius = 22; + TrustedOverlayChange trusted_overlay = 23; } } @@ -208,4 +209,8 @@ message DetachChildrenChange { message ShadowRadiusChange { required float radius = 1; +} + +message TrustedOverlayChange { + required float is_trusted_overlay = 1; } \ No newline at end of file diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index e43446ac8c..a897d1025f 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -117,6 +117,8 @@ status_t layer_state_t::write(Parcel& output) const output.writeFloat(frameRate); output.writeByte(frameRateCompatibility); output.writeUint32(fixedTransformHint); + output.writeBool(isTrustedOverlay); + return NO_ERROR; } @@ -200,6 +202,8 @@ status_t layer_state_t::read(const Parcel& input) frameRate = input.readFloat(); frameRateCompatibility = input.readByte(); fixedTransformHint = static_cast(input.readUint32()); + isTrustedOverlay = input.readBool(); + return NO_ERROR; } @@ -439,6 +443,10 @@ void layer_state_t::merge(const layer_state_t& other) { what |= eFixedTransformHintChanged; fixedTransformHint = other.fixedTransformHint; } + if (other.what & eTrustedOverlayChanged) { + what |= eTrustedOverlayChanged; + isTrustedOverlay = other.isTrustedOverlay; + } if ((other.what & what) != other.what) { ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating? " "other.what=0x%" PRIu64 " what=0x%" PRIu64, diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 83bc06997a..78d932cc81 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1484,6 +1484,19 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setFixed return *this; } +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setTrustedOverlay( + const sp& sc, bool isTrustedOverlay) { + layer_state_t* s = getLayerState(sc); + if (!s) { + mStatus = BAD_INDEX; + return *this; + } + + s->what |= layer_state_t::eTrustedOverlayChanged; + s->isTrustedOverlay = isTrustedOverlay; + return *this; +} + // --------------------------------------------------------------------------- DisplayState& SurfaceComposerClient::Transaction::getDisplayState(const sp& token) { diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index e60f6777ae..a77e4b0462 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -105,6 +105,7 @@ struct layer_state_t { eBackgroundBlurRadiusChanged = 0x80'00000000, eProducerDisconnect = 0x100'00000000, eFixedTransformHintChanged = 0x200'00000000, + eTrustedOverlayChanged = 0x400'00000000, }; layer_state_t() @@ -139,7 +140,8 @@ struct layer_state_t { frameRateSelectionPriority(-1), frameRate(0.0f), frameRateCompatibility(ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT), - fixedTransformHint(ui::Transform::ROT_INVALID) { + fixedTransformHint(ui::Transform::ROT_INVALID), + isTrustedOverlay(false) { matrix.dsdx = matrix.dtdy = 1.0f; matrix.dsdy = matrix.dtdx = 0.0f; hdrMetadata.validTypes = 0; @@ -237,6 +239,10 @@ struct layer_state_t { // a buffer of a different size. -1 means the transform hint is not set, // otherwise the value will be a valid ui::Rotation. ui::Transform::RotationFlags fixedTransformHint; + + // An inherited state that indicates that this surface control and its children + // should be trusted for input occlusion detection purposes + bool isTrustedOverlay; }; struct ComposerState { diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index adcb8982a0..eebd9ca774 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -529,6 +529,9 @@ class SurfaceComposerClient : public RefBase // a buffer of a different size. Transaction& setFixedTransformHint(const sp& sc, int32_t transformHint); + // Sets that this surface control and its children are trusted overlays for input + Transaction& setTrustedOverlay(const sp& sc, bool isTrustedOverlay); + status_t setDisplaySurface(const sp& token, const sp& bufferProducer); diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 03903f6d07..ceea79f980 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -1142,6 +1142,23 @@ bool Layer::setRelativeLayer(const sp& relativeToHandle, int32_t relati return true; } +bool Layer::setTrustedOverlay(bool isTrustedOverlay) { + if (mCurrentState.isTrustedOverlay == isTrustedOverlay) return false; + mCurrentState.isTrustedOverlay = isTrustedOverlay; + mCurrentState.modified = true; + mCurrentState.inputInfoChanged = true; + setTransactionFlags(eTransactionNeeded); + return true; +} + +bool Layer::isTrustedOverlay() const { + if (getDrawingState().isTrustedOverlay) { + return true; + } + const auto& p = mDrawingParent.promote(); + return (p != nullptr) && p->isTrustedOverlay(); +} + bool Layer::setSize(uint32_t w, uint32_t h) { if (mCurrentState.requested_legacy.w == w && mCurrentState.requested_legacy.h == h) return false; @@ -2245,6 +2262,7 @@ void Layer::writeToProtoDrawingState(LayerProto* layerInfo, uint32_t traceFlags, layerInfo->set_effective_scaling_mode(getEffectiveScalingMode()); layerInfo->set_corner_radius(getRoundedCornerState().radius); + layerInfo->set_is_trusted_overlay(isTrustedOverlay()); LayerProtoHelper::writeToProto(transform, layerInfo->mutable_transform()); LayerProtoHelper::writePositionToProto(transform.tx(), transform.ty(), [&]() { return layerInfo->mutable_position(); }); @@ -2435,6 +2453,10 @@ InputWindowInfo Layer::fillInputInfo() { info.touchableRegion = info.touchableRegion.intersect(Rect{cropLayer->mScreenBounds}); } + // Inherit the trusted state from the parent hierarchy, but don't clobber the trusted state + // if it was set by WM for a known system overlay + info.trustedOverlay = info.trustedOverlay || isTrustedOverlay(); + // If the layer is a clone, we need to crop the input region to cloned root to prevent // touches from going outside the cloned area. if (isClone()) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 2c90c92f6c..a12bae42fc 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -279,6 +279,9 @@ class Layer : public virtual RefBase, compositionengine::LayerFE { // a buffer of a different size. ui::Transform::ROT_INVALID means the // a fixed transform hint is not set. ui::Transform::RotationFlags fixedTransformHint; + + // Whether or not this layer is a trusted overlay for input + bool isTrustedOverlay; }; explicit Layer(const LayerCreationArgs& args); @@ -356,6 +359,7 @@ class Layer : public virtual RefBase, compositionengine::LayerFE { // is specified in pixels. virtual bool setBackgroundBlurRadius(int backgroundBlurRadius); virtual bool setTransparentRegionHint(const Region& transparent); + virtual bool setTrustedOverlay(bool); virtual bool setFlags(uint8_t flags, uint8_t mask); virtual bool setLayerStack(uint32_t layerStack); virtual uint32_t getLayerStack() const; @@ -1064,6 +1068,7 @@ class Layer : public virtual RefBase, compositionengine::LayerFE { const std::vector& layersInTree); void updateTreeHasFrameRateVote(); + bool isTrustedOverlay() const; // Cached properties computed from drawing state // Effective transform taking into account parent transforms and any parent scaling. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 4a60d5c6f2..4947210929 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3858,6 +3858,15 @@ uint32_t SurfaceFlinger::setClientStateLocked( flags |= eTraversalNeeded | eTransformHintUpdateNeeded; } } + if (what & layer_state_t::eTrustedOverlayChanged) { + if (privileged) { + if (layer->setTrustedOverlay(s.isTrustedOverlay)) { + flags |= eTraversalNeeded; + } + } else { + ALOGE("Attempt to set trusted overlay without permission ACCESS_SURFACE_FLINGER"); + } + } // This has to happen after we reparent children because when we reparent to null we remove // child layers from current state and remove its relative z. If the children are reparented in // the same transaction, then we have to make sure we reparent the children first so we do not diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp index 80102bdbb6..0555acff43 100644 --- a/services/surfaceflinger/SurfaceInterceptor.cpp +++ b/services/surfaceflinger/SurfaceInterceptor.cpp @@ -130,6 +130,7 @@ void SurfaceInterceptor::addInitialSurfaceStateLocked(Increment* increment, getLayerIdFromWeakRef(layer->mCurrentState.zOrderRelativeOf), layer->mCurrentState.z); addShadowRadiusLocked(transaction, layerId, layer->mCurrentState.shadowRadius); + addTrustedOverlayLocked(transaction, layerId, layer->mDrawingState.isTrustedOverlay); } void SurfaceInterceptor::addInitialDisplayStateLocked(Increment* increment, @@ -388,6 +389,13 @@ void SurfaceInterceptor::addShadowRadiusLocked(Transaction* transaction, int32_t overrideChange->set_radius(shadowRadius); } +void SurfaceInterceptor::addTrustedOverlayLocked(Transaction* transaction, int32_t layerId, + bool isTrustedOverlay) { + SurfaceChange* change(createSurfaceChangeLocked(transaction, layerId)); + TrustedOverlayChange* overrideChange(change->mutable_trusted_overlay()); + overrideChange->set_is_trusted_overlay(isTrustedOverlay); +} + void SurfaceInterceptor::addSurfaceChangesLocked(Transaction* transaction, const layer_state_t& state) { @@ -467,6 +475,9 @@ void SurfaceInterceptor::addSurfaceChangesLocked(Transaction* transaction, if (state.what & layer_state_t::eShadowRadiusChanged) { addShadowRadiusLocked(transaction, layerId, state.shadowRadius); } + if (state.what & layer_state_t::eTrustedOverlayChanged) { + addTrustedOverlayLocked(transaction, layerId, state.isTrustedOverlay); + } } void SurfaceInterceptor::addDisplayChangesLocked(Transaction* transaction, diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h index 896bdcc259..6b98b44420 100644 --- a/services/surfaceflinger/SurfaceInterceptor.h +++ b/services/surfaceflinger/SurfaceInterceptor.h @@ -168,6 +168,7 @@ class SurfaceInterceptor final : public android::SurfaceInterceptor { void addRelativeParentLocked(Transaction* transaction, int32_t layerId, int32_t parentId, int z); void addShadowRadiusLocked(Transaction* transaction, int32_t layerId, float shadowRadius); + void addTrustedOverlayLocked(Transaction* transaction, int32_t layerId, bool isTrustedOverlay); // Add display transactions to the trace DisplayChange* createDisplayChangeLocked(Transaction* transaction, int32_t sequenceId); diff --git a/services/surfaceflinger/layerproto/LayerProtoParser.cpp b/services/surfaceflinger/layerproto/LayerProtoParser.cpp index 8fce0c9bf3..2eceba848d 100644 --- a/services/surfaceflinger/layerproto/LayerProtoParser.cpp +++ b/services/surfaceflinger/layerproto/LayerProtoParser.cpp @@ -105,6 +105,7 @@ LayerProtoParser::Layer LayerProtoParser::generateLayer(const LayerProto& layerP layer.queuedFrames = layerProto.queued_frames(); layer.refreshPending = layerProto.refresh_pending(); layer.isProtected = layerProto.is_protected(); + layer.isTrustedOverlay = layerProto.is_trusted_overlay(); layer.cornerRadius = layerProto.corner_radius(); layer.backgroundBlurRadius = layerProto.background_blur_radius(); for (const auto& entry : layerProto.metadata()) { @@ -288,6 +289,7 @@ std::string LayerProtoParser::Layer::to_string() const { StringAppendF(&result, "crop=%s, ", crop.to_string().c_str()); StringAppendF(&result, "cornerRadius=%f, ", cornerRadius); StringAppendF(&result, "isProtected=%1d, ", isProtected); + StringAppendF(&result, "isTrustedOverlay=%1d, ", isTrustedOverlay); StringAppendF(&result, "isOpaque=%1d, invalidate=%1d, ", isOpaque, invalidate); StringAppendF(&result, "dataspace=%s, ", dataspace.c_str()); StringAppendF(&result, "defaultPixelFormat=%s, ", pixelFormat.c_str()); diff --git a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h index 52b916555f..67bbd76934 100644 --- a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h +++ b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h @@ -109,6 +109,7 @@ class LayerProtoParser { int32_t queuedFrames; bool refreshPending; bool isProtected; + bool isTrustedOverlay; float cornerRadius; int backgroundBlurRadius; LayerMetadata metadata; diff --git a/services/surfaceflinger/layerproto/layers.proto b/services/surfaceflinger/layerproto/layers.proto index 7f1f542e4b..780b066000 100644 --- a/services/surfaceflinger/layerproto/layers.proto +++ b/services/surfaceflinger/layerproto/layers.proto @@ -123,6 +123,8 @@ message LayerProto { bool is_relative_of = 51; // Layer's background blur radius in pixels. int32 background_blur_radius = 52; + + bool is_trusted_overlay = 53; } message PositionProto { diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp index 8d97f275ba..5bafbd8975 100644 --- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp +++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp @@ -195,6 +195,7 @@ class SurfaceInterceptorTest : public ::testing::Test { bool detachChildrenUpdateFound(const SurfaceChange& change, bool found); bool reparentChildrenUpdateFound(const SurfaceChange& change, bool found); bool shadowRadiusUpdateFound(const SurfaceChange& change, bool found); + bool trustedOverlayUpdateFound(const SurfaceChange& change, bool found); bool surfaceUpdateFound(const Trace& trace, SurfaceChange::SurfaceChangeCase changeCase); // Find all of the updates in the single trace @@ -233,6 +234,7 @@ class SurfaceInterceptorTest : public ::testing::Test { void detachChildrenUpdate(Transaction&); void reparentChildrenUpdate(Transaction&); void shadowRadiusUpdate(Transaction&); + void trustedOverlayUpdate(Transaction&); void surfaceCreation(Transaction&); void displayCreation(Transaction&); void displayDeletion(Transaction&); @@ -421,6 +423,10 @@ void SurfaceInterceptorTest::shadowRadiusUpdate(Transaction& t) { t.setShadowRadius(mBGSurfaceControl, SHADOW_RADIUS_UPDATE); } +void SurfaceInterceptorTest::trustedOverlayUpdate(Transaction& t) { + t.setTrustedOverlay(mBGSurfaceControl, true); +} + void SurfaceInterceptorTest::displayCreation(Transaction&) { sp testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true); SurfaceComposerClient::destroyDisplay(testDisplay); @@ -452,6 +458,7 @@ void SurfaceInterceptorTest::runAllUpdates() { runInTransaction(&SurfaceInterceptorTest::detachChildrenUpdate); runInTransaction(&SurfaceInterceptorTest::relativeParentUpdate); runInTransaction(&SurfaceInterceptorTest::shadowRadiusUpdate); + runInTransaction(&SurfaceInterceptorTest::trustedOverlayUpdate); } void SurfaceInterceptorTest::surfaceCreation(Transaction&) { @@ -695,6 +702,17 @@ bool SurfaceInterceptorTest::shadowRadiusUpdateFound(const SurfaceChange& change return foundShadowRadius; } +bool SurfaceInterceptorTest::trustedOverlayUpdateFound(const SurfaceChange& change, + bool foundTrustedOverlay) { + bool hasTrustedOverlay(change.trusted_overlay().is_trusted_overlay()); + if (hasTrustedOverlay && !foundTrustedOverlay) { + foundTrustedOverlay = true; + } else if (hasTrustedOverlay && foundTrustedOverlay) { + []() { FAIL(); }(); + } + return foundTrustedOverlay; +} + bool SurfaceInterceptorTest::surfaceUpdateFound(const Trace& trace, SurfaceChange::SurfaceChangeCase changeCase) { bool foundUpdate = false; @@ -764,6 +782,9 @@ bool SurfaceInterceptorTest::surfaceUpdateFound(const Trace& trace, case SurfaceChange::SurfaceChangeCase::kShadowRadius: foundUpdate = shadowRadiusUpdateFound(change, foundUpdate); break; + case SurfaceChange::SurfaceChangeCase::kTrustedOverlay: + foundUpdate = trustedOverlayUpdateFound(change, foundUpdate); + break; case SurfaceChange::SurfaceChangeCase::SURFACECHANGE_NOT_SET: break; } @@ -976,6 +997,11 @@ TEST_F(SurfaceInterceptorTest, InterceptShadowRadiusUpdateWorks) { SurfaceChange::SurfaceChangeCase::kShadowRadius); } +TEST_F(SurfaceInterceptorTest, InterceptTrustedOverlayUpdateWorks) { + captureTest(&SurfaceInterceptorTest::trustedOverlayUpdate, + SurfaceChange::SurfaceChangeCase::kTrustedOverlay); +} + TEST_F(SurfaceInterceptorTest, InterceptAllUpdatesWorks) { captureTest(&SurfaceInterceptorTest::runAllUpdates, &SurfaceInterceptorTest::assertAllUpdatesFound); From ed9f09b785d524586be9d405c7ad5527345e2cfd Mon Sep 17 00:00:00 2001 From: Robin Lee Date: Fri, 10 Sep 2021 12:03:42 +0000 Subject: [PATCH 33/43] DO NOT MERGE Initialize DrawingState::trustedOverlay to false in constructor To avoid it being initialised to true randomly. Bug: 199483370 Bug: 196389741 Change-Id: I75be2b1d305e22f8a71532b9f5b8ea6c469baaaa Merged-In: I75be2b1d305e22f8a71532b9f5b8ea6c469baaaa (cherry picked from commit 41f48c7b10132c94822ab109de99978ee65cc743) Merged-In: I75be2b1d305e22f8a71532b9f5b8ea6c469baaaa --- services/surfaceflinger/Layer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index ceea79f980..4e3ef2468a 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -118,6 +118,7 @@ Layer::Layer(const LayerCreationArgs& args) mCurrentState.shadowRadius = 0.f; mCurrentState.treeHasFrameRateVote = false; mCurrentState.fixedTransformHint = ui::Transform::ROT_INVALID; + mCurrentState.isTrustedOverlay = false; if (args.flags & ISurfaceComposerClient::eNoColorFill) { // Set an invalid color so there is no color fill. From 5be28bb5a71c51737f32e85372b5d95ec62c6edc Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Thu, 27 Jan 2022 16:20:46 +0000 Subject: [PATCH 34/43] SurfaceControl: Add setDropInputMode api Introduces an API to drop input events on this SurfaceControl. This policy will be inherited by its children. The caller must hold the ACCESS_SURFACE_FLINGER permission. Options include: ALL: SurfaceControl and its children will not receive any input regardless of whether it has a valid input channel. These policies are used to enable features that allow for a less trusted interaction model between apps. See the bug for more details. Note: this backport does not include the OBSCURED option since its not needed for the security fix. Test: atest libgui_test InputDispatcherDropInputFeatureTest Bug: 197296414 Merged-In: I443741d5ab51a45d37fb865f11c433c436d96c1e Change-Id: I443741d5ab51a45d37fb865f11c433c436d96c1e (cherry picked from commit 122c4d2da0405be75ae8c249e19ba692722c6e13) Merged-In: I443741d5ab51a45d37fb865f11c433c436d96c1e --- include/input/InputWindow.h | 1 + libs/gui/Android.bp | 9 +++++ libs/gui/LayerState.cpp | 9 ++++- libs/gui/SurfaceComposerClient.cpp | 15 ++++++++ libs/gui/android/gui/DropInputMode.aidl | 40 ++++++++++++++++++++ libs/gui/include/gui/LayerState.h | 8 +++- libs/gui/include/gui/SurfaceComposerClient.h | 3 +- services/surfaceflinger/Layer.cpp | 11 ++++++ services/surfaceflinger/Layer.h | 5 +++ services/surfaceflinger/SurfaceFlinger.cpp | 9 +++++ 10 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 libs/gui/android/gui/DropInputMode.aidl diff --git a/include/input/InputWindow.h b/include/input/InputWindow.h index 271fdb3a2f..bbf793e62b 100644 --- a/include/input/InputWindow.h +++ b/include/input/InputWindow.h @@ -115,6 +115,7 @@ struct InputWindowInfo { INPUT_FEATURE_DISABLE_TOUCH_PAD_GESTURES = 0x00000001, INPUT_FEATURE_NO_INPUT_CHANNEL = 0x00000002, INPUT_FEATURE_DISABLE_USER_ACTIVITY = 0x00000004, + INPUT_FEATURE_DROP_INPUT = 0x00000008, }; /* These values are filled in by the WM and passed through SurfaceFlinger diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 4a4510e047..6ec1f471c3 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -30,6 +30,14 @@ cc_library_headers { min_sdk_version: "29", } +// AIDL files that should be exposed to java +filegroup { + name: "guiconstants_aidl", + srcs: [ + "android/gui/DropInputMode.aidl", + ], +} + cc_library_shared { name: "libgui", vendor_available: false, @@ -41,6 +49,7 @@ cc_library_shared { defaults: ["libgui_bufferqueue-defaults"], srcs: [ + ":guiconstants_aidl", ":framework_native_aidl", ":libgui_bufferqueue_sources", diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index a897d1025f..dfcef8fe7a 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -118,7 +118,7 @@ status_t layer_state_t::write(Parcel& output) const output.writeByte(frameRateCompatibility); output.writeUint32(fixedTransformHint); output.writeBool(isTrustedOverlay); - + output.writeUint32(static_cast(dropInputMode)); return NO_ERROR; } @@ -204,6 +204,9 @@ status_t layer_state_t::read(const Parcel& input) fixedTransformHint = static_cast(input.readUint32()); isTrustedOverlay = input.readBool(); + uint32_t mode; + mode = input.readUint32(); + dropInputMode = static_cast(mode); return NO_ERROR; } @@ -447,6 +450,10 @@ void layer_state_t::merge(const layer_state_t& other) { what |= eTrustedOverlayChanged; isTrustedOverlay = other.isTrustedOverlay; } + if (other.what & eDropInputModeChanged) { + what |= eDropInputModeChanged; + dropInputMode = other.dropInputMode; + } if ((other.what & what) != other.what) { ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating? " "other.what=0x%" PRIu64 " what=0x%" PRIu64, diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 78d932cc81..d2c96218f4 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1497,6 +1497,21 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setTrust return *this; } +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setDropInputMode( + const sp& sc, gui::DropInputMode mode) { + layer_state_t* s = getLayerState(sc); + if (!s) { + mStatus = BAD_INDEX; + return *this; + } + + s->what |= layer_state_t::eDropInputModeChanged; + s->dropInputMode = mode; + + registerSurfaceControlForCallback(sc); + return *this; +} + // --------------------------------------------------------------------------- DisplayState& SurfaceComposerClient::Transaction::getDisplayState(const sp& token) { diff --git a/libs/gui/android/gui/DropInputMode.aidl b/libs/gui/android/gui/DropInputMode.aidl new file mode 100644 index 0000000000..248a0318b7 --- /dev/null +++ b/libs/gui/android/gui/DropInputMode.aidl @@ -0,0 +1,40 @@ +/** + * Copyright (c) 2022, 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. + */ + +package android.gui; + + +/** + * Input event drop modes: Input event drop options for windows and its children. + * + * @hide + */ +@Backing(type="int") +enum DropInputMode { + /** + * Default mode, input events are sent to the target as usual. + */ + NONE, + + /** + * Window and its children will not receive any input even if it has a valid input channel. + * Touches and keys will be dropped. If a window is focused, it will remain focused but will + * not receive any keys. If the window has a touchable region and is the target of an input + * event, the event will be dropped and will not go to the window behind. ref: b/197296414 + */ + ALL, +} + diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index a77e4b0462..39dbe9e035 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -25,6 +25,7 @@ #include #include +#include #ifndef NO_INPUT #include