From ab3f4e2a0c898e1a7bbf9a03b4392a809109ee8e Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Fri, 13 Sep 2024 20:54:00 +0200 Subject: [PATCH] Fix failing privacy tests (#5010) Task/Issue URL: https://app.asana.com/0/1202552961248957/1208263793923549/f ### Description ### Steps to test this PR _Feature 1_ - [ ] - [ ] ### UI changes | Before | After | | ------ | ----- | !(Upload before screenshot)|(Upload after screenshot)| --- .../app/browser/BrowserTabViewModelTest.kt | 34 +++++++++---------- .../app/browser/BrowserTabFragment.kt | 8 +++-- .../app/browser/BrowserTabViewModel.kt | 5 +-- .../duckplayer/impl/DuckPlayerDataStore.kt | 2 +- .../impl/DuckPlayerFeatureRepository.kt | 10 ++++-- 5 files changed, 35 insertions(+), 24 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 5984899c4c83..2d3aea3935dc 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -4828,7 +4828,7 @@ class BrowserTabViewModelTest { "webShare", "myId", JSONObject("""{ "my":"object"}"""), - "someUrl", + { "someUrl" }, ) assertCommandIssued { assertEquals("object", this.data.params.getString("my")) @@ -4848,7 +4848,7 @@ class BrowserTabViewModelTest { "permissionsQuery", "myId", JSONObject("""{ "name":"somePermission"}"""), - "someUrl", + { "someUrl" }, ) assertCommandIssued { assertEquals("granted", this.data.params.getString("state")) @@ -4866,7 +4866,7 @@ class BrowserTabViewModelTest { "screenLock", "myId", JSONObject("""{ "my":"object"}"""), - "someUrl", + { "someUrl" }, ) assertCommandNotIssued() } @@ -4879,7 +4879,7 @@ class BrowserTabViewModelTest { "screenLock", "myId", JSONObject("""{ "my":"object"}"""), - "someUrl", + { "someUrl" }, ) assertCommandIssued { assertEquals("object", this.data.params.getString("my")) @@ -4897,7 +4897,7 @@ class BrowserTabViewModelTest { "screenUnlock", "myId", JSONObject("""{ "my":"object"}"""), - "someUrl", + { "someUrl" }, ) assertCommandNotIssued() } @@ -4910,7 +4910,7 @@ class BrowserTabViewModelTest { "screenUnlock", "myId", JSONObject("""{ "my":"object"}"""), - "someUrl", + { "someUrl" }, ) assertCommandIssued() } @@ -4924,7 +4924,7 @@ class BrowserTabViewModelTest { "getUserValues", "id", data = null, - "someUrl", + { "someUrl" }, ) assertCommandIssued() } @@ -4938,7 +4938,7 @@ class BrowserTabViewModelTest { "setUserValues", "id", JSONObject("""{ overlayInteracted: "true", privatePlayerMode: {disabled: {} }}"""), - "someUrl", + { "someUrl" }, ) assertCommandIssued() verify(mockDuckPlayer).setUserPreferences(any(), any()) @@ -4954,7 +4954,7 @@ class BrowserTabViewModelTest { "setUserValues", "id", JSONObject("""{ overlayInteracted: "true", privatePlayerMode: {enabled: {} }}"""), - "someUrl", + { "someUrl" }, ) assertCommandIssued() verify(mockDuckPlayer).setUserPreferences(any(), any()) @@ -4970,7 +4970,7 @@ class BrowserTabViewModelTest { "setUserValues", "id", JSONObject("""{ overlayInteracted: "true", privatePlayerMode: {enabled: {} }}"""), - "someUrl", + { "someUrl" }, ) assertCommandIssued() verify(mockDuckPlayer).setUserPreferences(true, "enabled") @@ -4986,7 +4986,7 @@ class BrowserTabViewModelTest { "sendDuckPlayerPixel", "id", JSONObject("""{ pixelName: "pixel", params: {}}"""), - "someUrl", + { "someUrl" }, ) verify(mockDuckPlayer).sendDuckPlayerPixel("pixel", mapOf()) } @@ -5000,7 +5000,7 @@ class BrowserTabViewModelTest { "openDuckPlayer", "id", JSONObject("""{ href: "duck://player/1234" }"""), - "someUrl", + { "someUrl" }, ) assertCommandIssued() } @@ -5014,7 +5014,7 @@ class BrowserTabViewModelTest { "openDuckPlayer", "id", null, - "someUrl", + { "someUrl" }, ) assertCommandNotIssued() } @@ -5028,7 +5028,7 @@ class BrowserTabViewModelTest { "initialSetup", "id", null, - "someUrl", + { "someUrl" }, ) assertCommandIssued() } @@ -5042,7 +5042,7 @@ class BrowserTabViewModelTest { "initialSetup", "id", null, - "someUrl", + { "someUrl" }, ) assertCommandIssued() } @@ -5055,7 +5055,7 @@ class BrowserTabViewModelTest { "openSettings", "id", null, - "someUrl", + { "someUrl" }, ) assertCommandIssued() } @@ -5070,7 +5070,7 @@ class BrowserTabViewModelTest { "openInfo", "id", null, - "someUrl", + { "someUrl" }, ) assertCommandIssued() } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 60ee7a94d700..8cbb58953a87 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -2429,7 +2429,9 @@ class BrowserTabFragment : id: String?, data: JSONObject?, ) { - viewModel.processJsCallbackMessage(featureName, method, id, data, it.url) + viewModel.processJsCallbackMessage(featureName, method, id, data) { + it.url + } } }, ) @@ -2442,7 +2444,9 @@ class BrowserTabFragment : id: String?, data: JSONObject?, ) { - viewModel.processJsCallbackMessage(featureName, method, id, data, it.url) + viewModel.processJsCallbackMessage(featureName, method, id, data) { + it.url + } } }, ) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index a393d47955f4..cd6e01d32209 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -3172,7 +3172,7 @@ class BrowserTabViewModel @Inject constructor( method: String, id: String?, data: JSONObject?, - url: String?, + getWebViewUrl: () -> String?, ) { when (method) { "webShare" -> if (id != null && data != null) { @@ -3201,7 +3201,8 @@ class BrowserTabViewModel @Inject constructor( when (featureName) { DUCK_PLAYER_FEATURE_NAME, DUCK_PLAYER_PAGE_FEATURE_NAME -> { viewModelScope.launch(dispatchers.io()) { - val response = duckPlayerJSHelper.processJsCallbackMessage(featureName, method, id, data, url) + val webViewUrl = withContext(dispatchers.main()) { getWebViewUrl() } + val response = duckPlayerJSHelper.processJsCallbackMessage(featureName, method, id, data, webViewUrl) withContext(dispatchers.main()) { response?.let { command.value = it diff --git a/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerDataStore.kt b/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerDataStore.kt index 14227f3c917a..4b563f441e7d 100644 --- a/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerDataStore.kt +++ b/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerDataStore.kt @@ -116,7 +116,7 @@ class SharedPreferencesDuckPlayerDataStore @Inject constructor( private val duckPlayerRC: Flow get() = store.data .map { prefs -> - prefs[DUCK_PLAYER_RC] ?: "" + prefs[DUCK_PLAYER_RC] ?: "{}" } .distinctUntilChanged() diff --git a/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerFeatureRepository.kt b/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerFeatureRepository.kt index 139a38e1a986..12f134235bdf 100644 --- a/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerFeatureRepository.kt +++ b/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerFeatureRepository.kt @@ -17,6 +17,7 @@ package com.duckduckgo.duckplayer.impl import com.duckduckgo.app.di.AppCoroutineScope +import com.duckduckgo.app.di.IsMainProcess import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.duckplayer.api.DuckPlayer.UserPreferences @@ -24,6 +25,7 @@ import com.duckduckgo.duckplayer.api.PrivatePlayerMode.AlwaysAsk import com.duckduckgo.duckplayer.api.PrivatePlayerMode.Disabled import com.duckduckgo.duckplayer.api.PrivatePlayerMode.Enabled import com.squareup.anvil.annotations.ContributesBinding +import dagger.SingleInstanceIn import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow @@ -68,17 +70,21 @@ interface DuckPlayerFeatureRepository { suspend fun setUserOnboarded() } +@SingleInstanceIn(AppScope::class) @ContributesBinding(AppScope::class) class RealDuckPlayerFeatureRepository @Inject constructor( private val duckPlayerDataStore: DuckPlayerDataStore, @AppCoroutineScope private val appCoroutineScope: CoroutineScope, private val dispatcherProvider: DispatcherProvider, + @IsMainProcess isMainProcess: Boolean, ) : DuckPlayerFeatureRepository { - private var duckPlayerRC = "" + private var duckPlayerRC = "{}" init { - loadToMemory() + if (isMainProcess) { + loadToMemory() + } } private fun loadToMemory() {