From 207758b4b9465b76b164e9d01d750cdc831857b3 Mon Sep 17 00:00:00 2001 From: Domen Lanisnik Date: Thu, 17 Jul 2025 13:27:31 +0200 Subject: [PATCH 1/6] Add support for multiple profile queries in PirScan and PirOptOut --- .../pir/internal/common/BrokerStepsParser.kt | 21 +- .../pir/internal/common/PirActionsRunner.kt | 4 +- .../pir/internal/optout/PirOptOut.kt | 181 +++++++++++------- .../duckduckgo/pir/internal/scan/PirScan.kt | 131 +++++++++---- .../scripts/models/PirScriptProfile.kt | 2 +- .../pir/internal/store/PirRepository.kt | 54 ++++-- .../pir/internal/store/db/ScanResultsDao.kt | 2 +- 7 files changed, 248 insertions(+), 147 deletions(-) diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt index 6ff66aa5c8ca..a46be35eade9 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt @@ -23,6 +23,7 @@ import com.duckduckgo.pir.internal.common.BrokerStepsParser.BrokerStep.OptOutSte import com.duckduckgo.pir.internal.common.BrokerStepsParser.BrokerStep.ScanStep import com.duckduckgo.pir.internal.scripts.models.BrokerAction import com.duckduckgo.pir.internal.scripts.models.ExtractedProfile +import com.duckduckgo.pir.internal.scripts.models.ProfileQuery import com.duckduckgo.pir.internal.store.PirRepository import com.squareup.anvil.annotations.ContributesBinding import com.squareup.moshi.JsonAdapter @@ -40,12 +41,14 @@ interface BrokerStepsParser { * * @param brokerName - name of the broker to which these steps belong to * @param stepsJson - string in JSONObject format obtained from the broker's json representing a step (scan / opt-out). + * @param profileQuery - profile associated with the step (used for the opt-out step) * @return list of broker steps resulting from the passed params. If the step is of type OptOut, it will return a list of * OptOutSteps where an OptOut step is mapped to each of the profile for the broker. */ suspend fun parseStep( brokerName: String, stepsJson: String, + profileQuery: ProfileQuery? = null, ): List sealed class BrokerStep( @@ -101,16 +104,22 @@ class RealBrokerStepsParser @Inject constructor( override suspend fun parseStep( brokerName: String, stepsJson: String, + profileQuery: ProfileQuery?, ): List = withContext(dispatcherProvider.io()) { return@withContext runCatching { adapter.fromJson(stepsJson)?.run { if (this is OptOutStep) { - repository.getExtractProfileResultForBroker(brokerName)?.extractResults?.map { - this.copy( - brokerName = brokerName, - profileToOptOut = it, - ) - } + repository.getExtractProfileResultsForBroker(brokerName) + .filter { it.profileQuery != null && it.profileQuery.id == profileQuery?.id } + .map { + it.extractResults.map { extractedProfile -> + this.copy( + brokerName = brokerName, + profileToOptOut = extractedProfile, + ) + } + } + .flatten() } else { listOf((this as ScanStep).copy(brokerName = brokerName)) } diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt index 1aaa6e883fd2..84fc0ba63efa 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt @@ -145,8 +145,8 @@ class RealPirActionsRunner @AssistedInject constructor( } withContext(dispatcherProvider.main()) { - logcat { "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} Brokers to execute $brokerSteps" } - logcat { "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} Brokers size: ${brokerSteps.size}" } + logcat { "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} profile=$profileQuery Brokers to execute $brokerSteps" } + logcat { "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} profile=$profileQuery Brokers size: ${brokerSteps.size}" } detachedWebView = pirDetachedWebViewProvider.createInstance( context, pirScriptToLoad, diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt index ba5956d4994d..d146c2919668 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt @@ -101,21 +101,41 @@ class RealPirOptOut @Inject constructor( private val dispatcherProvider: DispatcherProvider, callbacks: PluginPoint, ) : PirOptOut, PirJob(callbacks) { - private var profileQuery: ProfileQuery = ProfileQuery( - firstName = "William", - lastName = "Smith", - city = "Chicago", - state = "IL", - addresses = listOf( - Address( - city = "Chicago", - state = "IL", + private var profileQueries: List = listOf( + ProfileQuery( + id = -1, + firstName = "William", + lastName = "Smith", + city = "Chicago", + state = "IL", + addresses = listOf( + Address( + city = "Chicago", + state = "IL", + ), ), + birthYear = 1993, + fullName = "William Smith", + age = 32, + deprecated = false, + ), + ProfileQuery( + id = -2, + firstName = "Jane", + lastName = "Doe", + city = "New York", + state = "NY", + addresses = listOf( + Address( + city = "New York", + state = "NY", + ), + ), + birthYear = 1990, + fullName = "Jane Doe", + age = 35, + deprecated = false, ), - birthYear = 1993, - fullName = "William Smith", - age = 32, - deprecated = false, ) private val runners: MutableList = mutableListOf() @@ -131,9 +151,9 @@ class RealPirOptOut @Inject constructor( cleanRunners() runners.clear() } - obtainProfile() + obtainProfiles() - logcat { "PIR-OPT-OUT: Running opt-out on profile: $profileQuery on ${Thread.currentThread().name}" } + logcat { "PIR-OPT-OUT: Running debug opt-out for $brokers on profiles: $profileQueries on ${Thread.currentThread().name}" } runners.add( pirActionsRunnerFactory.create( @@ -143,21 +163,28 @@ class RealPirOptOut @Inject constructor( ), ) - // Start each runner on a subset of the broker steps + // Load opt-out steps jsons for each broker + val brokerOptOutStepsJsons = brokers.mapNotNull { broker -> + repository.getBrokerOptOutSteps(broker)?.let { broker to it } + } - brokers.mapNotNull { broker -> - repository.getBrokerOptOutSteps(broker)?.run { - brokerStepsParser.parseStep(broker, this) - } - }.filter { - it.isNotEmpty() - }.flatten() - .also { list -> - runners[0].startOn(webView, profileQuery, list) - runners[0].stop() - } + // Create a map of profiles that have opt-out steps for those brokers + val profileBrokerStepsMap = profileQueries.associateWith { profileQuery -> + brokerOptOutStepsJsons.map { (broker, stepsJson) -> + brokerStepsParser.parseStep(broker, stepsJson, profileQuery) + }.flatten() + }.filter { it.value.isNotEmpty() } + + // Start each runner on the profile with the broker steps + profileBrokerStepsMap.entries.map { (profileQuery, brokerSteps) -> + logcat { "PIR-OPT-OUT: Start opt-out for profile: $profileQuery on ${Thread.currentThread().name}" } + runners[0].startOn(webView, profileQuery, brokerSteps) + runners[0].stop() + logcat { "PIR-OPT-OUT: Finish opt-out for profile: $profileQuery on ${Thread.currentThread().name}" } + } + + logcat { "PIR-OPT-OUT: Opt-out completed for all runners and profiles" } - logcat { "PIR-OPT-OUT: Optout completed for all runners" } emitCompletedPixel() onJobCompleted() return@withContext Result.success(Unit) @@ -173,26 +200,27 @@ class RealPirOptOut @Inject constructor( cleanRunners() runners.clear() } - obtainProfile() + obtainProfiles() - logcat { "PIR-OPT-OUT: Running opt-out on profile: $profileQuery on ${Thread.currentThread().name}" } + logcat { "PIR-OPT-OUT: Running opt-out on profiles: $profileQueries on ${Thread.currentThread().name}" } val script = pirCssScriptLoader.getScript() - val brokerSteps = brokers.mapNotNull { broker -> - repository.getBrokerOptOutSteps(broker)?.run { - brokerStepsParser.parseStep(broker, this) - } - }.filter { - it.isNotEmpty() - }.flatten() - - maxWebViewCount = if (brokerSteps.size <= MAX_DETACHED_WEBVIEW_COUNT) { - brokerSteps.size - } else { - MAX_DETACHED_WEBVIEW_COUNT + // Load opt-out steps jsons for each broker + val brokerOptOutStepsJsons = brokers.mapNotNull { broker -> + repository.getBrokerOptOutSteps(broker)?.let { broker to it } } + // Create a map of profiles that have opt-out steps for those brokers + val profileBrokerStepsMap = profileQueries.associateWith { profileQuery -> + brokerOptOutStepsJsons.map { (broker, stepsJson) -> + brokerStepsParser.parseStep(broker, stepsJson, profileQuery) + }.flatten() + }.filter { it.value.isNotEmpty() } + + val stepsCount = profileBrokerStepsMap.values.maxOf { it.size } + maxWebViewCount = minOf(stepsCount, MAX_DETACHED_WEBVIEW_COUNT) + logcat { "PIR-OPT-OUT: Attempting to create $maxWebViewCount parallel runners on ${Thread.currentThread().name}" } // Initiate runners @@ -208,45 +236,50 @@ class RealPirOptOut @Inject constructor( createCount++ } - // Start each runner on a subset of the broker steps - brokerSteps.splitIntoParts(maxWebViewCount) - .mapIndexed { index, part -> - async { - runners[index].start(profileQuery, part) - runners[index].stop() - } - }.awaitAll() + // Start each runner on the profile with the broker steps + profileBrokerStepsMap.entries.map { (profileQuery, brokerSteps) -> + logcat { "PIR-OPT-OUT: Start opt-out on profile: $profileQuery" } + brokerSteps.splitIntoParts(maxWebViewCount) + .mapIndexed { index, part -> + async { + runners[index].start(profileQuery, part) + runners[index].stop() + } + }.awaitAll() + logcat { "PIR-OPT-OUT: Finish opt-out on profile: $profileQuery" } + } - logcat { "PIR-OPT-OUT: Optout completed for all runners" } + logcat { "PIR-OPT-OUT: Opt-out completed for all runners and profiles" } emitCompletedPixel() onJobCompleted() return@withContext Result.success(Unit) } - private suspend fun obtainProfile() { - repository.getUserProfiles().also { - if (it.isNotEmpty()) { - // Temporarily taking the first profile only for the PoC. In the reality, more than 1 should be allowed. - val storedProfile = it[0] - profileQuery = ProfileQuery( - firstName = storedProfile.userName.firstName, - lastName = storedProfile.userName.lastName, - city = storedProfile.addresses.city, - state = storedProfile.addresses.state, - addresses = listOf( - Address( - city = storedProfile.addresses.city, - state = storedProfile.addresses.state, + private suspend fun obtainProfiles() { + repository.getUserProfiles().also { profiles -> + if (profiles.isNotEmpty()) { + profileQueries = profiles.map { storedProfile -> + ProfileQuery( + id = storedProfile.id, + firstName = storedProfile.userName.firstName, + lastName = storedProfile.userName.lastName, + city = storedProfile.addresses.city, + state = storedProfile.addresses.state, + addresses = listOf( + Address( + city = storedProfile.addresses.city, + state = storedProfile.addresses.state, + ), ), - ), - birthYear = storedProfile.birthYear, - fullName = storedProfile.userName.middleName?.run { - "${storedProfile.userName.firstName} $this ${storedProfile.userName.lastName}" - } - ?: "${storedProfile.userName.firstName} ${storedProfile.userName.lastName}", - age = LocalDate.now().year - storedProfile.birthYear, - deprecated = false, - ) + birthYear = storedProfile.birthYear, + fullName = storedProfile.userName.middleName?.run { + "${storedProfile.userName.firstName} $this ${storedProfile.userName.lastName}" + } + ?: "${storedProfile.userName.firstName} ${storedProfile.userName.lastName}", + age = LocalDate.now().year - storedProfile.birthYear, + deprecated = false, + ) + } } } } diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt index e4da5464abd1..573e39d0b967 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt @@ -96,21 +96,58 @@ class RealPirScan @Inject constructor( callbacks: PluginPoint, ) : PirScan, PirJob(callbacks) { - private var profileQuery: ProfileQuery = ProfileQuery( - firstName = "William", - lastName = "Smith", - city = "Chicago", - state = "IL", - addresses = listOf( - Address( - city = "Chicago", - state = "IL", + private var profileQueries: List = listOf( + ProfileQuery( + id = -1, + firstName = "William", + lastName = "Smith", + city = "Chicago", + state = "IL", + addresses = listOf( + Address( + city = "Chicago", + state = "IL", + ), + ), + birthYear = 1993, + fullName = "William Smith", + age = 32, + deprecated = false, + ), + ProfileQuery( + id = -2, + firstName = "Jane", + lastName = "Doe", + city = "New York", + state = "NY", + addresses = listOf( + Address( + city = "New York", + state = "NY", + ), + ), + birthYear = 1990, + fullName = "Jane Doe", + age = 35, + deprecated = false, + ), + ProfileQuery( + id = -3, + firstName = "Alicia", + lastName = "West", + city = "Los Angeles", + state = "CA", + addresses = listOf( + Address( + city = "Los Angeles", + state = "CA", + ), ), + birthYear = 1985, + fullName = "Alicia West", + age = 40, + deprecated = false, ), - birthYear = 1993, - fullName = "William Smith", - age = 32, - deprecated = false, ) private val runners: MutableList = mutableListOf() @@ -149,23 +186,32 @@ class RealPirScan @Inject constructor( createCount++ } - // Start each runner on a subset of the broker steps. - brokers.mapNotNull { broker -> + // Split all the broker steps into parts that can be executed in parallel + val brokerScanSteps = brokers.mapNotNull { broker -> repository.getBrokerScanSteps(broker)?.run { brokerStepsParser.parseStep(broker, this) } }.filter { it.isNotEmpty() - }.flatten().splitIntoParts(maxWebViewCount) - .mapIndexed { index, part -> + }.flatten() + .splitIntoParts(maxWebViewCount) + + // Run the scan steps for all profiles one after another + profileQueries.forEach { profile -> + logcat { "PIR-SCAN: Start scan on profile=$profile" } + + brokerScanSteps.mapIndexed { index, part -> // We want to run the runners in parallel but wait for everything complete before we proceed async { - runners[index].start(profileQuery, part) + runners[index].start(profile, part) runners[index].stop() } }.awaitAll() - logcat { "PIR-SCAN: Scan completed for all runners" } + logcat { "PIR-SCAN: Finish scan on profile=$profile" } + } + + logcat { "PIR-SCAN: Scan completed for all runners on all profiles" } emitScanCompletedPixel( runType, currentTimeProvider.currentTimeMillis() - startTimeMillis, @@ -183,33 +229,34 @@ class RealPirScan @Inject constructor( runners.clear() } repository.deleteAllScanResults() - repository.getUserProfiles().also { - if (it.isNotEmpty()) { - // Temporarily taking the first profile only for the PoC. In the reality, more than 1 should be allowed. - val storedProfile = it[0] - profileQuery = ProfileQuery( - firstName = storedProfile.userName.firstName, - lastName = storedProfile.userName.lastName, - city = storedProfile.addresses.city, - state = storedProfile.addresses.state, - addresses = listOf( - Address( - city = storedProfile.addresses.city, - state = storedProfile.addresses.state, + repository.getUserProfiles().also { profiles -> + if (profiles.isNotEmpty()) { + profileQueries = profiles.map { storedProfile -> + ProfileQuery( + id = storedProfile.id, + firstName = storedProfile.userName.firstName, + lastName = storedProfile.userName.lastName, + city = storedProfile.addresses.city, + state = storedProfile.addresses.state, + addresses = listOf( + Address( + city = storedProfile.addresses.city, + state = storedProfile.addresses.state, + ), ), - ), - birthYear = storedProfile.birthYear, - fullName = storedProfile.userName.middleName?.run { - "${storedProfile.userName.firstName} $this ${storedProfile.userName.lastName}" - } - ?: "${storedProfile.userName.firstName} ${storedProfile.userName.lastName}", - age = LocalDate.now().year - storedProfile.birthYear, - deprecated = false, - ) + birthYear = storedProfile.birthYear, + fullName = storedProfile.userName.middleName?.run { + "${storedProfile.userName.firstName} $this ${storedProfile.userName.lastName}" + } + ?: "${storedProfile.userName.firstName} ${storedProfile.userName.lastName}", + age = LocalDate.now().year - storedProfile.birthYear, + deprecated = false, + ) + } } } - logcat { "PIR-SCAN: Running scan on profile: $profileQuery on ${Thread.currentThread().name}" } + logcat { "PIR-SCAN: Running scan on profiles: $profileQueries on ${Thread.currentThread().name}" } } override suspend fun executeAllBrokers( diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scripts/models/PirScriptProfile.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scripts/models/PirScriptProfile.kt index c0a9e8441f41..200fd13fc788 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scripts/models/PirScriptProfile.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scripts/models/PirScriptProfile.kt @@ -20,7 +20,7 @@ package com.duckduckgo.pir.internal.scripts.models * This profile represents the data we can get from the web UI / from the user */ data class ProfileQuery( - val id: Int? = null, + val id: Long? = null, val firstName: String, val lastName: String, val middleName: String? = null, diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/PirRepository.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/PirRepository.kt index 7fd7e67b416f..eabea00c69df 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/PirRepository.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/PirRepository.kt @@ -81,6 +81,11 @@ interface PirRepository { suspend fun getBrokerOptOutSteps(name: String): String? + /** + * Returns a list of broker names for which an extract profile result is available. + * + * @param formOptOutOnly - True to only return the brokers with a form opt-out type. + */ suspend fun getBrokersForOptOut(formOptOutOnly: Boolean): List suspend fun saveNavigateResult( @@ -99,9 +104,14 @@ interface PirRepository { response: ExtractedResponse, ) - suspend fun getExtractProfileResultForBroker( + /** + * Returns a list of all [ExtractedProfileResult] found for this particular broker. + * + * @param brokerName - Name of the broker + */ + suspend fun getExtractProfileResultsForBroker( brokerName: String, - ): ExtractedProfileResult? + ): List fun getAllScanResultsFlow(): Flow> @@ -306,15 +316,16 @@ class RealPirRepository( it.extractResults.isNotEmpty() }.map { it.brokerName - }.run { - if (formOptOutOnly) { - this.filter { - brokerDao.getOptOutJson(it)?.contains("\"optOutType\":\"formOptOut\"") == true + }.distinct() + .run { + if (formOptOutOnly) { + this.filter { + brokerDao.getOptOutJson(it)?.contains("\"optOutType\":\"formOptOut\"") == true + } + } else { + this } - } else { - this } - } } override suspend fun saveNavigateResult( @@ -371,18 +382,19 @@ class RealPirRepository( } } - override suspend fun getExtractProfileResultForBroker(brokerName: String): ExtractedProfileResult? = withContext(dispatcherProvider.io()) { - return@withContext scanResultsDao.getExtractProfileResultForProfile(brokerName).firstOrNull()?.run { - ExtractedProfileResult( - brokerName = this.brokerName, - completionTimeInMillis = this.completionTimeInMillis, - actionType = this.actionType, - extractResults = this.extractResults.mapNotNull { - extractedProfileAdapter.fromJson(it) - }, - profileQuery = null, - ) - } + override suspend fun getExtractProfileResultsForBroker(brokerName: String): List = withContext(dispatcherProvider.io()) { + return@withContext scanResultsDao.getExtractProfileResultForBroker(brokerName) + .map { extractProfileResult -> + ExtractedProfileResult( + brokerName = extractProfileResult.brokerName, + completionTimeInMillis = extractProfileResult.completionTimeInMillis, + actionType = extractProfileResult.actionType, + extractResults = extractProfileResult.extractResults.mapNotNull { + extractedProfileAdapter.fromJson(it) + }, + profileQuery = profileQueryAdapter.fromJson(extractProfileResult.userData), + ) + } } override fun getAllScanResultsFlow(): Flow> { diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/db/ScanResultsDao.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/db/ScanResultsDao.kt index 1f65396432ba..b2f730973e25 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/db/ScanResultsDao.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/db/ScanResultsDao.kt @@ -55,7 +55,7 @@ interface ScanResultsDao { fun getAllExtractProfileResult(): List @Query("SELECT * FROM pir_scan_extracted_profile WHERE brokerName = :brokerName ORDER BY completionTimeInMillis") - fun getExtractProfileResultForProfile(brokerName: String): List + fun getExtractProfileResultForBroker(brokerName: String): List @Insert(onConflict = OnConflictStrategy.REPLACE) fun insertExtractProfileResult(extractProfileResult: ExtractProfileResult) From 575be3aaca95572b6f831c05c025ff53d9a8b9a4 Mon Sep 17 00:00:00 2001 From: Domen Lanisnik Date: Thu, 17 Jul 2025 15:41:03 +0200 Subject: [PATCH 2/6] Add additional default profile to PirOptOut --- .../duckduckgo/pir/internal/optout/PirOptOut.kt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt index d146c2919668..ea0788dfb514 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt @@ -136,6 +136,23 @@ class RealPirOptOut @Inject constructor( age = 35, deprecated = false, ), + ProfileQuery( + id = -3, + firstName = "Alicia", + lastName = "West", + city = "Los Angeles", + state = "CA", + addresses = listOf( + Address( + city = "Los Angeles", + state = "CA", + ), + ), + birthYear = 1985, + fullName = "Alicia West", + age = 40, + deprecated = false, + ), ) private val runners: MutableList = mutableListOf() From 7daa1ef251ef094331017d8e2c7d20efda9fb223 Mon Sep 17 00:00:00 2001 From: Domen Lanisnik Date: Thu, 17 Jul 2025 15:57:45 +0200 Subject: [PATCH 3/6] Fix code formatting --- .../duckduckgo/pir/internal/common/PirActionsRunner.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt index 84fc0ba63efa..79bea92ba8a4 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt @@ -145,8 +145,12 @@ class RealPirActionsRunner @AssistedInject constructor( } withContext(dispatcherProvider.main()) { - logcat { "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} profile=$profileQuery Brokers to execute $brokerSteps" } - logcat { "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} profile=$profileQuery Brokers size: ${brokerSteps.size}" } + logcat { + "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} " + + "Brokers size: ${brokerSteps.size} " + + "profile=$profileQuery " + + "Brokers to execute $brokerSteps" + } detachedWebView = pirDetachedWebViewProvider.createInstance( context, pirScriptToLoad, From 8e407655e3a22c475a67ed97eb98b15165beced7 Mon Sep 17 00:00:00 2001 From: Domen Lanisnik Date: Mon, 21 Jul 2025 08:18:30 +0200 Subject: [PATCH 4/6] Optimize the runner work distribution --- .../pir/internal/common/PirUtils.kt | 23 +++++-- .../pir/internal/optout/PirOptOut.kt | 68 +++++++++++-------- .../duckduckgo/pir/internal/scan/PirScan.kt | 38 ++++++----- 3 files changed, 76 insertions(+), 53 deletions(-) diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirUtils.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirUtils.kt index 40f297daee3d..ba341c59ee07 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirUtils.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirUtils.kt @@ -17,10 +17,23 @@ package com.duckduckgo.pir.internal.common internal fun List.splitIntoParts(parts: Int): List> { - return if (this.isEmpty()) { - emptyList() - } else { - val chunkSize = (this.size + parts - 1) / parts // Ensure rounding up - this.chunked(chunkSize) + if (this.isEmpty()) { + return emptyList() } + + val partSize = this.size / parts + val remainder = this.size % parts + + val result = mutableListOf>() + var startIndex = 0 + + for (i in 0 until parts) { + val currentPartSize = partSize + if (i < remainder) 1 else 0 + val endIndex = startIndex + currentPartSize + + result.add(this.subList(startIndex, endIndex)) + startIndex = endIndex + } + + return result } diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt index ea0788dfb514..e8e0cc2368d7 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt @@ -185,20 +185,27 @@ class RealPirOptOut @Inject constructor( repository.getBrokerOptOutSteps(broker)?.let { broker to it } } - // Create a map of profiles that have opt-out steps for those brokers - val profileBrokerStepsMap = profileQueries.associateWith { profileQuery -> + // Map broker steps with their associated profile queries + val allSteps = profileQueries.map { profileQuery -> brokerOptOutStepsJsons.map { (broker, stepsJson) -> brokerStepsParser.parseStep(broker, stepsJson, profileQuery) - }.flatten() - }.filter { it.value.isNotEmpty() } - - // Start each runner on the profile with the broker steps - profileBrokerStepsMap.entries.map { (profileQuery, brokerSteps) -> - logcat { "PIR-OPT-OUT: Start opt-out for profile: $profileQuery on ${Thread.currentThread().name}" } - runners[0].startOn(webView, profileQuery, brokerSteps) - runners[0].stop() - logcat { "PIR-OPT-OUT: Finish opt-out for profile: $profileQuery on ${Thread.currentThread().name}" } - } + }.flatten().map { step -> profileQuery to step } + }.flatten() + + // Assign steps to runners based on the maximum number of WebViews we can use + val stepsPerRunner = allSteps.splitIntoParts(maxWebViewCount) + + // Execute the steps on all runners in parallel + stepsPerRunner.mapIndexed { index, partSteps -> + async { + partSteps.map { (profileQuery, step) -> + logcat { "PIR-OPT-OUT: Start on runner=$index, thread=${Thread.currentThread().name}, profile=$profileQuery and step=$step" } + runners[index].start(profileQuery, listOf(step)) + runners[index].stop() + logcat { "PIR-OPT-OUT: Finish on runner=$index, thread=${Thread.currentThread().name}, profile=$profileQuery and step=$step" } + } + } + }.awaitAll() logcat { "PIR-OPT-OUT: Opt-out completed for all runners and profiles" } @@ -228,15 +235,17 @@ class RealPirOptOut @Inject constructor( repository.getBrokerOptOutSteps(broker)?.let { broker to it } } - // Create a map of profiles that have opt-out steps for those brokers - val profileBrokerStepsMap = profileQueries.associateWith { profileQuery -> + // Map broker steps with their associated profile queries + val allSteps = profileQueries.map { profileQuery -> brokerOptOutStepsJsons.map { (broker, stepsJson) -> brokerStepsParser.parseStep(broker, stepsJson, profileQuery) - }.flatten() - }.filter { it.value.isNotEmpty() } + }.flatten().map { step -> profileQuery to step } + }.flatten() + + maxWebViewCount = minOf(allSteps.size, MAX_DETACHED_WEBVIEW_COUNT) - val stepsCount = profileBrokerStepsMap.values.maxOf { it.size } - maxWebViewCount = minOf(stepsCount, MAX_DETACHED_WEBVIEW_COUNT) + // Assign steps to runners based on the maximum number of WebViews we can use + val stepsPerRunner = allSteps.splitIntoParts(maxWebViewCount) logcat { "PIR-OPT-OUT: Attempting to create $maxWebViewCount parallel runners on ${Thread.currentThread().name}" } @@ -253,18 +262,17 @@ class RealPirOptOut @Inject constructor( createCount++ } - // Start each runner on the profile with the broker steps - profileBrokerStepsMap.entries.map { (profileQuery, brokerSteps) -> - logcat { "PIR-OPT-OUT: Start opt-out on profile: $profileQuery" } - brokerSteps.splitIntoParts(maxWebViewCount) - .mapIndexed { index, part -> - async { - runners[index].start(profileQuery, part) - runners[index].stop() - } - }.awaitAll() - logcat { "PIR-OPT-OUT: Finish opt-out on profile: $profileQuery" } - } + // Execute the steps on all runners in parallel + stepsPerRunner.mapIndexed { index, partSteps -> + async { + partSteps.map { (profileQuery, step) -> + logcat { "PIR-OPT-OUT: Start opt-out on runner=$index, profile=$profileQuery and step=$step" } + runners[index].start(profileQuery, listOf(step)) + runners[index].stop() + logcat { "PIR-OPT-OUT: Finish opt-out on runner=$index, profile=$profileQuery and step=$step" } + } + } + }.awaitAll() logcat { "PIR-OPT-OUT: Opt-out completed for all runners and profiles" } emitCompletedPixel() diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt index 573e39d0b967..f850c8a02958 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt @@ -166,11 +166,8 @@ class RealPirScan @Inject constructor( cleanPreviousRun() val script = pirCssScriptLoader.getScript() - maxWebViewCount = if (brokers.size <= MAX_DETACHED_WEBVIEW_COUNT) { - brokers.size - } else { - MAX_DETACHED_WEBVIEW_COUNT - } + maxWebViewCount = minOf(brokers.size * profileQueries.size, MAX_DETACHED_WEBVIEW_COUNT) + logcat { "PIR-SCAN: Attempting to create $maxWebViewCount parallel runners on ${Thread.currentThread().name}" } // Initiate runners @@ -186,7 +183,7 @@ class RealPirScan @Inject constructor( createCount++ } - // Split all the broker steps into parts that can be executed in parallel + // Prepare a list of all broker steps that need to be run val brokerScanSteps = brokers.mapNotNull { broker -> repository.getBrokerScanSteps(broker)?.run { brokerStepsParser.parseStep(broker, this) @@ -194,22 +191,27 @@ class RealPirScan @Inject constructor( }.filter { it.isNotEmpty() }.flatten() - .splitIntoParts(maxWebViewCount) - // Run the scan steps for all profiles one after another - profileQueries.forEach { profile -> - logcat { "PIR-SCAN: Start scan on profile=$profile" } + // Combine the broker steps with each profile and split into equal parts + val stepsPerRunner = profileQueries.map { profileQuery -> + brokerScanSteps.map { scanStep -> + profileQuery to scanStep + } + }.flatten() + .splitIntoParts(maxWebViewCount) - brokerScanSteps.mapIndexed { index, part -> - // We want to run the runners in parallel but wait for everything complete before we proceed - async { - runners[index].start(profile, part) + // Execute the steps in parallel + stepsPerRunner.mapIndexed { index, partSteps -> + // We want to run the runners in parallel but wait for everything to complete before we proceed + async { + partSteps.forEach { (profile, step) -> + logcat { "PIR-SCAN: Start scan on runner=$index for profile=$profile with step=$step" } + runners[index].start(profile, listOf(step)) runners[index].stop() + logcat { "PIR-SCAN: Finish scan on runner=$index for profile=$profile with step=$step" } } - }.awaitAll() - - logcat { "PIR-SCAN: Finish scan on profile=$profile" } - } + } + }.awaitAll() logcat { "PIR-SCAN: Scan completed for all runners on all profiles" } emitScanCompletedPixel( From a8a7496f124a3dbf4ad14b16023d186775c3a859 Mon Sep 17 00:00:00 2001 From: Domen Lanisnik Date: Mon, 21 Jul 2025 08:50:12 +0200 Subject: [PATCH 5/6] Update debug opt-out --- .../pir/internal/optout/PirOptOut.kt | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt index e8e0cc2368d7..3832e3354526 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt @@ -192,20 +192,13 @@ class RealPirOptOut @Inject constructor( }.flatten().map { step -> profileQuery to step } }.flatten() - // Assign steps to runners based on the maximum number of WebViews we can use - val stepsPerRunner = allSteps.splitIntoParts(maxWebViewCount) - - // Execute the steps on all runners in parallel - stepsPerRunner.mapIndexed { index, partSteps -> - async { - partSteps.map { (profileQuery, step) -> - logcat { "PIR-OPT-OUT: Start on runner=$index, thread=${Thread.currentThread().name}, profile=$profileQuery and step=$step" } - runners[index].start(profileQuery, listOf(step)) - runners[index].stop() - logcat { "PIR-OPT-OUT: Finish on runner=$index, thread=${Thread.currentThread().name}, profile=$profileQuery and step=$step" } - } - } - }.awaitAll() + // Execute each steps sequentially on the single runner + allSteps.forEach { (profileQuery, step) -> + logcat { "PIR-OPT-OUT: Start thread=${Thread.currentThread().name}, profile=$profileQuery and step=$step" } + runners[0].startOn(webView, profileQuery, listOf(step)) + runners[0].stop() + logcat { "PIR-OPT-OUT: Finish thread=${Thread.currentThread().name}, profile=$profileQuery and step=$step" } + } logcat { "PIR-OPT-OUT: Opt-out completed for all runners and profiles" } From 2133953d714739b021fc2568a2f169b2cd1c5a69 Mon Sep 17 00:00:00 2001 From: Domen Lanisnik Date: Mon, 21 Jul 2025 08:52:33 +0200 Subject: [PATCH 6/6] Simplify parse steps function to use profile id --- .../duckduckgo/pir/internal/common/BrokerStepsParser.kt | 9 ++++----- .../java/com/duckduckgo/pir/internal/optout/PirOptOut.kt | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt index a46be35eade9..796d98949ad3 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt @@ -23,7 +23,6 @@ import com.duckduckgo.pir.internal.common.BrokerStepsParser.BrokerStep.OptOutSte import com.duckduckgo.pir.internal.common.BrokerStepsParser.BrokerStep.ScanStep import com.duckduckgo.pir.internal.scripts.models.BrokerAction import com.duckduckgo.pir.internal.scripts.models.ExtractedProfile -import com.duckduckgo.pir.internal.scripts.models.ProfileQuery import com.duckduckgo.pir.internal.store.PirRepository import com.squareup.anvil.annotations.ContributesBinding import com.squareup.moshi.JsonAdapter @@ -41,14 +40,14 @@ interface BrokerStepsParser { * * @param brokerName - name of the broker to which these steps belong to * @param stepsJson - string in JSONObject format obtained from the broker's json representing a step (scan / opt-out). - * @param profileQuery - profile associated with the step (used for the opt-out step) + * @param profileQueryId - profile query id associated with the step (used for the opt-out step) * @return list of broker steps resulting from the passed params. If the step is of type OptOut, it will return a list of * OptOutSteps where an OptOut step is mapped to each of the profile for the broker. */ suspend fun parseStep( brokerName: String, stepsJson: String, - profileQuery: ProfileQuery? = null, + profileQueryId: Long? = null, ): List sealed class BrokerStep( @@ -104,13 +103,13 @@ class RealBrokerStepsParser @Inject constructor( override suspend fun parseStep( brokerName: String, stepsJson: String, - profileQuery: ProfileQuery?, + profileQueryId: Long?, ): List = withContext(dispatcherProvider.io()) { return@withContext runCatching { adapter.fromJson(stepsJson)?.run { if (this is OptOutStep) { repository.getExtractProfileResultsForBroker(brokerName) - .filter { it.profileQuery != null && it.profileQuery.id == profileQuery?.id } + .filter { it.profileQuery != null && it.profileQuery.id == profileQueryId } .map { it.extractResults.map { extractedProfile -> this.copy( diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt index 3832e3354526..db619ca0aded 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt @@ -188,7 +188,7 @@ class RealPirOptOut @Inject constructor( // Map broker steps with their associated profile queries val allSteps = profileQueries.map { profileQuery -> brokerOptOutStepsJsons.map { (broker, stepsJson) -> - brokerStepsParser.parseStep(broker, stepsJson, profileQuery) + brokerStepsParser.parseStep(broker, stepsJson, profileQuery.id) }.flatten().map { step -> profileQuery to step } }.flatten() @@ -231,7 +231,7 @@ class RealPirOptOut @Inject constructor( // Map broker steps with their associated profile queries val allSteps = profileQueries.map { profileQuery -> brokerOptOutStepsJsons.map { (broker, stepsJson) -> - brokerStepsParser.parseStep(broker, stepsJson, profileQuery) + brokerStepsParser.parseStep(broker, stepsJson, profileQuery.id) }.flatten().map { step -> profileQuery to step } }.flatten()