Merge "Add a open isPageEnabled() to BrowseActivity" into udc-dev

packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/BrowseActivity.kt

@@ -20,6 +20,7 @@ package com.android.settingslib.spa.framework
 
 import android.content.Intent
 import android.os.Bundle
+import android.util.Log
 import androidx.activity.ComponentActivity
 import androidx.activity.compose.setContent
 import androidx.annotation.VisibleForTesting
@@ -40,6 +41,7 @@ import androidx.navigation.NavGraph.Companion.findStartDestination
 import com.android.settingslib.spa.R
 import com.android.settingslib.spa.framework.common.LogCategory
 import com.android.settingslib.spa.framework.common.NullPageProvider
+import com.android.settingslib.spa.framework.common.SettingsPage
 import com.android.settingslib.spa.framework.common.SettingsPageProvider
 import com.android.settingslib.spa.framework.common.SettingsPageProviderRepository
 import com.android.settingslib.spa.framework.common.SpaEnvironmentFactory
@@ -51,7 +53,7 @@ import com.android.settingslib.spa.framework.compose.composable
 import com.android.settingslib.spa.framework.compose.localNavController
 import com.android.settingslib.spa.framework.compose.rememberAnimatedNavController
 import com.android.settingslib.spa.framework.theme.SettingsTheme
-import com.android.settingslib.spa.framework.util.PageWithEvent
+import com.android.settingslib.spa.framework.util.PageLogger
 import com.android.settingslib.spa.framework.util.getDestination
 import com.android.settingslib.spa.framework.util.getEntryId
 import com.android.settingslib.spa.framework.util.getSessionName
@@ -87,25 +89,50 @@ open class BrowseActivity : ComponentActivity() {
         setContent {
             SettingsTheme {
                 val sppRepository by spaEnvironment.pageProviderRepository
-                BrowseContent(sppRepository, intent)
+                BrowseContent(
+                    sppRepository = sppRepository,
+                    isPageEnabled = ::isPageEnabled,
+                    initialIntent = intent,
+                )
             }
         }
     }
+
+    open fun isPageEnabled(page: SettingsPage) = page.isEnabled()
 }
 
 @VisibleForTesting
 @Composable
-fun BrowseContent(sppRepository: SettingsPageProviderRepository, initialIntent: Intent? = null) {
+internal fun BrowseContent(
+    sppRepository: SettingsPageProviderRepository,
+    isPageEnabled: (SettingsPage) -> Boolean,
+    initialIntent: Intent?,
+) {
     val navController = rememberAnimatedNavController()
     CompositionLocalProvider(navController.localNavController()) {
         val controller = LocalNavController.current as NavControllerWrapperImpl
-        controller.NavContent(sppRepository.getAllProviders())
+        controller.NavContent(sppRepository.getAllProviders()) { page ->
+            if (remember { isPageEnabled(page) }) {
+                LaunchedEffect(Unit) {
+                    Log.d(TAG, "Launching page ${page.sppName}")
+                }
+                page.PageLogger()
+                page.UiLayout()
+            } else {
+                LaunchedEffect(Unit) {
+                    controller.navigateBack()
+                }
+            }
+        }
         controller.InitialDestination(initialIntent, sppRepository.getDefaultStartPage())
     }
 }
 
 @Composable
-private fun NavControllerWrapperImpl.NavContent(allProvider: Collection<SettingsPageProvider>) {
+private fun NavControllerWrapperImpl.NavContent(
+    allProvider: Collection<SettingsPageProvider>,
+    content: @Composable (SettingsPage) -> Unit,
+) {
     AnimatedNavHost(
         navController = navController,
         startDestination = NullPageProvider.name,
@@ -139,7 +166,7 @@ private fun NavControllerWrapperImpl.NavContent(allProvider: Collection<Settings
                 },
             ) { navBackStackEntry ->
                 val page = remember { spp.createSettingsPage(navBackStackEntry.arguments) }
-                page.PageWithEvent()
+                content(page)
             }
         }
     }

packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/util/PageLogger.kt

@@ -29,14 +29,12 @@ import com.android.settingslib.spa.framework.compose.LocalNavController
 import com.android.settingslib.spa.framework.compose.NavControllerWrapper
 
 @Composable
-internal fun SettingsPage.PageWithEvent() {
-    if (!isEnabled()) return
+internal fun SettingsPage.PageLogger() {
     val navController = LocalNavController.current
     LifecycleEffect(
         onStart = { logPageEvent(LogEvent.PAGE_ENTER, navController) },
         onStop = { logPageEvent(LogEvent.PAGE_LEAVE, navController) },
     )
-    UiLayout()
 }
 
 private fun SettingsPage.logPageEvent(event: LogEvent, navController: NavControllerWrapper) {

packages/SettingsLib/Spa/tests/src/com/android/settingslib/spa/framework/BrowseActivityTest.kt

@@ -26,6 +26,7 @@ import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.android.settingslib.spa.framework.common.LogCategory
 import com.android.settingslib.spa.framework.common.LogEvent
+import com.android.settingslib.spa.framework.common.SettingsPage
 import com.android.settingslib.spa.framework.common.SpaEnvironmentFactory
 import com.android.settingslib.spa.framework.common.createSettingsPage
 import com.android.settingslib.spa.tests.testutils.SpaEnvironmentForTest
@@ -38,8 +39,6 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 
-const val WAIT_UNTIL_TIMEOUT = 1000L
-
 @RunWith(AndroidJUnit4::class)
 class BrowseActivityTest {
     @get:Rule
@@ -49,19 +48,26 @@ class BrowseActivityTest {
     private val spaLogger = SpaLoggerForTest()
 
     @Test
-    fun testBrowsePage() {
-        spaLogger.reset()
-        val spaEnvironment =
-            SpaEnvironmentForTest(context, listOf(SppHome.createSettingsPage()), logger = spaLogger)
+    fun browseContent_onNavigate_logPageEvent() {
+        val spaEnvironment = SpaEnvironmentForTest(
+            context = context,
+            rootPages = listOf(SppHome.createSettingsPage()),
+            logger = spaLogger,
+        )
         SpaEnvironmentFactory.reset(spaEnvironment)
-
         val sppRepository by spaEnvironment.pageProviderRepository
         val sppHome = sppRepository.getProviderOrNull("SppHome")!!
         val pageHome = sppHome.createSettingsPage()
         val sppLayer1 = sppRepository.getProviderOrNull("SppLayer1")!!
         val pageLayer1 = sppLayer1.createSettingsPage()
 
-        composeTestRule.setContent { BrowseContent(sppRepository) }
+        composeTestRule.setContent {
+            BrowseContent(
+                sppRepository = sppRepository,
+                isPageEnabled = SettingsPage::isEnabled,
+                initialIntent = null,
+            )
+        }
 
         composeTestRule.onNodeWithText(sppHome.getTitle(null)).assertIsDisplayed()
         spaLogger.verifyPageEvent(pageHome.id, 1, 0)
@@ -69,7 +75,7 @@ class BrowseActivityTest {
 
         // click to layer1 page
         composeTestRule.onNodeWithText("SppHome to Layer1").assertIsDisplayed().performClick()
-        waitUntil(WAIT_UNTIL_TIMEOUT) {
+        waitUntil {
             composeTestRule.onAllNodesWithText(sppLayer1.getTitle(null))
                 .fetchSemanticsNodes().size == 1
         }
@@ -78,18 +84,24 @@ class BrowseActivityTest {
     }
 
     @Test
-    fun testBrowseDisabledPage() {
-        spaLogger.reset()
+    fun browseContent_whenDisabled_noLogPageEvent() {
         val spaEnvironment = SpaEnvironmentForTest(
-            context, listOf(SppDisabled.createSettingsPage()), logger = spaLogger
+            context = context,
+            rootPages = listOf(SppDisabled.createSettingsPage()),
+            logger = spaLogger,
         )
         SpaEnvironmentFactory.reset(spaEnvironment)
-
         val sppRepository by spaEnvironment.pageProviderRepository
         val sppDisabled = sppRepository.getProviderOrNull("SppDisabled")!!
         val pageDisabled = sppDisabled.createSettingsPage()
 
-        composeTestRule.setContent { BrowseContent(sppRepository) }
+        composeTestRule.setContent {
+            BrowseContent(
+                sppRepository = sppRepository,
+                isPageEnabled = SettingsPage::isEnabled,
+                initialIntent = null,
+            )
+        }
 
         composeTestRule.onNodeWithText(sppDisabled.getTitle(null)).assertDoesNotExist()
         spaLogger.verifyPageEvent(pageDisabled.id, 0, 0)