1861459 - [lib-state] Remove `BrowsingModeManager` and track Private Browsing Mode in the `AppStore` exclusively

Status: ASSIGNED

(Firefox for Android :: Homepage, enhancement, P3)

Description

[Matt Tighe [:matt-tighe]]

The BrowsingModeManager is a utility class that tracks changes to the Private Browsing Mode.

Utility classes like this do not follow architecture best practices. Instead of tracking this state in a one-off class, it should be tracked in the Store.

Remove BrowsingModeManager and replace its usages with State reads from the AppStore as well as Actions that update the AppStore.

Comment 1

[Matt Tighe [:matt-tighe]]

For QA:

This is a fairly large refactor of most things related to private browsing mode. I thought it might make it easier on your end to be able to do a large smoke test of these changes, rather than address them individually. Hope that was an okay assumption. There should be no change in functionality from what previously existed.

I tried to make a list of the areas this patch touches. It may not be completely exhaustive, so it would be great to add anything from existing test suites that relate to private browsing mode.

Mode retained across app sessions
Mode inherited from a deep link
Window is in secure mode while in private mode if setting is enabled
PBM button
New tabs from long-press on tab counter
New tabs from tabs tray
Tabs tray opens in correct mode from Home Screen and browser screen
Long-press top site to open in PBM
Opening tabs from bookmarks
Opening tabs from history
Opening tabs from history groups
Opening recently closed private tab
Addons report button and settings open tabs in private mode if app is in private mode
Reader mode gets themed differently
Recent tabs section visibility and recent bookmarks count metrics not reported in private mode
Standard homepage view metric not incremented when in private mode
Wallpapers onboarding not shown in private mode
Search suggestions not shown in private mode if settings is enabled

Comment 2

[BMO Github Automation]

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/32c3e5aae7acc037949c38027ae23d76bc80da1a
[main] Bug 1861459 - Add a binding from BrowserStore to AppStore for selected tab changes

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/d5856268ec720ad0e41585a3f75201ae8d5e6755
[main] Bug 1861459 - Remove BrowsingModeManager usages outside of the Home Screen

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/86e0f3c0ef1dd3a16194eeaff330c8517d18b49f
[main] Bug 1861459 - Remove Home Screen usages of BrowsingModeManager

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/38af1c1363aa34b564305a1b2c809249e1be5b1b
[main] Bug 1861459 - Remove BrowsingModeManager

Comment 3

[Lorand Janos]

Hi,
I've tried covering all the points mentioned in comment 1 to be tested by going through the steps on some of the test cases we have that cover the mentioned scenarios.
No issues were found.
Build tested Nightly 123.0a1 (from 2024-01-10).
Device used for testing: Samsung Galaxy S23 Ultra (Android 14).
Marking the ticket as verified for 123.

Comment 4

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Revert "Bug 1861459 - Remove BrowsingModeManager" (#5205)

Comment 5

[BMO Github Automation]

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/f69aa6e9408f0022bb7f4da9d91eaa622a330fbe
[main] Revert "Bug 1861459 - Remove BrowsingModeManager"

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/c0c89dc85e1a6b5033bec9a10fea70ce584ea67a
[main] Revert "Bug 1861459 - Remove Home Screen usages of BrowsingModeManager"

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/768b8368f9d945567af139ab4db823adcdc32e89
[main] Revert "Bug 1861459 - Remove BrowsingModeManager usages outside of the Home Screen"

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/b21496d484f738938c6f95fe64af3f1aecfb5b4c
[main] Revert "Bug 1861459 - Add a binding from BrowserStore to AppStore for selected tab changes"

Comment 6

[Mira Lobontiu (Android QA)]

Attachment: az_recorder_20240119_163407.mp4

I've encountered this issue:

1. Open the private browsing homescreen on Fenix nightly 123.0a1.
2. Send the app in the background and kill it (remove also from recently used apps).
3. Re-open Fenix. Observe the homescreen.

Actual result: Firefox for Android opens in normal browsing mode.
Not reproducible on Beta 122.0b9, nor on RC 122.0 with the same Google Pixel 6 (Android 14) device.

Comment 7

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1861459 - Update AppState.mode based on selected tab changes (#5284)

Comment 8

[BMO Github Automation]

Authored by https://github.com/MatthewTighe
https://github.com/mozilla-mobile/firefox-android/commit/db3bbc8ef52479ed6c813458c13487f749165101
[main] Bug 1861459 - Update AppState.mode based on selected tab changes

Comment 9

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1861459 - Update Home Screen to use BrowsingMode from AppStore (#5502)

Comment 10

[Matt Tighe [:matt-tighe]]

Attachment: WIP: Bug 1861459 - Include previous and updated state in ChangeDetectionMiddleware

Comment 11

[Matt Tighe [:matt-tighe]]

Attachment: WIP: Bug 1861459 - Swap BrowserStoreBinding for a ChangeDetectionMiddleware

Comment 12

[Matt Tighe [:matt-tighe]]

Attachment: WIP: Bug 1861459 - Swap Home Screen usages of PBM from Manager to the AppStore

Comment 13

[Matt Tighe [:matt-tighe]]

Attachment: WIP: Bug 1861459 - Update PBM state reads to use AppStore

Comment 14

[Matt Tighe [:matt-tighe]]

Attachment: WIP: Bug 1861459 - Add optional mode parameter to openToBrowserAndLoad

Comment 15

[Matt Tighe [:matt-tighe]]

Attachment: WIP: Bug 1861459 - Change PBM state updates from BrowsingModeManager to AppStore

Comment 16

[Matt Tighe [:matt-tighe]]

Attachment: WIP: Bug 1861459 - Remove BrowsingModeManager