4780 switch off private mode

[severinrudie]

Pull Request checklist

• Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
• Tests: This PR includes thorough tests or an explanation of why it does not
• Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  Doesn't change the UI
• Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

• Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

To download an APK when reviewing a PR:

1. click on Show All Checks,
2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
3. click on the "Fenix - assemble" task, then click "Run Artifacts".
4. the APK links should be on the left side of the screen, named for each CPU architecture

[codecov-io]

Codecov Report

Merging #5614 into master will increase coverage by <.01%.
The diff coverage is 25%.

@@             Coverage Diff              @@
##             master    #5614      +/-   ##
============================================
+ Coverage     14.46%   14.46%   +<.01%     
- Complexity      322      325       +3     
============================================
  Files           255      255              
  Lines         10490    10517      +27     
  Branches       1526     1536      +10     
============================================
+ Hits           1517     1521       +4     
- Misses         8851     8873      +22     
- Partials        122      123       +1

Impacted Files	Coverage Δ	Complexity Δ
.../src/main/java/org/mozilla/fenix/utils/Settings.kt	72.25% <ø> (ø)	15 <0> (ø)	⬇️
...ozilla/fenix/session/SessionNotificationService.kt	0% <0%> (ø)	0 <0> (ø)	⬇️
...rc/main/java/org/mozilla/fenix/FenixApplication.kt	4.5% <33.33%> (+0.8%)	4 <2> (+2)	⬆️
...deletebrowsingdata/DeleteBrowsingDataController.kt	79.31% <0%> (-8.69%)	0% <0%> (ø)
...s/deletebrowsingdata/DeleteBrowsingDataFragment.kt	0% <0%> (ø)	0% <0%> (ø)	⬇️
...ava/org/mozilla/fenix/settings/SettingsFragment.kt	0% <0%> (ø)	0% <0%> (ø)	⬇️
...pp/src/main/java/org/mozilla/fenix/FeatureFlags.kt	90.9% <0%> (+2.02%)	3% <0%> (+1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2896b36...112410e. Read the comment docs.

[sblatz]

This doesn't seem to be working for me:

[sblatz]

alwaysOpenInPrivateMode is actually a misleading variable name. I'm submitting a patch right now to change it, but currently it controls "launch links in a private tab."

I don't think we actually want to stop the user from opening in normal mode because of this pref.

[sblatz]

PR opened: https://github.com/mozilla-mobile/fenix/pull/5701/files

[severinrudie]

This doesn't seem to be working for me:

To clarify, there are two flows here.

Flow 1 (working as expected)

STR:

• Open FF
• Set private mode
• Kill FF
• Reopen

Expected:

• FF opens in normal mode

Actual:

• FF opens in normal mode

Flow 2 (failing)

STR:

• Open FF
• Set private mode
• Navigate to any website
• Kill FF
• Reopen

Expected:

• No tabs are open
• FF opens in normal mode

Actual:

• No tabs are open
• FF opens in private mode

I'm working on a fix

[severinrudie]

It looks like the application is not being destroyed if the app is force killed while a private tab is opened. Checked by running adb shell ps | grep org.mozilla.fenix after force killing with and without private tabs open.

Seems to be related to the private tab notification. I commented out this notification, and the app was killed as expected.

[severinrudie]

@sblatz: @ boek suggested storing usePrivateMode in memory as an alternative to this approach. I think that's a really great suggestion, but until we work out why the application is not being destroyed when the notification service is, it won't solve that edge case. I haven't worked out a approach than this in the meantime, and I don't think it's worth spending more time on it.

[sblatz]

This looks good now 😄