diff options
30 files changed, 559 insertions, 110 deletions
diff --git a/core/java/android/util/ArrayMap.java b/core/java/android/util/ArrayMap.java index 7ee0ff15c5ad..c59907937d6a 100644 --- a/core/java/android/util/ArrayMap.java +++ b/core/java/android/util/ArrayMap.java @@ -129,7 +129,7 @@ public final class ArrayMap<K, V> implements Map<K, V> { return ContainerHelpers.binarySearch(hashes, N, hash); } catch (ArrayIndexOutOfBoundsException e) { if (CONCURRENT_MODIFICATION_EXCEPTIONS) { - throw new ConcurrentModificationException(); + throw new ConcurrentModificationException(e); } else { throw e; // the cache is poisoned at this point, there's not much we can do } diff --git a/core/res/res/values/config_telephony.xml b/core/res/res/values/config_telephony.xml index e9f5a2f5913f..849ca2882889 100644 --- a/core/res/res/values/config_telephony.xml +++ b/core/res/res/values/config_telephony.xml @@ -261,7 +261,9 @@ to identify providers that should be ignored if the carrier config carrier_supported_satellite_services_per_provider_bundle does not support them. --> - <string-array name="config_satellite_providers" translatable="false"></string-array> + <string-array name="config_satellite_providers" translatable="false"> + <item>"310830"</item> + </string-array> <java-symbol type="array" name="config_satellite_providers" /> <!-- The identifier of the satellite's SIM profile. The identifier is composed of MCC and MNC diff --git a/core/tests/coretests/src/com/android/internal/app/MediaRouteDialogPresenterTest.kt b/core/tests/coretests/src/com/android/internal/app/MediaRouteDialogPresenterTest.kt new file mode 100644 index 000000000000..e80d3a6e625f --- /dev/null +++ b/core/tests/coretests/src/com/android/internal/app/MediaRouteDialogPresenterTest.kt @@ -0,0 +1,92 @@ +/* + * Copyright (C) 2025 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 com.android.internal.app + +import android.content.Context +import android.media.MediaRouter +import android.testing.TestableLooper.RunWithLooper +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SmallTest +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.stub + +@SmallTest +@RunWithLooper(setAsMainLooper = true) +@RunWith(AndroidJUnit4::class) +class MediaRouteDialogPresenterTest { + private var selectedRoute: MediaRouter.RouteInfo = mock() + private var mediaRouter: MediaRouter = mock<MediaRouter> { + on { selectedRoute } doReturn selectedRoute + } + private var context: Context = mock<Context> { + on { getSystemServiceName(MediaRouter::class.java) } doReturn Context.MEDIA_ROUTER_SERVICE + on { getSystemService(MediaRouter::class.java) } doReturn mediaRouter + } + + @Test + fun shouldShowChooserDialog_routeNotDefault_returnsFalse() { + selectedRoute.stub { + on { isDefault } doReturn false + on { matchesTypes(anyInt()) } doReturn true + } + + assertThat(MediaRouteDialogPresenter.shouldShowChooserDialog( + context, MediaRouter.ROUTE_TYPE_REMOTE_DISPLAY)) + .isEqualTo(false) + } + + @Test + fun shouldShowChooserDialog_routeDefault_returnsTrue() { + selectedRoute.stub { + on { isDefault } doReturn true + on { matchesTypes(anyInt()) } doReturn true + } + + assertThat(MediaRouteDialogPresenter.shouldShowChooserDialog( + context, MediaRouter.ROUTE_TYPE_REMOTE_DISPLAY)) + .isEqualTo(true) + } + + @Test + fun shouldShowChooserDialog_routeNotMatch_returnsTrue() { + selectedRoute.stub { + on { isDefault } doReturn false + on { matchesTypes(anyInt()) } doReturn false + } + + assertThat(MediaRouteDialogPresenter.shouldShowChooserDialog( + context, MediaRouter.ROUTE_TYPE_REMOTE_DISPLAY)) + .isEqualTo(true) + } + + @Test + fun shouldShowChooserDialog_routeDefaultAndNotMatch_returnsTrue() { + selectedRoute.stub { + on { isDefault } doReturn true + on { matchesTypes(anyInt()) } doReturn false + } + + assertThat(MediaRouteDialogPresenter.shouldShowChooserDialog( + context, MediaRouter.ROUTE_TYPE_REMOTE_DISPLAY)) + .isEqualTo(true) + } +}
\ No newline at end of file diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/common/split/FlexParallaxSpec.java b/libs/WindowManager/Shell/src/com/android/wm/shell/common/split/FlexParallaxSpec.java index 9fa162164e0e..d9a66e1d64b2 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/common/split/FlexParallaxSpec.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/common/split/FlexParallaxSpec.java @@ -57,6 +57,11 @@ public class FlexParallaxSpec implements ParallaxSpec { * @return 0f = no dim applied. 1f = full black. */ public float getDimValue(int position, DividerSnapAlgorithm snapAlgorithm) { + // On tablets, apps don't go offscreen, so only dim for dismissal. + if (!snapAlgorithm.areOffscreenRatiosSupported()) { + return ParallaxSpec.super.getDimValue(position, snapAlgorithm); + } + int startDismissPos = snapAlgorithm.getDismissStartTarget().getPosition(); int firstTargetPos = snapAlgorithm.getFirstSplitTarget().getPosition(); int middleTargetPos = snapAlgorithm.getMiddleTarget().getPosition(); diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/HandleMenu.kt b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/HandleMenu.kt index 2d44395b1340..5f13ba907831 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/HandleMenu.kt +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/HandleMenu.kt @@ -762,6 +762,7 @@ class HandleMenu( floatingBtn.isEnabled = !taskInfo.isPinned floatingBtn.imageTintList = style.windowingButtonColor desktopBtn.isGone = !shouldShowDesktopModeButton + desktopBtnSpace.isGone = !shouldShowDesktopModeButton desktopBtn.isSelected = taskInfo.isFreeform desktopBtn.isEnabled = !taskInfo.isFreeform desktopBtn.imageTintList = style.windowingButtonColor diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/split/FlexParallaxSpecTests.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/split/FlexParallaxSpecTests.java index 22a85fc49a4b..9f2534eb2662 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/split/FlexParallaxSpecTests.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/split/FlexParallaxSpecTests.java @@ -71,6 +71,7 @@ public class FlexParallaxSpecTests { when(mockSnapAlgorithm.getMiddleTarget()).thenReturn(mockMiddleTarget); when(mockSnapAlgorithm.getLastSplitTarget()).thenReturn(mockLastTarget); when(mockSnapAlgorithm.getDismissEndTarget()).thenReturn(mockEndEdge); + when(mockSnapAlgorithm.areOffscreenRatiosSupported()).thenReturn(true); when(mockStartEdge.getPosition()).thenReturn(0); when(mockFirstTarget.getPosition()).thenReturn(250); diff --git a/packages/SettingsLib/src/com/android/settingslib/notification/data/repository/ZenModeRepository.kt b/packages/SettingsLib/src/com/android/settingslib/notification/data/repository/ZenModeRepository.kt index c4e724554c04..21d518a644a9 100644 --- a/packages/SettingsLib/src/com/android/settingslib/notification/data/repository/ZenModeRepository.kt +++ b/packages/SettingsLib/src/com/android/settingslib/notification/data/repository/ZenModeRepository.kt @@ -27,7 +27,6 @@ import android.content.IntentFilter import android.database.ContentObserver import android.os.Handler import android.provider.Settings -import com.android.settingslib.flags.Flags import com.android.settingslib.notification.modes.ZenMode import com.android.settingslib.notification.modes.ZenModesBackend import java.time.Duration @@ -35,6 +34,7 @@ import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.callbackFlow @@ -72,7 +72,7 @@ class ZenModeRepositoryImpl( private val notificationManager: NotificationManager, private val backend: ZenModesBackend, private val contentResolver: ContentResolver, - val scope: CoroutineScope, + val applicationScope: CoroutineScope, val backgroundCoroutineContext: CoroutineContext, // This is nullable just to simplify testing, since SettingsLib doesn't have a good way // to create a fake handler. @@ -104,7 +104,7 @@ class ZenModeRepositoryImpl( awaitClose { context.unregisterReceiver(receiver) } } .flowOn(backgroundCoroutineContext) - .shareIn(started = SharingStarted.WhileSubscribed(), scope = scope) + .shareIn(started = SharingStarted.WhileSubscribed(), scope = applicationScope) } override val consolidatedNotificationPolicy: StateFlow<NotificationManager.Policy?> by lazy { @@ -129,14 +129,11 @@ class ZenModeRepositoryImpl( .map { mapper(it) } .onStart { emit(mapper(null)) } .flowOn(backgroundCoroutineContext) - .stateIn(scope, SharingStarted.WhileSubscribed(), null) + .stateIn(applicationScope, SharingStarted.WhileSubscribed(), null) private val zenConfigChanged by lazy { if (android.app.Flags.modesUi()) { callbackFlow { - // emit an initial value - trySend(Unit) - val observer = object : ContentObserver(backgroundHandler) { override fun onChange(selfChange: Boolean) { @@ -163,16 +160,18 @@ class ZenModeRepositoryImpl( } } - override val modes: Flow<List<ZenMode>> by lazy { - if (android.app.Flags.modesUi()) { + override val modes: StateFlow<List<ZenMode>> = + if (android.app.Flags.modesUi()) zenConfigChanged .map { backend.modes } .distinctUntilChanged() .flowOn(backgroundCoroutineContext) - } else { - flowOf(emptyList()) - } - } + .stateIn( + scope = applicationScope, + started = SharingStarted.Eagerly, + initialValue = backend.modes, + ) + else MutableStateFlow<List<ZenMode>>(emptyList()) /** * Gets the current list of [ZenMode] instances according to the backend. diff --git a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/notification/data/repository/ZenModeRepositoryTest.kt b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/notification/data/repository/ZenModeRepositoryTest.kt index b364368df473..ec7baf6bf081 100644 --- a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/notification/data/repository/ZenModeRepositoryTest.kt +++ b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/notification/data/repository/ZenModeRepositoryTest.kt @@ -75,10 +75,14 @@ class ZenModeRepositoryTest { private val testScope: TestScope = TestScope() + private val initialModes = listOf(TestModeBuilder().setId("Built-in").build()) + @Before fun setup() { MockitoAnnotations.initMocks(this) + `when`(zenModesBackend.modes).thenReturn(initialModes) + underTest = ZenModeRepositoryImpl( context, @@ -151,8 +155,8 @@ class ZenModeRepositoryTest { fun modesListEmitsOnSettingsChange() { testScope.runTest { val values = mutableListOf<List<ZenMode>>() - val modes1 = listOf(TestModeBuilder().setId("One").build()) - `when`(zenModesBackend.modes).thenReturn(modes1) + + // an initial list of modes is read when the stateflow is created underTest.modes.onEach { values.add(it) }.launchIn(backgroundScope) runCurrent() @@ -172,7 +176,7 @@ class ZenModeRepositoryTest { triggerZenModeSettingUpdate() runCurrent() - assertThat(values).containsExactly(modes1, modes2, modes3).inOrder() + assertThat(values).containsExactly(initialModes, modes2, modes3).inOrder() } } diff --git a/packages/SystemUI/aconfig/systemui.aconfig b/packages/SystemUI/aconfig/systemui.aconfig index cea91a18ae32..685c689b6c28 100644 --- a/packages/SystemUI/aconfig/systemui.aconfig +++ b/packages/SystemUI/aconfig/systemui.aconfig @@ -289,6 +289,15 @@ flag { } } +flag { + name: "notification_skip_silent_updates" + namespace: "systemui" + description: "Do not notify HeadsUpManager for silent updates." + bug: "401068530" + metadata { + purpose: PURPOSE_BUGFIX + } +} flag { name: "scene_container" diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/dialog/CastDetailsViewModelTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/dialog/CastDetailsViewModelTest.kt new file mode 100644 index 000000000000..468c3dc3be93 --- /dev/null +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/dialog/CastDetailsViewModelTest.kt @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2025 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 com.android.systemui.qs.tiles.dialog + +import android.content.Context +import android.media.MediaRouter +import android.provider.Settings +import android.testing.TestableLooper.RunWithLooper +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SmallTest +import com.android.internal.app.MediaRouteDialogPresenter +import com.android.systemui.SysuiTestCase +import com.android.systemui.qs.tiles.base.domain.actions.FakeQSTileIntentUserInputHandler +import com.android.systemui.qs.tiles.base.domain.actions.intentInputs +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.stub + +@SmallTest +@RunWithLooper(setAsMainLooper = true) +@RunWith(AndroidJUnit4::class) +class CastDetailsViewModelTest : SysuiTestCase() { + var inputHandler: FakeQSTileIntentUserInputHandler = FakeQSTileIntentUserInputHandler() + private var context: Context = mock() + private var mediaRouter: MediaRouter = mock() + private var selectedRoute: MediaRouter.RouteInfo = mock() + + @Test + fun testClickOnSettingsButton() { + var viewModel = CastDetailsViewModel(inputHandler, context, MediaRouter.ROUTE_TYPE_REMOTE_DISPLAY) + + viewModel.clickOnSettingsButton() + + assertThat(inputHandler.handledInputs).hasSize(1) + val intentInput = inputHandler.intentInputs.last() + assertThat(intentInput.expandable).isNull() + assertThat(intentInput.intent.action).isEqualTo(Settings.ACTION_CAST_SETTINGS) + } + + @Test + fun testShouldShowChooserDialog() { + context.stub { + on { getSystemService(MediaRouter::class.java) } doReturn mediaRouter + } + mediaRouter.stub { + on { selectedRoute } doReturn selectedRoute + } + + var viewModel = + CastDetailsViewModel(inputHandler, context, MediaRouter.ROUTE_TYPE_REMOTE_DISPLAY) + + assertThat(viewModel.shouldShowChooserDialog()) + .isEqualTo( + MediaRouteDialogPresenter.shouldShowChooserDialog( + context, + MediaRouter.ROUTE_TYPE_REMOTE_DISPLAY, + ) + ) + } +} diff --git a/packages/SystemUI/res/layout/activity_rear_display_enabled.xml b/packages/SystemUI/res/layout/activity_rear_display_enabled.xml index f900626b4da6..6b633e03f1f2 100644 --- a/packages/SystemUI/res/layout/activity_rear_display_enabled.xml +++ b/packages/SystemUI/res/layout/activity_rear_display_enabled.xml @@ -56,6 +56,7 @@ android:gravity="center_horizontal" /> <TextView + android:id="@+id/seekbar_instructions" android:layout_width="wrap_content" android:layout_height="wrap_content" android:text="@string/rear_display_unfolded_front_screen_on_slide_to_cancel" @@ -73,4 +74,13 @@ android:background="@null" android:gravity="center_horizontal" /> + <Button + android:id="@+id/cancel_button" + android:text="@string/cancel" + android:layout_width="@dimen/rear_display_animation_width_opened" + android:layout_height="wrap_content" + android:gravity="center_horizontal" + android:visibility="gone" + style="@style/Widget.Dialog.Button.BorderButton"/> + </LinearLayout> diff --git a/packages/SystemUI/src/com/android/systemui/reardisplay/RearDisplayCoreStartable.kt b/packages/SystemUI/src/com/android/systemui/reardisplay/RearDisplayCoreStartable.kt index 263ef09ea767..5d4a774d77f9 100644 --- a/packages/SystemUI/src/com/android/systemui/reardisplay/RearDisplayCoreStartable.kt +++ b/packages/SystemUI/src/com/android/systemui/reardisplay/RearDisplayCoreStartable.kt @@ -19,14 +19,18 @@ package com.android.systemui.reardisplay import android.content.Context import android.hardware.devicestate.DeviceStateManager import android.hardware.devicestate.feature.flags.Flags +import android.os.Handler +import android.view.accessibility.AccessibilityManager import androidx.annotation.VisibleForTesting import com.android.keyguard.KeyguardUpdateMonitor import com.android.keyguard.KeyguardUpdateMonitorCallback import com.android.systemui.CoreStartable import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Application +import com.android.systemui.dagger.qualifiers.Background import com.android.systemui.display.domain.interactor.RearDisplayStateInteractor import com.android.systemui.statusbar.phone.SystemUIDialog +import java.util.concurrent.atomic.AtomicBoolean import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job @@ -52,6 +56,8 @@ internal constructor( private val rearDisplayInnerDialogDelegateFactory: RearDisplayInnerDialogDelegate.Factory, @Application private val scope: CoroutineScope, private val keyguardUpdateMonitor: KeyguardUpdateMonitor, + private val accessibilityManager: AccessibilityManager, + @Background private val handler: Handler, ) : CoreStartable, AutoCloseable { companion object { @@ -77,6 +83,12 @@ internal constructor( override fun start() { if (Flags.deviceStateRdmV2()) { var dialog: SystemUIDialog? = null + var touchExplorationEnabled = AtomicBoolean(false) + + accessibilityManager.addTouchExplorationStateChangeListener( + { enabled -> touchExplorationEnabled.set(enabled) }, + handler, + ) keyguardUpdateMonitor.registerCallback(keyguardCallback) @@ -99,6 +111,7 @@ internal constructor( rearDisplayInnerDialogDelegateFactory.create( rearDisplayContext, deviceStateManager::cancelStateRequest, + touchExplorationEnabled.get(), ) dialog = delegate.createDialog().apply { show() } } diff --git a/packages/SystemUI/src/com/android/systemui/reardisplay/RearDisplayInnerDialogDelegate.kt b/packages/SystemUI/src/com/android/systemui/reardisplay/RearDisplayInnerDialogDelegate.kt index f5facf42ee67..96f1bd270239 100644 --- a/packages/SystemUI/src/com/android/systemui/reardisplay/RearDisplayInnerDialogDelegate.kt +++ b/packages/SystemUI/src/com/android/systemui/reardisplay/RearDisplayInnerDialogDelegate.kt @@ -20,7 +20,10 @@ import android.annotation.SuppressLint import android.content.Context import android.os.Bundle import android.view.MotionEvent +import android.view.View +import android.widget.Button import android.widget.SeekBar +import android.widget.TextView import com.android.systemui.haptics.slider.HapticSlider import com.android.systemui.haptics.slider.HapticSliderPlugin import com.android.systemui.haptics.slider.HapticSliderViewBinder @@ -45,6 +48,7 @@ class RearDisplayInnerDialogDelegate internal constructor( private val systemUIDialogFactory: SystemUIDialog.Factory, @Assisted private val rearDisplayContext: Context, + @Assisted private val touchExplorationEnabled: Boolean, private val vibratorHelper: VibratorHelper, private val msdlPlayer: MSDLPlayer, private val systemClock: SystemClock, @@ -82,6 +86,7 @@ internal constructor( fun create( rearDisplayContext: Context, onCanceledRunnable: Runnable, + touchExplorationEnabled: Boolean, ): RearDisplayInnerDialogDelegate } @@ -95,11 +100,32 @@ internal constructor( @SuppressLint("ClickableViewAccessibility") override fun onCreate(dialog: SystemUIDialog, savedInstanceState: Bundle?) { + dialog.apply { setContentView(R.layout.activity_rear_display_enabled) setCanceledOnTouchOutside(false) + requireViewById<Button>(R.id.cancel_button).let { it -> + if (!touchExplorationEnabled) { + return@let + } + + it.visibility = View.VISIBLE + it.setOnClickListener { onCanceledRunnable.run() } + } + + requireViewById<TextView>(R.id.seekbar_instructions).let { it -> + if (touchExplorationEnabled) { + it.visibility = View.GONE + } + } + requireViewById<SeekBar>(R.id.seekbar).let { it -> + if (touchExplorationEnabled) { + it.visibility = View.GONE + return@let + } + // Create and bind the HapticSliderPlugin val hapticSliderPlugin = HapticSliderPlugin( diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt index a0eab43f854b..26b86f9ed74d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt @@ -15,6 +15,8 @@ */ package com.android.systemui.statusbar.notification.collection.coordinator +import com.android.systemui.Flags.notificationSkipSilentUpdates + import android.app.Notification import android.app.Notification.GROUP_ALERT_SUMMARY import android.util.ArrayMap @@ -465,15 +467,32 @@ constructor( } hunMutator.updateNotification(posted.key, pinnedStatus) } - } else { + } else { // shouldHeadsUpEver = false if (posted.isHeadsUpEntry) { - // We don't want this to be interrupting anymore, let's remove it - // If the notification is pinned by the user, the only way a user can un-pin - // it is by tapping the status bar notification chip. Since that's a clear - // user action, we should remove the HUN immediately instead of waiting for - // any sort of minimum timeout. - val shouldRemoveImmediately = posted.isPinnedByUser - hunMutator.removeNotification(posted.key, shouldRemoveImmediately) + if (notificationSkipSilentUpdates()) { + if (posted.isPinnedByUser) { + // We don't want this to be interrupting anymore, let's remove it + // If the notification is pinned by the user, the only way a user + // can un-pin it by tapping the status bar notification chip. Since + // that's a clear user action, we should remove the HUN immediately + // instead of waiting for any sort of minimum timeout. + // TODO(b/401068530) Ensure that status bar chip HUNs are not + // removed for silent update + hunMutator.removeNotification(posted.key, + /* releaseImmediately= */ true) + } else { + // Do NOT remove HUN for non-user update. + // Let the HUN show for its remaining duration. + } + } else { + // We don't want this to be interrupting anymore, let's remove it + // If the notification is pinned by the user, the only way a user can + // un-pin it is by tapping the status bar notification chip. Since + // that's a clear user action, we should remove the HUN immediately + // instead of waiting for any sort of minimum timeout. + val shouldRemoveImmediately = posted.isPinnedByUser + hunMutator.removeNotification(posted.key, shouldRemoveImmediately) + } } else { // Don't let the bind finish cancelHeadsUpBind(posted.entry) @@ -573,24 +592,34 @@ constructor( isBinding = isBinding, ) } - // Handle cancelling heads up here, rather than in the OnBeforeFinalizeFilter, so - // that - // work can be done before the ShadeListBuilder is run. This prevents re-entrant - // behavior between this Coordinator, HeadsUpManager, and VisualStabilityManager. - if (posted?.shouldHeadsUpEver == false) { - if (posted.isHeadsUpEntry) { - // We don't want this to be interrupting anymore, let's remove it - mHeadsUpManager.removeNotification( - posted.key, - /* removeImmediately= */ false, - "onEntryUpdated", - ) - } else if (posted.isBinding) { + if (notificationSkipSilentUpdates()) { + // TODO(b/403703828) Move canceling to OnBeforeFinalizeFilter, since we are not + // removing from HeadsUpManager and don't need to deal with re-entrant behavior + // between HeadsUpCoordinator, HeadsUpManager, and VisualStabilityManager. + if (posted?.shouldHeadsUpEver == false + && !posted.isHeadsUpEntry && posted.isBinding) { // Don't let the bind finish cancelHeadsUpBind(posted.entry) } + } else { + // Handle cancelling heads up here, rather than in the OnBeforeFinalizeFilter, + // so that work can be done before the ShadeListBuilder is run. This prevents + // re-entrant behavior between this Coordinator, HeadsUpManager, and + // VisualStabilityManager. + if (posted?.shouldHeadsUpEver == false) { + if (posted.isHeadsUpEntry) { + // We don't want this to be interrupting anymore, let's remove it + mHeadsUpManager.removeNotification( + posted.key, + /* removeImmediately= */ false, + "onEntryUpdated", + ) + } else if (posted.isBinding) { + // Don't let the bind finish + cancelHeadsUpBind(posted.entry) + } + } } - // Update last updated time for this entry setUpdateTime(entry, mSystemClock.currentTimeMillis()) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java index 1e5aa01714be..990ad9b5cd51 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java @@ -83,7 +83,6 @@ import com.android.systemui.statusbar.notification.logging.dagger.NotificationsL import com.android.systemui.statusbar.notification.promoted.PromotedNotificationContentExtractor; import com.android.systemui.statusbar.notification.promoted.PromotedNotificationContentExtractorImpl; import com.android.systemui.statusbar.notification.promoted.shared.model.PromotedNotificationContentModel; -import com.android.systemui.statusbar.notification.row.NotificationActionClickManager; import com.android.systemui.statusbar.notification.row.NotificationEntryProcessorFactory; import com.android.systemui.statusbar.notification.row.NotificationEntryProcessorFactoryLooperImpl; import com.android.systemui.statusbar.notification.row.NotificationGutsManager; diff --git a/packages/SystemUI/tests/src/com/android/systemui/reardisplay/RearDisplayCoreStartableTest.kt b/packages/SystemUI/tests/src/com/android/systemui/reardisplay/RearDisplayCoreStartableTest.kt index 997cf417fe10..f4d0c26f12ee 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/reardisplay/RearDisplayCoreStartableTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/reardisplay/RearDisplayCoreStartableTest.kt @@ -18,9 +18,11 @@ package com.android.systemui.reardisplay import android.hardware.devicestate.feature.flags.Flags.FLAG_DEVICE_STATE_RDM_V2 import android.hardware.display.rearDisplay +import android.os.fakeExecutorHandler import android.platform.test.annotations.DisableFlags import android.platform.test.annotations.EnableFlags import android.view.Display +import android.view.accessibility.accessibilityManager import androidx.test.filters.SmallTest import com.android.keyguard.keyguardUpdateMonitor import com.android.systemui.SysuiTestCase @@ -62,6 +64,8 @@ class RearDisplayCoreStartableTest : SysuiTestCase() { kosmos.rearDisplayInnerDialogDelegateFactory, kosmos.testScope, kosmos.keyguardUpdateMonitor, + kosmos.accessibilityManager, + kosmos.fakeExecutorHandler, ) @Before @@ -69,7 +73,7 @@ class RearDisplayCoreStartableTest : SysuiTestCase() { whenever(kosmos.rearDisplay.flags).thenReturn(Display.FLAG_REAR) whenever(kosmos.rearDisplay.displayAdjustments) .thenReturn(mContext.display.displayAdjustments) - whenever(kosmos.rearDisplayInnerDialogDelegateFactory.create(any(), any())) + whenever(kosmos.rearDisplayInnerDialogDelegateFactory.create(any(), any(), any())) .thenReturn(mockDelegate) whenever(mockDelegate.createDialog()).thenReturn(mockDialog) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/reardisplay/RearDisplayInnerDialogDelegateTest.kt b/packages/SystemUI/tests/src/com/android/systemui/reardisplay/RearDisplayInnerDialogDelegateTest.kt index fc7661666825..477e5babdcc3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/reardisplay/RearDisplayInnerDialogDelegateTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/reardisplay/RearDisplayInnerDialogDelegateTest.kt @@ -17,7 +17,10 @@ package com.android.systemui.reardisplay import android.testing.TestableLooper +import android.view.View +import android.widget.Button import android.widget.SeekBar +import android.widget.TextView import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.haptics.msdl.msdlPlayer @@ -28,6 +31,7 @@ import com.android.systemui.res.R import com.android.systemui.statusbar.phone.systemUIDialogDotFactory import com.android.systemui.testKosmos import com.android.systemui.util.time.systemClock +import com.google.common.truth.Truth.assertThat import junit.framework.Assert.assertFalse import junit.framework.Assert.assertTrue import org.junit.Test @@ -49,6 +53,7 @@ class RearDisplayInnerDialogDelegateTest : SysuiTestCase() { RearDisplayInnerDialogDelegate( kosmos.systemUIDialogDotFactory, mContext, + false /* touchExplorationEnabled */, kosmos.vibratorHelper, kosmos.msdlPlayer, kosmos.systemClock, @@ -68,6 +73,7 @@ class RearDisplayInnerDialogDelegateTest : SysuiTestCase() { RearDisplayInnerDialogDelegate( kosmos.systemUIDialogDotFactory, mContext, + false /* touchExplorationEnabled */, kosmos.vibratorHelper, kosmos.msdlPlayer, kosmos.systemClock, @@ -78,6 +84,9 @@ class RearDisplayInnerDialogDelegateTest : SysuiTestCase() { .apply { show() val seekbar = findViewById<SeekBar>(R.id.seekbar) + assertThat(seekbar.visibility).isEqualTo(View.VISIBLE) + assertThat(findViewById<TextView>(R.id.seekbar_instructions).visibility) + .isEqualTo(View.VISIBLE) seekbar.progress = 50 seekbar.progress = 100 verify(mockCallback).run() @@ -90,6 +99,7 @@ class RearDisplayInnerDialogDelegateTest : SysuiTestCase() { RearDisplayInnerDialogDelegate( kosmos.systemUIDialogDotFactory, mContext, + false /* touchExplorationEnabled */, kosmos.vibratorHelper, kosmos.msdlPlayer, kosmos.systemClock, @@ -118,4 +128,33 @@ class RearDisplayInnerDialogDelegateTest : SysuiTestCase() { // Progress is reset verify(mockSeekbar).setProgress(eq(0)) } + + @Test + fun testTouchExplorationEnabled() { + val mockCallback = mock<Runnable>() + + RearDisplayInnerDialogDelegate( + kosmos.systemUIDialogDotFactory, + mContext, + true /* touchExplorationEnabled */, + kosmos.vibratorHelper, + kosmos.msdlPlayer, + kosmos.systemClock, + ) { + mockCallback.run() + } + .createDialog() + .apply { + show() + assertThat(findViewById<SeekBar>(R.id.seekbar).visibility).isEqualTo(View.GONE) + assertThat(findViewById<TextView>(R.id.seekbar_instructions).visibility) + .isEqualTo(View.GONE) + + val cancelButton = findViewById<Button>(R.id.cancel_button) + assertThat(cancelButton.visibility).isEqualTo(View.VISIBLE) + + cancelButton.performClick() + verify(mockCallback).run() + } + } } diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java b/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java index 846db6389d0c..2facc1c01ae1 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java @@ -283,6 +283,9 @@ public abstract class SysuiTestCase { } public FakeBroadcastDispatcher getFakeBroadcastDispatcher() { + if (mSysuiDependency == null) { + return null; + } return mSysuiDependency.getFakeBroadcastDispatcher(); } diff --git a/services/core/java/com/android/server/hdmi/HdmiControlService.java b/services/core/java/com/android/server/hdmi/HdmiControlService.java index 41b0b4dc716a..a2d065400045 100644 --- a/services/core/java/com/android/server/hdmi/HdmiControlService.java +++ b/services/core/java/com/android/server/hdmi/HdmiControlService.java @@ -252,17 +252,22 @@ public class HdmiControlService extends SystemService { static final AudioDeviceAttributes AUDIO_OUTPUT_DEVICE_HDMI_EARC = new AudioDeviceAttributes(AudioDeviceAttributes.ROLE_OUTPUT, AudioDeviceInfo.TYPE_HDMI_EARC, ""); + static final AudioDeviceAttributes AUDIO_OUTPUT_DEVICE_LINE_DIGITAL = + new AudioDeviceAttributes(AudioDeviceAttributes.ROLE_OUTPUT, + AudioDeviceInfo.TYPE_LINE_DIGITAL, ""); // Audio output devices used for absolute volume behavior private static final List<AudioDeviceAttributes> AVB_AUDIO_OUTPUT_DEVICES = List.of(AUDIO_OUTPUT_DEVICE_HDMI, AUDIO_OUTPUT_DEVICE_HDMI_ARC, - AUDIO_OUTPUT_DEVICE_HDMI_EARC); + AUDIO_OUTPUT_DEVICE_HDMI_EARC, + AUDIO_OUTPUT_DEVICE_LINE_DIGITAL); // Audio output devices used for absolute volume behavior on TV panels private static final List<AudioDeviceAttributes> TV_AVB_AUDIO_OUTPUT_DEVICES = List.of(AUDIO_OUTPUT_DEVICE_HDMI_ARC, - AUDIO_OUTPUT_DEVICE_HDMI_EARC); + AUDIO_OUTPUT_DEVICE_HDMI_EARC, + AUDIO_OUTPUT_DEVICE_LINE_DIGITAL); // Audio output devices used for absolute volume behavior on Playback devices private static final List<AudioDeviceAttributes> PLAYBACK_AVB_AUDIO_OUTPUT_DEVICES = diff --git a/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java b/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java index ccb9e3ea5cbe..bbf7732c9596 100644 --- a/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java +++ b/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java @@ -33,6 +33,7 @@ import android.hardware.contexthub.MessageDeliveryStatus; import android.hardware.contexthub.Reason; import android.hardware.location.ContextHubTransaction; import android.hardware.location.IContextHubTransactionCallback; +import android.hardware.location.NanoAppState; import android.os.Binder; import android.os.IBinder; import android.os.PowerManager; @@ -48,6 +49,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -182,8 +184,11 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub long expiryMillis = RELIABLE_MESSAGE_DUPLICATE_DETECTION_TIMEOUT.toMillis(); if (nowMillis >= nextEntry.getValue() + expiryMillis) { iterator.remove(); + } else { + // Safe to break since LinkedHashMap is insertion-ordered, so the next entry + // will have a later timestamp and will not be expired. + break; } - break; } return mRxMessageHistoryMap.containsKey(message.getMessageSequenceNumber()); @@ -276,6 +281,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub int sessionId = mEndpointManager.reserveSessionId(); EndpointInfo halEndpointInfo = ContextHubServiceUtil.convertHalEndpointInfo(destination); + Log.d(TAG, "openSession: sessionId=" + sessionId); synchronized (mOpenSessionLock) { try { @@ -301,6 +307,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub throw new IllegalArgumentException( "Unknown session ID in closeSession: id=" + sessionId); } + Log.d(TAG, "closeSession: sessionId=" + sessionId + " reason=" + reason); mEndpointManager.halCloseEndpointSession( sessionId, ContextHubServiceUtil.toHalReason(reason)); } @@ -373,12 +380,43 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub try { mHubInterface.sendMessageToEndpoint(sessionId, halMessage); } catch (RemoteException e) { - Log.w(TAG, "Exception while sending message on session " + sessionId, e); + Log.e( + TAG, + "Exception while sending message on session " + + sessionId + + ", closing session", + e); + notifySessionClosedToBoth(sessionId, Reason.UNSPECIFIED); } } else { + IContextHubTransactionCallback wrappedCallback = + new IContextHubTransactionCallback.Stub() { + @Override + public void onQueryResponse(int result, List<NanoAppState> appStates) + throws RemoteException { + Log.w(TAG, "Unexpected onQueryResponse callback"); + } + + @Override + public void onTransactionComplete(int result) throws RemoteException { + callback.onTransactionComplete(result); + if (result != ContextHubTransaction.RESULT_SUCCESS) { + Log.e( + TAG, + "Failed to send reliable message " + + message + + ", closing session"); + notifySessionClosedToBoth(sessionId, Reason.UNSPECIFIED); + } + } + }; ContextHubServiceTransaction transaction = mTransactionManager.createSessionMessageTransaction( - mHubInterface, sessionId, halMessage, mPackageName, callback); + mHubInterface, + sessionId, + halMessage, + mPackageName, + wrappedCallback); try { mTransactionManager.addTransaction(transaction); info.setReliableMessagePending(transaction.getMessageSequenceNumber()); @@ -445,10 +483,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub int id = mSessionMap.keyAt(i); HubEndpointInfo target = mSessionMap.get(id).getRemoteEndpointInfo(); if (!hasEndpointPermissions(target)) { - mEndpointManager.halCloseEndpointSessionNoThrow( - id, Reason.PERMISSION_DENIED); - onCloseEndpointSession(id, Reason.PERMISSION_DENIED); - // Resource cleanup is done in onCloseEndpointSession + notifySessionClosedToBoth(id, Reason.PERMISSION_DENIED); } } } @@ -532,8 +567,17 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub /* package */ void onMessageReceived(int sessionId, HubMessage message) { byte errorCode = onMessageReceivedInternal(sessionId, message); - if (errorCode != ErrorCode.OK && message.isResponseRequired()) { - sendMessageDeliveryStatus(sessionId, message.getMessageSequenceNumber(), errorCode); + if (errorCode != ErrorCode.OK) { + Log.e(TAG, "Failed to send message to endpoint: " + message + ", closing session"); + if (message.isResponseRequired()) { + sendMessageDeliveryStatus(sessionId, message.getMessageSequenceNumber(), errorCode); + } else { + notifySessionClosedToBoth( + sessionId, + (errorCode == ErrorCode.PERMISSION_DENIED) + ? Reason.PERMISSION_DENIED + : Reason.UNSPECIFIED); + } } } @@ -800,4 +844,16 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub + "-0x" + Long.toHexString(endpoint.getIdentifier().getEndpoint())); } + + /** + * Notifies to both the HAL and the app that a session has been closed. + * + * @param sessionId The ID of the session that was closed + * @param halReason The HAL reason for closing the session + */ + private void notifySessionClosedToBoth(int sessionId, byte halReason) { + Log.d(TAG, "notifySessionClosedToBoth: sessionId=" + sessionId + ", reason=" + halReason); + mEndpointManager.halCloseEndpointSessionNoThrow(sessionId, halReason); + onCloseEndpointSession(sessionId, halReason); + } } diff --git a/services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java b/services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java index 43b1ec393bf6..87cd1560509c 100644 --- a/services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java +++ b/services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java @@ -19,7 +19,9 @@ package com.android.server.location.contexthub; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; @@ -29,6 +31,7 @@ import static org.mockito.Mockito.when; import android.content.Context; import android.hardware.contexthub.EndpointInfo; import android.hardware.contexthub.ErrorCode; +import android.hardware.contexthub.HubEndpoint; import android.hardware.contexthub.HubEndpointInfo; import android.hardware.contexthub.HubEndpointInfo.HubEndpointIdentifier; import android.hardware.contexthub.HubMessage; @@ -385,6 +388,49 @@ public class ContextHubEndpointTest { unregisterExampleEndpoint(endpoint); } + @Test + public void testUnreliableMessageFailureClosesSession() throws RemoteException { + IContextHubEndpoint endpoint = registerExampleEndpoint(); + int sessionId = openTestSession(endpoint); + + doThrow(new RemoteException("Intended exception in test")) + .when(mMockCallback) + .onMessageReceived(anyInt(), any(HubMessage.class)); + mEndpointManager.onMessageReceived(sessionId, SAMPLE_UNRELIABLE_MESSAGE); + ArgumentCaptor<HubMessage> messageCaptor = ArgumentCaptor.forClass(HubMessage.class); + verify(mMockCallback).onMessageReceived(eq(sessionId), messageCaptor.capture()); + assertThat(messageCaptor.getValue()).isEqualTo(SAMPLE_UNRELIABLE_MESSAGE); + + verify(mMockEndpointCommunications).closeEndpointSession(sessionId, Reason.UNSPECIFIED); + verify(mMockCallback).onSessionClosed(sessionId, HubEndpoint.REASON_FAILURE); + assertThat(mEndpointManager.getNumAvailableSessions()).isEqualTo(SESSION_ID_RANGE); + + unregisterExampleEndpoint(endpoint); + } + + @Test + public void testSendUnreliableMessageFailureClosesSession() throws RemoteException { + IContextHubEndpoint endpoint = registerExampleEndpoint(); + int sessionId = openTestSession(endpoint); + + doThrow(new RemoteException("Intended exception in test")) + .when(mMockEndpointCommunications) + .sendMessageToEndpoint(anyInt(), any(Message.class)); + endpoint.sendMessage(sessionId, SAMPLE_UNRELIABLE_MESSAGE, /* callback= */ null); + ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class); + verify(mMockEndpointCommunications) + .sendMessageToEndpoint(eq(sessionId), messageCaptor.capture()); + Message message = messageCaptor.getValue(); + assertThat(message.type).isEqualTo(SAMPLE_UNRELIABLE_MESSAGE.getMessageType()); + assertThat(message.content).isEqualTo(SAMPLE_UNRELIABLE_MESSAGE.getMessageBody()); + + verify(mMockEndpointCommunications).closeEndpointSession(sessionId, Reason.UNSPECIFIED); + verify(mMockCallback).onSessionClosed(sessionId, HubEndpoint.REASON_FAILURE); + assertThat(mEndpointManager.getNumAvailableSessions()).isEqualTo(SESSION_ID_RANGE); + + unregisterExampleEndpoint(endpoint); + } + /** A helper method to create a session and validates reliable message sending. */ private void testMessageTransactionInternal( IContextHubEndpoint endpoint, boolean deliverMessageStatus) throws RemoteException { diff --git a/tools/aapt2/ResourcesInternal.proto b/tools/aapt2/ResourcesInternal.proto index f4735a2f6ce7..380c5f21103c 100644 --- a/tools/aapt2/ResourcesInternal.proto +++ b/tools/aapt2/ResourcesInternal.proto @@ -50,8 +50,11 @@ message CompiledFile { // Any symbols this file auto-generates/exports (eg. @+id/foo in an XML file). repeated Symbol exported_symbol = 5; - // The status of the flag the file is behind if any + // The status of the read only flag the file is behind if any uint32 flag_status = 6; bool flag_negated = 7; string flag_name = 8; + + // Whether the file uses read/write feature flags + bool uses_readwrite_feature_flags = 9; } diff --git a/tools/aapt2/cmd/Compile.cpp b/tools/aapt2/cmd/Compile.cpp index a5e18d35a256..3b4f5429f254 100644 --- a/tools/aapt2/cmd/Compile.cpp +++ b/tools/aapt2/cmd/Compile.cpp @@ -407,6 +407,45 @@ static bool IsValidFile(IAaptContext* context, const std::string& input_path) { return true; } +class FindReadWriteFlagsVisitor : public xml::Visitor { + public: + FindReadWriteFlagsVisitor(const FeatureFlagValues& feature_flag_values) + : feature_flag_values_(feature_flag_values) { + } + + void Visit(xml::Element* node) override { + if (had_flags_) { + return; + } + auto* attr = node->FindAttribute(xml::kSchemaAndroid, xml::kAttrFeatureFlag); + if (attr != nullptr) { + std::string_view flag_name = util::TrimWhitespace(attr->value); + if (flag_name.starts_with('!')) { + flag_name = flag_name.substr(1); + } + if (auto it = feature_flag_values_.find(flag_name); it != feature_flag_values_.end()) { + if (!it->second.read_only) { + had_flags_ = true; + return; + } + } else { + // Flag not passed to aapt2, must evaluate at runtime + had_flags_ = true; + return; + } + } + VisitChildren(node); + } + + bool HadFlags() const { + return had_flags_; + } + + private: + bool had_flags_ = false; + const FeatureFlagValues& feature_flag_values_; +}; + static bool CompileXml(IAaptContext* context, const CompileOptions& options, const ResourcePathData& path_data, io::IFile* file, IArchiveWriter* writer, const std::string& output_path) { @@ -436,6 +475,10 @@ static bool CompileXml(IAaptContext* context, const CompileOptions& options, xmlres->file.type = ResourceFile::Type::kProtoXml; xmlres->file.flag = ParseFlag(path_data.flag_name); + FindReadWriteFlagsVisitor visitor(options.feature_flag_values); + xmlres->root->Accept(&visitor); + xmlres->file.uses_readwrite_feature_flags = visitor.HadFlags(); + if (xmlres->file.flag) { std::string error; auto flag_status = GetFlagStatus(xmlres->file.flag, options.feature_flag_values, &error); diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 755dbb6f8e42..0e18ee250993 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -615,6 +615,8 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv file_op.xml_to_flatten->file.source = file_ref->GetSource(); file_op.xml_to_flatten->file.name = ResourceName(pkg->name, type->named_type, entry->name); + file_op.xml_to_flatten->file.uses_readwrite_feature_flags = + config_value->uses_readwrite_feature_flags; } // NOTE(adamlesinski): Explicitly construct a StringPiece here, or @@ -647,6 +649,17 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv } } + FeatureFlagsFilterOptions flags_filter_options; + // Don't fail on unrecognized flags or flags without values as these flags might be + // defined and have a value by the time they are evaluated at runtime. + flags_filter_options.fail_on_unrecognized_flags = false; + flags_filter_options.flags_must_have_value = false; + flags_filter_options.remove_disabled_elements = true; + FeatureFlagsFilter flags_filter(options_.feature_flag_values, flags_filter_options); + if (!flags_filter.Consume(context_, file_op.xml_to_flatten.get())) { + return 1; + } + std::vector<std::unique_ptr<xml::XmlResource>> versioned_docs = LinkAndVersionXmlFile(table, &file_op); if (versioned_docs.empty()) { @@ -673,6 +686,7 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv // Update the output format of this XML file. file_ref->type = XmlFileTypeForOutputFormat(options_.output_format); + bool result = table->AddResource( NewResourceBuilder(file.name) .SetValue(std::move(file_ref), file.config) @@ -685,14 +699,6 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv } } - FeatureFlagsFilterOptions flags_filter_options; - flags_filter_options.fail_on_unrecognized_flags = false; - flags_filter_options.flags_must_have_value = false; - FeatureFlagsFilter flags_filter(options_.feature_flag_values, flags_filter_options); - if (!flags_filter.Consume(context_, doc.get())) { - return 1; - } - error |= !FlattenXml(context_, *doc, dst_path, options_.keep_raw_values, false /*utf16*/, options_.output_format, archive_writer); } diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp index 91ec3485ac3b..b8936553a193 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -640,6 +640,7 @@ bool DeserializeCompiledFileFromPb(const pb::internal::CompiledFile& pb_file, out_file->name = name_ref.ToResourceName(); out_file->source.path = pb_file.source_path(); out_file->type = DeserializeFileReferenceTypeFromPb(pb_file.type()); + out_file->uses_readwrite_feature_flags = pb_file.uses_readwrite_feature_flags(); out_file->flag_status = (FlagStatus)pb_file.flag_status(); if (!pb_file.flag_name().empty()) { diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp index fcc77d5a9d6d..da99c4f5917c 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -767,6 +767,7 @@ void SerializeCompiledFileToPb(const ResourceFile& file, pb::internal::CompiledF out_file->set_flag_negated(file.flag->negated); out_file->set_flag_name(file.flag->name); } + out_file->set_uses_readwrite_feature_flags(file.uses_readwrite_feature_flags); for (const SourcedResourceName& exported : file.exported_symbols) { pb::internal::CompiledFile_Symbol* pb_symbol = out_file->add_exported_symbol(); diff --git a/tools/aapt2/link/FlaggedResources_test.cpp b/tools/aapt2/link/FlaggedResources_test.cpp index 47a71fe36e9f..4dcb8507fa45 100644 --- a/tools/aapt2/link/FlaggedResources_test.cpp +++ b/tools/aapt2/link/FlaggedResources_test.cpp @@ -226,9 +226,11 @@ TEST_F(FlaggedResourcesTest, ReadWriteFlagInXmlGetsFlagged) { } } } + ASSERT_TRUE(found) << "No entry for layout1 at v36 with FLAG_USES_FEATURE_FLAGS bit set"; - // There should only be 1 entry that has the FLAG_USES_FEATURE_FLAGS bit of flags set to 1 - ASSERT_EQ(fields_flagged, 1); + // There should only be 2 entry that has the FLAG_USES_FEATURE_FLAGS bit of flags set to 1, the + // three versions of the layout file that has flags + ASSERT_EQ(fields_flagged, 3); } } // namespace aapt diff --git a/tools/aapt2/link/FlaggedXmlVersioner.cpp b/tools/aapt2/link/FlaggedXmlVersioner.cpp index 8a3337c446cb..626cae73bfa2 100644 --- a/tools/aapt2/link/FlaggedXmlVersioner.cpp +++ b/tools/aapt2/link/FlaggedXmlVersioner.cpp @@ -35,10 +35,6 @@ class AllDisabledFlagsVisitor : public xml::Visitor { VisitChildren(node); } - bool HadFlags() const { - return had_flags_; - } - private: bool FixupOrShouldRemove(const std::unique_ptr<xml::Node>& node) { if (auto* el = NodeCast<Element>(node.get())) { @@ -47,7 +43,6 @@ class AllDisabledFlagsVisitor : public xml::Visitor { return false; } - had_flags_ = true; // This class assumes all flags are disabled so we want to remove any elements behind flags // unless the flag specification is negated. In the negated case we remove the featureFlag // attribute because we have already determined whether we are keeping the element or not. @@ -62,56 +57,27 @@ class AllDisabledFlagsVisitor : public xml::Visitor { return false; } - - bool had_flags_ = false; -}; - -// An xml visitor that goes through the a doc and determines if any elements are behind a flag. -class FindFlagsVisitor : public xml::Visitor { - public: - void Visit(xml::Element* node) override { - if (had_flags_) { - return; - } - auto* attr = node->FindAttribute(xml::kSchemaAndroid, xml::kAttrFeatureFlag); - if (attr != nullptr) { - had_flags_ = true; - return; - } - VisitChildren(node); - } - - bool HadFlags() const { - return had_flags_; - } - - bool had_flags_ = false; }; std::vector<std::unique_ptr<xml::XmlResource>> FlaggedXmlVersioner::Process(IAaptContext* context, xml::XmlResource* doc) { std::vector<std::unique_ptr<xml::XmlResource>> docs; - if ((static_cast<ApiVersion>(doc->file.config.sdkVersion) >= SDK_BAKLAVA) || - (static_cast<ApiVersion>(context->GetMinSdkVersion()) >= SDK_BAKLAVA)) { + if (!doc->file.uses_readwrite_feature_flags) { + docs.push_back(doc->Clone()); + } else if ((static_cast<ApiVersion>(doc->file.config.sdkVersion) >= SDK_BAKLAVA) || + (static_cast<ApiVersion>(context->GetMinSdkVersion()) >= SDK_BAKLAVA)) { // Support for read/write flags was added in baklava so if the doc will only get used on // baklava or later we can just return the original doc. docs.push_back(doc->Clone()); - FindFlagsVisitor visitor; - doc->root->Accept(&visitor); - docs.back()->file.uses_readwrite_feature_flags = visitor.HadFlags(); } else { auto preBaklavaVersion = doc->Clone(); AllDisabledFlagsVisitor visitor; preBaklavaVersion->root->Accept(&visitor); - preBaklavaVersion->file.uses_readwrite_feature_flags = false; docs.push_back(std::move(preBaklavaVersion)); - if (visitor.HadFlags()) { - auto baklavaVersion = doc->Clone(); - baklavaVersion->file.config.sdkVersion = SDK_BAKLAVA; - baklavaVersion->file.uses_readwrite_feature_flags = true; - docs.push_back(std::move(baklavaVersion)); - } + auto baklavaVersion = doc->Clone(); + baklavaVersion->file.config.sdkVersion = SDK_BAKLAVA; + docs.push_back(std::move(baklavaVersion)); } return docs; } diff --git a/tools/aapt2/link/FlaggedXmlVersioner_test.cpp b/tools/aapt2/link/FlaggedXmlVersioner_test.cpp index 0c1314f165cc..0dc464253385 100644 --- a/tools/aapt2/link/FlaggedXmlVersioner_test.cpp +++ b/tools/aapt2/link/FlaggedXmlVersioner_test.cpp @@ -101,6 +101,7 @@ TEST_F(FlaggedXmlVersionerTest, PreBaklavaGetsSplit) { <TextView android:featureFlag="package.flag" /><TextView /><TextView /> </LinearLayout>)"); doc->file.config.sdkVersion = SDK_GINGERBREAD; + doc->file.uses_readwrite_feature_flags = true; FlaggedXmlVersioner versioner; auto results = versioner.Process(context_.get(), doc.get()); @@ -131,6 +132,7 @@ TEST_F(FlaggedXmlVersionerTest, NoVersionGetsSplit) { <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"> <TextView android:featureFlag="package.flag" /><TextView /><TextView /> </LinearLayout>)"); + doc->file.uses_readwrite_feature_flags = true; FlaggedXmlVersioner versioner; auto results = versioner.Process(context_.get(), doc.get()); @@ -162,6 +164,7 @@ TEST_F(FlaggedXmlVersionerTest, NegatedFlagAttributeRemoved) { <TextView android:featureFlag="!package.flag" /><TextView /><TextView /> </LinearLayout>)"); doc->file.config.sdkVersion = SDK_GINGERBREAD; + doc->file.uses_readwrite_feature_flags = true; FlaggedXmlVersioner versioner; auto results = versioner.Process(context_.get(), doc.get()); @@ -192,6 +195,7 @@ TEST_F(FlaggedXmlVersionerTest, NegatedFlagAttributeRemovedNoSpecifiedVersion) { <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"> <TextView android:featureFlag="!package.flag" /><TextView /><TextView /> </LinearLayout>)"); + doc->file.uses_readwrite_feature_flags = true; FlaggedXmlVersioner versioner; auto results = versioner.Process(context_.get(), doc.get()); diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index 1d4adc4a57d8..17f332397317 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -295,6 +295,8 @@ bool TableMerger::DoMerge(const android::Source& src, ResourceTablePackage* src_ dst_config_value = dst_entry->FindOrCreateValue(src_config_value->config, src_config_value->product); } + dst_config_value->uses_readwrite_feature_flags |= + src_config_value->uses_readwrite_feature_flags; // Continue if we're taking the new resource. CloningValueTransformer cloner(&main_table_->string_pool); @@ -378,12 +380,13 @@ bool TableMerger::MergeFile(const ResourceFile& file_desc, bool overlay, io::IFi file_ref->file = file; file_ref->SetFlagStatus(file_desc.flag_status); file_ref->SetFlag(file_desc.flag); - ResourceTablePackage* pkg = table.FindOrCreatePackage(file_desc.name.package); - pkg->FindOrCreateType(file_desc.name.type) - ->FindOrCreateEntry(file_desc.name.entry) - ->FindOrCreateValue(file_desc.config, {}) - ->value = std::move(file_ref); + ResourceConfigValue* config_value = pkg->FindOrCreateType(file_desc.name.type) + ->FindOrCreateEntry(file_desc.name.entry) + ->FindOrCreateValue(file_desc.config, {}); + + config_value->value = std::move(file_ref); + config_value->uses_readwrite_feature_flags = file_desc.uses_readwrite_feature_flags; return DoMerge(file->GetSource(), pkg, false /*mangle*/, overlay /*overlay*/, true /*allow_new*/); } |