Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

FNX-14508 ⁃ For #11903: Fix private custom tabs #13327

Merged
merged 1 commit into from
Aug 5, 2020
Merged

FNX-14508 ⁃ For #11903: Fix private custom tabs #13327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import android.os.Bundle
import android.os.StrictMode
import androidx.annotation.VisibleForTesting
import mozilla.components.feature.intent.processing.IntentProcessor
import org.mozilla.fenix.HomeActivity.Companion.PRIVATE_BROWSING_MODE
import org.mozilla.fenix.components.IntentProcessorType
import org.mozilla.fenix.components.getType
import org.mozilla.fenix.components.metrics.Event
Expand Down Expand Up @@ -43,7 +44,15 @@ class IntentReceiverActivity : Activity() {

fun processIntent(intent: Intent) {
// Call process for side effects, short on the first that returns true
val processor = getIntentProcessors().firstOrNull { it.process(intent) }
val private = settings().openLinksInAPrivateTab
intent.putExtra(PRIVATE_BROWSING_MODE, private)
if (private) {
components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.PRIVATE))
} else {
components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.NORMAL))
}

val processor = getIntentProcessors(private).firstOrNull { it.process(intent) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So AFAICT this getIntentProcessors will also process externalAppIntentProcessors and others. Before even though it wasn't working (lol) it was attempting to just add this extra on the custom tab and intent processors. Will there be any bad side effects from adding this extra to other intent entry points?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be side effects when we add extra intents. If we want to be super safe we can set the boolean on both this.intent (before) and the home activity intent (now).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that was a confusing comment. I meant as an example - now the TWA/PWA externalAppIntentProcessors will also be attaching PRIVATE_BROWSING_MODE, right? Does that mean PWAs will also be attempting to present / be in PBM?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly we wanted it to work that way in the first place, so if they do present in PBM that fixes another bug :)

We can do another pass in the future to re-evaulate what data is getting passed in intents like this.

val intentProcessorType = components.intentProcessors.getType(processor)

launch(intent, intentProcessorType)
Expand All @@ -65,17 +74,14 @@ class IntentReceiverActivity : Activity() {
finish() // must finish() after starting the other activity
}

private fun getIntentProcessors(): List<IntentProcessor> {
val modeDependentProcessors = if (settings().openLinksInAPrivateTab) {
components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.PRIVATE))
intent.putExtra(HomeActivity.PRIVATE_BROWSING_MODE, true)
private fun getIntentProcessors(private: Boolean): List<IntentProcessor> {
val modeDependentProcessors = if (private) {
listOf(
components.intentProcessors.privateCustomTabIntentProcessor,
components.intentProcessors.privateIntentProcessor
)
} else {
components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.NORMAL))
intent.putExtra(HomeActivity.PRIVATE_BROWSING_MODE, false)
listOf(
components.intentProcessors.customTabIntentProcessor,
components.intentProcessors.intentProcessor
Expand Down
10 changes: 8 additions & 2 deletions app/src/test/java/org/mozilla/fenix/IntentReceiverActivityTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.unmockkStatic
import io.mockk.verify
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runBlockingTest
import mozilla.components.feature.intent.processing.IntentProcessor
Expand Down Expand Up @@ -139,9 +140,14 @@ class IntentReceiverActivityTest {
attachMocks(activity)
activity.processIntent(intent)

val shadow = shadowOf(activity)
val actualIntent = shadow.peekNextStartedActivity()

val normalProcessor = intentProcessors.intentProcessor
coVerify(exactly = 0) { normalProcessor.process(intent) }
coVerify { intentProcessors.privateIntentProcessor.process(intent) }
verify(exactly = 0) { normalProcessor.process(intent) }
verify { intentProcessors.privateIntentProcessor.process(intent) }
assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
assertTrue(actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false))
}

@Test
Expand Down