1857021 - Support for "logout" and "delete_account" messages is missing from fx-webchannel

Status: RESOLVED FIXED

(Firefox for Android :: Accounts and Sync, defect, P1)

Description

[Mike a [:mavduevskiy]]

Problem

When a user is deleting the account at "accounts.firefox.com", the app enters reath state

Solution

Listen to the webchannel message confirming account removal, and change the app state accordingly. UX should be similar to what we have on desktop.

Comment 1

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1857021 – Support delete webchannel message (#4490)

Comment 2

[Jonathan Almeida [:jonalmeida]]

I'm going to take this since we are landing an FXA patch that would be nice to have both our platform across both of our platforms.

Comment 3

[Jonathan Almeida [:jonalmeida]]

Attachment: Bug 1857021 - Part 1: Add support for LOGOUT and DELETE_ACCOUNT web channel messages r=#android-reviewers,chenba,dschomburg

We log out from the FxaAccountManager here for the fxaccounts:logout
and fxaccounts:delete_account messages to stay in line with desktop
and [iOS][1] who the same thing today.

[1[: https://github.com/mozilla-mobile/firefox-ios/blob/3d0481675753805b68ca41a166d9384126f45b02/firefox-ios/RustFxA/FxAWebViewModel.swift#L262-L264

Comment 4

[Jonathan Almeida [:jonalmeida]]

Attachment: Bug 1857021 - Part 2: Add Activity.isIntentInternal() for reusability r=#android-reviewers

This is something we want to use to make sure that the Custom Tab
intent that we received is from our own application so that we don't
mistakenly try and close an actual Custom Tab in a third party app
that has the web channel code running there.

Depends on D217156

Comment 5

[Jonathan Almeida [:jonalmeida]]

Attachment: Bug 1857021 - Part 3: Make the FxaWebChannelFeature dismiss the activity on logout r=#android-reviewers

To connect the feature to the UI, we wrap it in an integration class
so that its isolated as we start to handle more commands.

We also early-return when we know that the shared UI is opened from
within the app instead of from a third party app.

Depends on D217158

Comment 6

[Jonathan Almeida [:jonalmeida]]

Attachment: Bug 1857021 - Part 4: Add unit tests to FxaWebChannelFeature r=#android-reviewers,chenba,dschomburg

When adding some of these tests, we add a MainCoroutineRule which
now makes anything that was wrapped in the a Dispatchers.Main
coroutine. Since we weren't doing this before, a bunch of tests
started to fail that were not exercising the real codepaths.

Another example where we need to stop using mocks everywhere unless
it is necessary.

Depends on D217159

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50fe4f973e73
Part 1: Add support for LOGOUT and DELETE_ACCOUNT web channel messages r=chenba,android-reviewers
https://hg.mozilla.org/integration/autoland/rev/e84bc1a197fc
Part 2: Add Activity.isIntentInternal() for reusability r=android-reviewers,amejiamarmol
https://hg.mozilla.org/integration/autoland/rev/fa0ced592e7d
Part 3: Make the FxaWebChannelFeature dismiss the activity on logout r=android-reviewers,amejiamarmol
https://hg.mozilla.org/integration/autoland/rev/82c5d07c2f16
Part 4: Add unit tests to FxaWebChannelFeature r=android-reviewers,amejiamarmol

Comment 8

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/50fe4f973e73
https://hg.mozilla.org/mozilla-central/rev/e84bc1a197fc
https://hg.mozilla.org/mozilla-central/rev/fa0ced592e7d
https://hg.mozilla.org/mozilla-central/rev/82c5d07c2f16

Comment 9

[[:skhan]]

Attachment: Screen_recording_20240923_014823.mp4

Hi Jonathon, I was planning to use this function isInternalIntent() you added for a little something but it wasn’t serving my purpose unfortunately.
If you go to Settings -> Sync and save your data -> Use email instead.
This returns an internal custom tab (as I understand it, means any custom tab that our app launched?). But this function returns false and the reason for that is safeIntent.getStringExtra(EXTRA_ACTIVITY_REFERRER_PACKAGE) returns null .
Could you please take a look whenever you have time?
Let me know if this is a bug or is expected?
(Attaching recording for reference)

Comment 10

[Jonathan Almeida [:jonalmeida]]

(In reply to [:skhan] from comment #9)

Created attachment 9426585 [details]
Screen_recording_20240923_014823.mp4

Hi Jonathon, I was planning to use this function isInternalIntent() you added for a little something but it wasn’t serving my purpose unfortunately.
If you go to Settings -> Sync and save your data -> Use email instead.
This returns an internal custom tab (as I understand it, means any custom tab that our app launched?). But this function returns false and the reason for that is safeIntent.getStringExtra(EXTRA_ACTIVITY_REFERRER_PACKAGE) returns null .
Could you please take a look whenever you have time?
Let me know if this is a bug or is expected?
(Attaching recording for reference)

I spoke with skhan offline. This question was related to bug 1912642.

The TLDR is that the extension method is documented to only work for intents coming from IntentReceiverActivity, so any intent that comes indirectly from there, cannot be guaranteed to have the same EXTRA_ACTIVITY_REFERRER_PACKAGE property.

Comment 11

[Jonathan Almeida [:jonalmeida]]

Comment on attachment 9426585 [details]
Screen_recording_20240923_014823.mp4

Marking as obsolete, so we don't confuse this attachment with this bug.