diff options
| author | 2024-07-15 20:12:56 +0000 | |
|---|---|---|
| committer | 2024-07-15 20:13:00 +0000 | |
| commit | 236f2a163e657ec4db7eda810db45cac16897ffa (patch) | |
| tree | c54cb6d5d138c1584b5b572e8a8091198e026aa1 | |
| parent | 69d9721f3b62c58a55ac4087560c3d23384c9798 (diff) | |
Revert "Migrate audio permission provider prop listener"
Revert submission 27791457-audioserver_perm4
Reason for revert: Droidmonitor created revert due to b/353333914.
Reverted changes: /q/submissionid:27791457-audioserver_perm4
Change-Id: I684a3071feb78c214b083dde37b196e4b8f64f87
9 files changed, 31 insertions, 124 deletions
diff --git a/core/jni/android_media_AudioSystem.cpp b/core/jni/android_media_AudioSystem.cpp index a278fea3b9bc..eaff7608ce3b 100644 --- a/core/jni/android_media_AudioSystem.cpp +++ b/core/jni/android_media_AudioSystem.cpp @@ -27,7 +27,6 @@ #include <android_media_audiopolicy.h> #include <android_os_Parcel.h> #include <audiomanager/AudioManager.h> -#include <android-base/properties.h> #include <binder/IBinder.h> #include <jni.h> #include <media/AidlConversion.h> @@ -42,10 +41,8 @@ #include <system/audio_policy.h> #include <utils/Log.h> -#include <thread> #include <optional> #include <sstream> -#include <memory> #include <vector> #include "android_media_AudioAttributes.h" @@ -264,13 +261,6 @@ static struct { jfieldID mMixerBehavior; } gAudioMixerAttributesField; -static struct { - jclass clazz; - jmethodID run; -} gRunnableClassInfo; - -static JavaVM* gVm; - static Mutex gLock; enum AudioError { @@ -3373,55 +3363,6 @@ static jboolean android_media_AudioSystem_isBluetoothVariableLatencyEnabled(JNIE return enabled; } -class JavaSystemPropertyListener { - public: - JavaSystemPropertyListener(JNIEnv* env, jobject javaCallback, std::string sysPropName) : - mCallback(env->NewGlobalRef(javaCallback)), - mCachedProperty(android::base::CachedProperty{std::move(sysPropName)}) { - mListenerThread = std::thread([this]() mutable { - JNIEnv* threadEnv = GetOrAttachJNIEnvironment(gVm); - while (!mCleanupSignal.load()) { - using namespace std::chrono_literals; - // 1s timeout so this thread can read the cleanup signal to (slowly) be able to - // be destroyed. - std::string newVal = mCachedProperty.WaitForChange(1000ms) ?: ""; - if (newVal != "" && mLastVal != newVal) { - threadEnv->CallVoidMethod(mCallback, gRunnableClassInfo.run); - mLastVal = std::move(newVal); - } - } - }); - } - - ~JavaSystemPropertyListener() { - mCleanupSignal.store(true); - mListenerThread.join(); - JNIEnv* env = GetOrAttachJNIEnvironment(gVm); - env->DeleteGlobalRef(mCallback); - } - - private: - jobject mCallback; - android::base::CachedProperty mCachedProperty; - std::thread mListenerThread; - std::atomic<bool> mCleanupSignal{false}; - std::string mLastVal = ""; -}; - -std::vector<std::unique_ptr<JavaSystemPropertyListener>> gSystemPropertyListeners; -std::mutex gSysPropLock{}; - -static void android_media_AudioSystem_listenForSystemPropertyChange(JNIEnv *env, jobject thiz, - jstring sysProp, - jobject javaCallback) { - ScopedUtfChars sysPropChars{env, sysProp}; - auto listener = std::make_unique<JavaSystemPropertyListener>(env, javaCallback, - std::string{sysPropChars.c_str()}); - std::unique_lock _l{gSysPropLock}; - gSystemPropertyListeners.push_back(std::move(listener)); -} - - // ---------------------------------------------------------------------------- #define MAKE_AUDIO_SYSTEM_METHOD(x) \ @@ -3594,12 +3535,7 @@ static const JNINativeMethod gMethods[] = android_media_AudioSystem_clearPreferredMixerAttributes), MAKE_AUDIO_SYSTEM_METHOD(supportsBluetoothVariableLatency), MAKE_AUDIO_SYSTEM_METHOD(setBluetoothVariableLatencyEnabled), - MAKE_AUDIO_SYSTEM_METHOD(isBluetoothVariableLatencyEnabled), - MAKE_JNI_NATIVE_METHOD("listenForSystemPropertyChange", - "(Ljava/lang/String;Ljava/lang/Runnable;)V", - android_media_AudioSystem_listenForSystemPropertyChange), - - }; + MAKE_AUDIO_SYSTEM_METHOD(isBluetoothVariableLatencyEnabled)}; static const JNINativeMethod gEventHandlerMethods[] = {MAKE_JNI_NATIVE_METHOD("native_setup", "(Ljava/lang/Object;)V", @@ -3881,12 +3817,6 @@ int register_android_media_AudioSystem(JNIEnv *env) gAudioMixerAttributesField.mMixerBehavior = GetFieldIDOrDie(env, audioMixerAttributesClass, "mMixerBehavior", "I"); - jclass runnableClazz = FindClassOrDie(env, "java/lang/Runnable"); - gRunnableClassInfo.clazz = MakeGlobalRefOrDie(env, runnableClazz); - gRunnableClassInfo.run = GetMethodIDOrDie(env, runnableClazz, "run", "()V"); - - LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&gVm) != 0); - AudioSystem::addErrorCallback(android_media_AudioSystem_error_callback); RegisterMethodsOrDie(env, kClassPathName, gMethods, NELEM(gMethods)); diff --git a/media/java/android/media/AudioSystem.java b/media/java/android/media/AudioSystem.java index 52b5ff77c031..d148afd3c15d 100644 --- a/media/java/android/media/AudioSystem.java +++ b/media/java/android/media/AudioSystem.java @@ -2649,11 +2649,4 @@ public class AudioSystem * @hide */ public static native boolean isBluetoothVariableLatencyEnabled(); - - /** - * Register a native listener for system property sysprop - * @param callback the listener which fires when the property changes - * @hide - */ - public static native void listenForSystemPropertyChange(String sysprop, Runnable callback); } diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index 41b984b67d02..74d218cba7ab 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -734,8 +734,6 @@ public class AudioService extends IAudioService.Stub // Broadcast receiver for device connections intent broadcasts private final BroadcastReceiver mReceiver = new AudioServiceBroadcastReceiver(); - private final Executor mAudioServerLifecycleExecutor; - private IMediaProjectionManager mProjectionService; // to validate projection token /** Interface for UserManagerService. */ @@ -1054,8 +1052,7 @@ public class AudioService extends IAudioService.Stub audioserverPermissions() ? initializeAudioServerPermissionProvider( context, audioPolicyFacade, audioserverLifecycleExecutor) : - null, - audioserverLifecycleExecutor + null ); } @@ -1141,16 +1138,13 @@ public class AudioService extends IAudioService.Stub * {@link AudioSystemThread} is created as the messaging thread instead. * @param appOps {@link AppOpsManager} system service * @param enforcer Used for permission enforcing - * @param permissionProvider Used to push permissions to audioserver - * @param audioserverLifecycleExecutor Used for tasks managing audioserver lifecycle */ @RequiresPermission(Manifest.permission.READ_DEVICE_CONFIG) public AudioService(Context context, AudioSystemAdapter audioSystem, SystemServerAdapter systemServer, SettingsAdapter settings, AudioVolumeGroupHelperBase audioVolumeGroupHelper, AudioPolicyFacade audioPolicy, @Nullable Looper looper, AppOpsManager appOps, @NonNull PermissionEnforcer enforcer, - /* @NonNull */ AudioServerPermissionProvider permissionProvider, - Executor audioserverLifecycleExecutor) { + /* @NonNull */ AudioServerPermissionProvider permissionProvider) { super(enforcer); sLifecycleLogger.enqueue(new EventLogger.StringEvent("AudioService()")); mContext = context; @@ -1158,7 +1152,6 @@ public class AudioService extends IAudioService.Stub mAppOps = appOps; mPermissionProvider = permissionProvider; - mAudioServerLifecycleExecutor = audioserverLifecycleExecutor; mAudioSystem = audioSystem; mSystemServer = systemServer; @@ -1170,34 +1163,6 @@ public class AudioService extends IAudioService.Stub mBroadcastHandlerThread = new HandlerThread("AudioService Broadcast"); mBroadcastHandlerThread.start(); - // Listen to permission invalidations for the PermissionProvider - if (audioserverPermissions()) { - final Handler broadcastHandler = mBroadcastHandlerThread.getThreadHandler(); - mAudioSystem.listenForSystemPropertyChange(PermissionManager.CACHE_KEY_PACKAGE_INFO, - new Runnable() { - // Roughly chosen to be long enough to suppress the autocork behavior - // of the permission cache (50ms), and longer than the task could reasonably - // take, even with many packages and users, while not introducing visible - // permission leaks - since the app needs to restart, and trigger an action - // which requires permissions from audioserver before this delay. - // For RECORD_AUDIO, we are additionally protected by appops. - final long UPDATE_DELAY_MS = 110; - final AtomicLong scheduledUpdateTimestamp = new AtomicLong(0); - @Override - public void run() { - var currentTime = SystemClock.uptimeMillis(); - if (currentTime > scheduledUpdateTimestamp.get()) { - scheduledUpdateTimestamp.set(currentTime + UPDATE_DELAY_MS); - broadcastHandler.postAtTime( () -> - mAudioServerLifecycleExecutor.execute(mPermissionProvider - ::onPermissionStateChanged), - currentTime + UPDATE_DELAY_MS - ); - } - } - }); - } - mDeviceBroker = new AudioDeviceBroker(mContext, this, mAudioSystem); mIsSingleVolume = AudioSystem.isSingleVolume(context); @@ -12007,6 +11972,29 @@ public class AudioService extends IAudioService.Stub provider.onServiceStart(audioPolicy.getPermissionController()); }); + // Set up event listeners + // Must be kept in sync with PermissionManager + Runnable cacheSysPropHandler = new Runnable() { + private AtomicReference<SystemProperties.Handle> mHandle = new AtomicReference(); + private AtomicLong mNonce = new AtomicLong(); + @Override + public void run() { + if (mHandle.get() == null) { + // Cache the handle + mHandle.compareAndSet(null, SystemProperties.find( + PermissionManager.CACHE_KEY_PACKAGE_INFO)); + } + long nonce; + SystemProperties.Handle ref; + if ((ref = mHandle.get()) != null && (nonce = ref.getLong(0)) != 0 && + mNonce.getAndSet(nonce) != nonce) { + audioserverExecutor.execute(() -> provider.onPermissionStateChanged()); + } + } + }; + + SystemProperties.addChangeCallback(cacheSysPropHandler); + IntentFilter packageUpdateFilter = new IntentFilter(); packageUpdateFilter.addAction(ACTION_PACKAGE_ADDED); packageUpdateFilter.addAction(ACTION_PACKAGE_REMOVED); diff --git a/services/core/java/com/android/server/audio/AudioSystemAdapter.java b/services/core/java/com/android/server/audio/AudioSystemAdapter.java index d083c68c4c2c..7f4bc74bd59e 100644 --- a/services/core/java/com/android/server/audio/AudioSystemAdapter.java +++ b/services/core/java/com/android/server/audio/AudioSystemAdapter.java @@ -748,10 +748,6 @@ public class AudioSystemAdapter implements AudioSystem.RoutingUpdateCallback, return AudioSystem.setMasterMute(mute); } - public void listenForSystemPropertyChange(String systemPropertyName, Runnable callback) { - AudioSystem.listenForSystemPropertyChange(systemPropertyName, callback); - } - /** * Part of AudioService dump * @param pw diff --git a/services/tests/servicestests/src/com/android/server/audio/AbsoluteVolumeBehaviorTest.java b/services/tests/servicestests/src/com/android/server/audio/AbsoluteVolumeBehaviorTest.java index ef9580c54de6..758c84a26dcd 100644 --- a/services/tests/servicestests/src/com/android/server/audio/AbsoluteVolumeBehaviorTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/AbsoluteVolumeBehaviorTest.java @@ -101,7 +101,7 @@ public class AbsoluteVolumeBehaviorTest { mAudioService = new AudioService(mContext, mSpyAudioSystem, mSystemServer, mSettingsAdapter, mAudioVolumeGroupHelper, mMockAudioPolicy, mTestLooper.getLooper(), mock(AppOpsManager.class), mock(PermissionEnforcer.class), - mock(AudioServerPermissionProvider.class), r -> r.run()) { + mock(AudioServerPermissionProvider.class)) { @Override public int getDeviceForStream(int stream) { return AudioSystem.DEVICE_OUT_SPEAKER; diff --git a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceVolumeManagerTest.java b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceVolumeManagerTest.java index 464515632997..2cb02bdd2806 100644 --- a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceVolumeManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceVolumeManagerTest.java @@ -78,7 +78,7 @@ public class AudioDeviceVolumeManagerTest { mAudioService = new AudioService(mContext, mSpyAudioSystem, mSystemServer, mSettingsAdapter, mAudioVolumeGroupHelper, mAudioPolicyMock, mTestLooper.getLooper(), mock(AppOpsManager.class), mock(PermissionEnforcer.class), - mock(AudioServerPermissionProvider.class), r -> r.run()) { + mock(AudioServerPermissionProvider.class)) { @Override public int getDeviceForStream(int stream) { return AudioSystem.DEVICE_OUT_SPEAKER; diff --git a/services/tests/servicestests/src/com/android/server/audio/AudioServiceTest.java b/services/tests/servicestests/src/com/android/server/audio/AudioServiceTest.java index b7100ea00a40..037c3c00443c 100644 --- a/services/tests/servicestests/src/com/android/server/audio/AudioServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/AudioServiceTest.java @@ -87,7 +87,7 @@ public class AudioServiceTest { .thenReturn(AppOpsManager.MODE_ALLOWED); mAudioService = new AudioService(mContext, mSpyAudioSystem, mSpySystemServer, mSettingsAdapter, mAudioVolumeGroupHelper, mMockAudioPolicy, null, - mMockAppOpsManager, mMockPermissionEnforcer, mMockPermissionProvider, r -> r.run()); + mMockAppOpsManager, mMockPermissionEnforcer, mMockPermissionProvider); } /** diff --git a/services/tests/servicestests/src/com/android/server/audio/DeviceVolumeBehaviorTest.java b/services/tests/servicestests/src/com/android/server/audio/DeviceVolumeBehaviorTest.java index 746645a8c585..27b552fa7cdd 100644 --- a/services/tests/servicestests/src/com/android/server/audio/DeviceVolumeBehaviorTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/DeviceVolumeBehaviorTest.java @@ -78,7 +78,7 @@ public class DeviceVolumeBehaviorTest { mAudioService = new AudioService(mContext, mAudioSystem, mSystemServer, mSettingsAdapter, mAudioVolumeGroupHelper, mAudioPolicyMock, mTestLooper.getLooper(), mock(AppOpsManager.class), mock(PermissionEnforcer.class), - mock(AudioServerPermissionProvider.class), r -> r.run()); + mock(AudioServerPermissionProvider.class)); mTestLooper.dispatchAll(); } diff --git a/services/tests/servicestests/src/com/android/server/audio/VolumeHelperTest.java b/services/tests/servicestests/src/com/android/server/audio/VolumeHelperTest.java index e45ab319146c..8e34ee1b6a42 100644 --- a/services/tests/servicestests/src/com/android/server/audio/VolumeHelperTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/VolumeHelperTest.java @@ -160,7 +160,7 @@ public class VolumeHelperTest { @NonNull PermissionEnforcer enforcer, AudioServerPermissionProvider permissionProvider) { super(context, audioSystem, systemServer, settings, audioVolumeGroupHelper, - audioPolicy, looper, appOps, enforcer, permissionProvider, r -> r.run()); + audioPolicy, looper, appOps, enforcer, permissionProvider); } public void setDeviceForStream(int stream, int device) { |