Rebase Tor Browser mobile/ patches for Firefox ESR 68
This is the Android-specific patches for #30429 (moved).
Activity
Copying from the [ticket:30429 parent] ticket:
Replying to [ticket:30429#comment:2 acat]:
{{{ 43582f5809cf Bug 29859: Disable HLS support for now
34e52459b150 Bug 29982 - Force single-pane UI on Tor Preferences
6cba968b5a7e Bug 30136: Use 'Tor Browser' as brand name on mobile, too 321bba05d203 Bug 30239 - Render Fragments after crash
22e72c1e7716 Bug 30214 - Kill background thread when Activity is null 75dcaffedcee Bug 30086 - Prevent Sync-related crashes on Android 545db499b0c7 Bug 28622: Update Tor Browser icon for mobile
b563b207c38c Bug 29238 - Prevent crash on Android after update 323ebe2ca2d4 Bug 28329 - Part 4. Add new Tor Bootstrapping and configuration screens
79c9b8acba0b Bug 28329 - Part 3. Remove OrbotActivity dependency
dd3f22090e52 Bug 28329 - Part 2. Implement checking if the Tor service is running
fe784a8284e5 Bug 28329 - Part 1. Add new Tor resources a195c7180350 Bug 28507 - Don't call Push and Sync services during Sanitize f6f8859b3891 Bug 26690: Port padlock states for .onion services to mobile 722afbb6c5c6 Bug 28051 - Use Orbot's notification-builder wrapper class
8436a9443fde Bug 28051 - Stop the background service when we're quitting
464c03696814 Bug 28051 - Use our Orbot for proxying our connections
a0b886f177d8 Bug 28051 - Launch Orbot if it isn't running in the background
a5e24cd02c8e Bug 28051 - Integrate Orbot and add dependencies 39316ccaedbb Bug 28507: Implement fallback to delete private data in the browser startup
b0b54ec7c7a0 Bug 28507: Add prefs that allow the browser to delete browsing history by defa.. 894c64799812 Bug 28507: Parse a set of strings in Android Set Preferences e860dd77665d Bug 27125 - Move localized Tor Browser for Android strings into separate file 06f9c75d7725 Bug 27111: Configure tor browser for mobile to load about:tor 28a2ea997ba0 Bug 28125 - Prevent non-Necko network connections 878c9536c888 Bug 27472 - Export MOZILLA_OFFICIAL during Android build
826e0934adb4 Bug 27400 - Target Android API 26 cb63b9e84081 Bug 27473 - Change app name on Android for Alpha version 6177eef07093 Bug 25696 - Design of alpha onboarding for Tor Browser for Android 8e272a5204e2 Bug 25696 - Implement alpha onboarding for Tor Browser for Android 2c08bd6d5df5 Bug 24796 - Comment out excess permissions from GeckoView 1bd41f99acd1 Bug 26825 - Delete RECEIVE_BOOT_COMPLETED permission 9023dea11376 Bug 26826 - Disable tab queue and delete SYSTEM_ALERT_WINDOW permission
350766eecb63 Bug 25906 - Imply false both Adjust and Leanplum configure options
9312fdf1ac53 Bug 27016 - Create proxy connection during image download bff13bd887c3 Bug 26528 - Don't allow Fennec to use UpdateService when installed through the.. 9b218c05cecc Orfox: hook up default panic trigger to "quit and clear"
80b18b10f092 Orfox: quit button added
f1c13fc12685 Orfox: disable screenshots and prevent page from being in "recent apps"
e96b86f7a6f6 Bug 25741 - TBA: Disable GeckoNetworkManager
ffcc8cefd762 Bug 25741 - TBA: Adjust the User Agent String so it doesn't leak Android version 0e4ae4cbd11a Bug 25741 - TBA: top sites changed, used bookmarks icon temporarily.
6e806c3cb9ed Orfox: Centralized proxy applied to AbstractCommunicator and BaseResources.
433766efba25 Orfox: add BroadcastReceiver to receive Tor status from Orbot
41b333e3a9d3 Orfox: NetCipher enabled, checks if orbot is installed
d073b92b20ba Bug 25741 - TBA: Neuter Firefox Accounts
f96dc4c9ae4d Bug 25741 - TBA: Only include GCM permissions if we want them
e8f22d0b795e Bug 25741 - TBA: Only include Firefox Account permissions if we want them
6299c89034d3 Bug 25741 - TBA: Always Quit, do not restore the last session
a66bd47fd601 Bug 25741 - TBA: Disable all data reporting by default
eebbebcb7010 Bug 25741 - TBA: Clear state when the app exits, by default
0820ddf07093 Bug 25741 - TBA: Do not import bookmarks and history from native browser by de.. 8839bac75801 Bug 25741 - TBA: Do not save browsing history by default
3531fb88ab73 Bug 25741 - TBA: Move CAMERA permission within MOZ_WEBRTC
c910edef7833 Bug 25741 - TBA: Conditionally require *_LOCATION permissions
d2fa5a6928bf Bug 25741 - TBA: Conditionally require WIFI and NETWORK permissions
f90df9c52d2d Bug 25741 - TBA: Disable QR Code reader by default
b6584da608bf Bug 25741 - TBA: Disable the microphone by default
92959f9de719 Bug 25741 - TBA: Disable telemetry and experiments
e4ea4bf5fbfd Bug 25741 - TBA: Remove sync option from preferences
7122131c88d9 Bug 25741 - TBA: Add mobile-override of 000-tor-browser prefs
39979b254f2c Bug 25741 - TBA: Add default configure options in dedicated file
d9b6b1116a67 Bug 25741 - TBA: Do not register Stumbler listener at start up
765b0fb6cdee Bug 25741 - TBA: Add an AppConstant for TOR_BROWSER_VERSION
de5d9f3c4f83 Bug 25741 - TBA: Disable features at compile-time
2e75b845a93f Bug 25741 - TBA: Add mozconfig for Android and pertinent branding files.
b4aef2103fef Bug 25741 - TBA: Move GCM Push prefs within preprocessor guard
db59d0ef5a5b Bug 25741 - TBA: Exclude unwanted Stumbler tests }}}I successfully built a branch based on acat's
30429+1branch. The branch with the Android patches is in my repo as branchacat30429+1_tor-browser_android_68esr_19. This is very much a work in progress, but I'll rebase and squash in different branches.This branch contains many squashed commits.
I've not checked the rebased commits yet but while looking at #28672 (moved) I realized we still have the
tor-browser-buildpatch for #29575 (moved), which I think we should move intotor-browser. Could you moveandroid-dependencies.patchover to that repo and integrate it into your rebased patches?I'm hitting a problem at startup, and I haven't found the cause yet.
08-06 21:08:44.062 4117-4141/org.torproject.torbrowser D/GeckoThread: State changed to LIBS_READY 08-06 21:08:44.062 4117-4141/org.torproject.torbrowser W/GeckoThread: zerdatime 25373549 - runGecko 08-06 21:08:44.062 4117-4141/org.torproject.torbrowser D/GeckoProfile: Found profile dir. 08-06 21:08:44.096 4117-4141/org.torproject.torbrowser I/Gecko:DumpUtils: Fifo watcher disabled via pref. 08-06 21:08:44.163 4117-4141/org.torproject.torbrowser D/GeckoThread: State changed to JNI_READY 08-06 21:08:45.266 4117-4141/org.torproject.torbrowser I/fennec: XRE_main returned 1 08-06 21:08:45.266 4117-4141/org.torproject.torbrowser D/GeckoThread: State changed to EXITEDBecause it is setting JNI_READY, it seems like this is failing after XPCom initialization.
Bisecting leads to this commit (commit
d03d360bd773c8d8a97ef1f69e565afcc19796e5on acat's30429+5branch). This'll require some digging. mcs/brade, if you have any suggestions about where I should look, they'll be very much appreciated.Specifically, I'm hitting this result:
08-06 21:08:45.266 4117-4141/org.torproject.torbrowser I/fennec: XRE_main returned 1I'm having trouble setting breakpoints in the code, so I'm working through the code manually right now. I think this failure is triggered after mScopedXPCOM->Initialize() because I'm getting the log message:
08-06 21:08:44.163 4117-4141/org.torproject.torbrowser D/GeckoThread: State changed to JNI_READYand I believe that is set at the end of the
Initialize()call (after a few layers of indirection, assuming I traced this correctly).Trac:
Cc: N/A to mcs, brade
Replying to sysrqb:
Bisecting leads to this commit (commit
d03d360bd773c8d8a97ef1f69e565afcc19796e5on acat's30429+5branch). This'll require some digging. mcs/brade, if you have any suggestions about where I should look, they'll be very much appreciated. ...Without knowing a lot about the Android-specific code, my guess is that the #9173 (closed) patch is just not correct by itself for Android. What is missing from acat's 30429+5 branch is a patch for #13252 (moved), which Kathy and I have been working on along with the updater patches. The #13252 (moved) patch adds the "side-by-side" TorBrowser-Data magic for macOS and also changes how the app data directory is determined on all platforms. As of yesterday, we have a "hot off the press" patch for #13252 (moved) which we plan to get to acat soon. I will attach a copy to this ticket so you can try it now.
One caveat: the ESR68 #13252 (moved) patch is intended to be inserted into the commit stream immediately after the #11641 (closed) patch (69c8b59f9cdc792ee533187723286fdd75dbabd0 on acat's 30429+5 branch). You may get conflicts if you tack it on the end.
-
ESR68 rebased patch for #13252 (moved)
Some comments on acat30429+5_tor-browser_android_68esr_39:
d0386613a4884393ff3324e43f8d06d30f9d6564 - this is not a patch in the original branch, does the build fail without this? 546cab59acc827d7cf50ae227c2aa84f01c23c2d - ok d37580f4e17b6f3709d6906ef13441e29b61d86e - ok 4b1f9425adf546aecb094dc9a501c61e32fe3394 - In .mozconfig-android, should we delete instead of commenting out lines, to be consistent with what we did in desktop .mozconfigs?
+CC="/home/android/.mozbuild/clang/bin/clang" +CXX="/home/android/.mozbuild/clang/bin/clang++" Shouldn't this be removed?
6041d5df003aed06f8dfe0349f53d2821777331b - ok f5c33693baf1d386c8bccedaee7cc13ce218a4b6 - ok 3c070da60f5d9b851d8c3d85d14e5dabf108d068 - ok 9fa528e6853c0538b8968d5980ab7e0fdbe9be1f - ok 1fd0ea742bdb74e99c484cbda24f4942aea2663c -
-
browser.mirroring.enabledpref seems not to be used anymore - app.update.enabled same (see #29611 (moved), but not sure how relevant is that for Android)
- In the code now I only see
media.autoplay.enabled.user-gestures-neededinstead ofmedia.autoplay.enabled, do we need to change this?
805a0b25be322d44138bae0d3a862070c9348fea
+ // Avoid throwing an error because Ci.nsIPushService isn't implemented + // All other clearing actions should succeed if we arrive here. + Promise.resolve();Promise.resolve(); is not really doing anything I think.
cdf51822421ad608cee5a2733a5eb37928da9484 - ok cd35aebe2de754c3fbc221ee4864cd5ffa162b16 - ok d8f702013cab81db385316f0c5bbd191574ef9ac - ok cab59d99f96bf37aaa016e987cb8729e1d551f1c -
+<!--#endif--> <uses-feature - android:name="android.hardware.microphone" + android:name="android.hardware.audio.low_latency" android:required="false"/> <uses-feature - android:name="android.hardware.camera.any" + android:name="android.hardware.microphone" android:required="false"/>These last two are outside of the ifdef, which is not the case in the original patch, why?
++#ifdef MOZ_ANDROID_LOCATION + <uses-permission android:name="com.android.browser.permission.READ_HISTORY_BOOKMARKS"/> + <uses-permission android:name="android.permission.FOREGROUND_SERVICE"/> ++#endifThis was originally in
Bug 25741 - TBA: Only include Firefox Account permissions if we want them, why was it moved here? Besides, FOREGROUND_SERVICE is new, not present in esr60. how do we know we also need to put it under MOZ_ANDROID_LOCATION?17db91c073521555d5d515a11a75348bde44b52a - ok 69401455d5e8ce26b69c6c8d033e5645775c157c - ok 47e7d5c8bc94039d3b240faa630e3186c649c49e - private.data.passwords was removed, do we need to do something to make sure they are cleared now? a7f568a6cd7c078a749a97fc8510e8fd962a46f1 - ok 61b94b2e7d92a816742a0babf0de92465f927c97 - ok e4f639671ce4b5c5f108e9fa3c841a2445434a19 - ok f7d637ab2f80dc54a3da69e6183676d7aaf0821d - Regarding the search engines, I guess it's fine to leave as it is for now. But as GeKo commented in #30429 (moved), it would be good at some point to solve #30017 (moved) and #30606 (moved). That could happen as a fixup of the
Omnibox: Add DDG, Startpage, Disconnect, Youtube...commit, maybe, and remove the changes from this one.0c62b14f6dc334ebef796f771f8b2e3bf623b989 - ok 8d56195c1bc6e15b5139d05606876da2736928d5 - ok eb0f60e48235872c141288e8bf3be44ac7c55a27 - ok 42c9bb7856f1f65b208ceb294ad881e9f4dfd66e - ok 998e9eb14df9691f9c1ae041ea085474a4379222 - ok b3262f7ef872b534b9f9804ab3fe7649a0e0a79e - ok f5ce3d9f078c4796f9f6e84e27970c82944c3859 - ok 7f42fb42206b5924dd4b769e1c2a3e22fa76eb56 - ok 4aaccecd9afeeac49990aaf45bb7e3c9a4c5613e - ok 8ee68edc2342c64ba629f40c8564664da261a3c8 - Don't we need
mobile/android/base/java/org/mozilla/gecko/CrashReporterActivity.javaandmobile/android/base/java/org/mozilla/gecko/updater/Updater.javatoo?aa6eecff7e7e7b3197b436d5ffa5694f19c6c050 - ok 5f43ed64c8311c80b1125ac34e7c5d251550e759 - ok
b7abcc2f4062fd56869827ecc181e93782ea8127 - A couple of changes from the original patch were dropped, why?
+ result.isOnionHost = this.isOnionHost(); + result.hasCert = !!this._lastStatus; // Don't show identity data for pages with an unknown identity or if any // mixed content is loaded (mixed display content is loaded by default). @@ -5757,7 +5769,7 @@ var IdentityHandler = { // hasMatchingOverride does not handle that, so avoid calling it. // Updating the tooltip value in those cases isn't critical. // FIXME: Fixing bug 646690 would probably makes this check unnecessary - if (this._lastLocation.hostname && + if (this._lastLocation.hostname && iData.cert &&8b7e055d36246e6ff8aa66e1aa5554dba676f3cc - ok 139207e5efb48b308b1fe37489299aa8d7e9c432 - ok 1ab5a30000edd23dd452c77922e4a20c124583ba - ok 394b735e8821cd41861b381ee95ff93299bded12 - ok feb7ccdd6b03c0c476ce5201833e942e33297db5 - ok 23a394180b896579d8254d158d523ee27a448bca - ok 9dadbe00e257efa034c3c2323449c7d138b9b427 - ok 3330fc0b90144b9fccef43aebdfd9e635ed17ee8 - Is this patch ready? I see it as need_revision. 8424156c8cb3c39953a1bd8d40cd384e2153c7d8 - ok
These two patches still need to be rebased, right? 41b333e3a9d3 Orfox: NetCipher enabled, checks if orbot is installed ?? 433766efba25 Orfox: add BroadcastReceiver to receive Tor status from Orbot
-
Replying to mcs:
Replying to sysrqb:
Bisecting leads to this commit (commit
d03d360bd773c8d8a97ef1f69e565afcc19796e5on acat's30429+5branch). This'll require some digging. mcs/brade, if you have any suggestions about where I should look, they'll be very much appreciated. ...Without knowing a lot about the Android-specific code, my guess is that the #9173 (closed) patch is just not correct by itself for Android. What is missing from acat's 30429+5 branch is a patch for #13252 (moved), which Kathy and I have been working on along with the updater patches. The #13252 (moved) patch adds the "side-by-side" TorBrowser-Data magic for macOS and also changes how the app data directory is determined on all platforms. As of yesterday, we have a "hot off the press" patch for #13252 (moved) which we plan to get to acat soon. I will attach a copy to this ticket so you can try it now.
One caveat: the ESR68 #13252 (moved) patch is intended to be inserted into the commit stream immediately after the #11641 (closed) patch (69c8b59f9cdc792ee533187723286fdd75dbabd0 on acat's 30429+5 branch). You may get conflicts if you tack it on the end.
Thanks, that patch did help.
Replying to acat:
Some comments on acat30429+5_tor-browser_android_68esr_39:
Thanks for review!
d0386613a4884393ff3324e43f8d06d30f9d6564 - this is not a patch in the original branch, does the build fail without this?
Yes, the build fails without this, I still must open a bugzilla ticket and uplift it.
546cab59acc827d7cf50ae227c2aa84f01c23c2d - ok d37580f4e17b6f3709d6906ef13441e29b61d86e - ok 4b1f9425adf546aecb094dc9a501c61e32fe3394 - In .mozconfig-android, should we delete instead of commenting out lines, to be consistent with what we did in desktop .mozconfigs?
+CC="/home/android/.mozbuild/clang/bin/clang" +CXX="/home/android/.mozbuild/clang/bin/clang++" Shouldn't this be removed?
Yes, I'll delete the commented-out lines. CC and CXX must be defined now (I believe), so I'd like to keep these. I'll try making them more general, so we can use them for troubleshooting in the future.
6041d5df003aed06f8dfe0349f53d2821777331b - ok f5c33693baf1d386c8bccedaee7cc13ce218a4b6 - ok 3c070da60f5d9b851d8c3d85d14e5dabf108d068 - ok 9fa528e6853c0538b8968d5980ab7e0fdbe9be1f - ok 1fd0ea742bdb74e99c484cbda24f4942aea2663c -
-
browser.mirroring.enabledpref seems not to be used anymore - app.update.enabled same (see #29611 (moved), but not sure how relevant is that for Android)
- In the code now I only see
media.autoplay.enabled.user-gestures-neededinstead ofmedia.autoplay.enabled, do we need to change this?
Thanks for looking at these. I haven't reviewed the prefs yet, so it is likely we can delete some of them, and we probably should add new ones.
805a0b25be322d44138bae0d3a862070c9348fea
+ // Avoid throwing an error because Ci.nsIPushService isn't implemented + // All other clearing actions should succeed if we arrive here. + Promise.resolve(); }}} Promise.resolve(); is not really doing anything I think.I don't actually know. I'll try testing this without that line and see if it still works.
cdf51822421ad608cee5a2733a5eb37928da9484 - ok cd35aebe2de754c3fbc221ee4864cd5ffa162b16 - ok d8f702013cab81db385316f0c5bbd191574ef9ac - ok cab59d99f96bf37aaa016e987cb8729e1d551f1c - {{{ + <uses-feature
-
android:name="android.hardware.microphone"
-
<uses-feature
android:name="android.hardware.audio.low_latency" android:required="false"/>
-
android:name="android.hardware.camera.any"
-
android:name="android.hardware.microphone" android:required="false"/>
}}} These last two are outside of the ifdef, which is not the case in the original patch, why?
I thought we needed them for a feature (the QRCode reader and voice dictation), but I looked at them again and you are correct that we don't need them. I moved them within the ifdef again.
{{{ ++#ifdef MOZ_ANDROID_LOCATION
-
<uses-permission android:name="com.android.browser.permission.READ_HISTORY_BOOKMARKS"/> -
<uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
++#endif }}} This was originally in
Bug 25741 - TBA: Only include Firefox Account permissions if we want them, why was it moved here? Besides, FOREGROUND_SERVICE is new, not present in esr60. how do we know we also need to put it under MOZ_ANDROID_LOCATION?I squashed all of the permissions-related commits into a single commit. For FOREGROUND_SERVICE, I wasn't sure if we needed it, so I excluded it for initial testing, but that results in a crash. I moved FOREGROUND_SERVICE out of the ifdef in a newer branch.
17db91c073521555d5d515a11a75348bde44b52a - ok 69401455d5e8ce26b69c6c8d033e5645775c157c - ok 47e7d5c8bc94039d3b240faa630e3186c649c49e - private.data.passwords was removed, do we need to do something to make sure they are cleared now? a7f568a6cd7c078a749a97fc8510e8fd962a46f1 - ok 61b94b2e7d92a816742a0babf0de92465f927c97 - ok e4f639671ce4b5c5f108e9fa3c841a2445434a19 - ok f7d637ab2f80dc54a3da69e6183676d7aaf0821d - Regarding the search engines, I guess it's fine to leave as it is for now. But as GeKo commented in #30429 (moved), it would be good at some point to solve #30017 (moved) and #30606 (moved). That could happen as a fixup of the
Omnibox: Add DDG, Startpage, Disconnect, Youtube...commit, maybe, and remove the changes from this one.Sounds good.
0c62b14f6dc334ebef796f771f8b2e3bf623b989 - ok 8d56195c1bc6e15b5139d05606876da2736928d5 - ok eb0f60e48235872c141288e8bf3be44ac7c55a27 - ok 42c9bb7856f1f65b208ceb294ad881e9f4dfd66e - ok 998e9eb14df9691f9c1ae041ea085474a4379222 - ok b3262f7ef872b534b9f9804ab3fe7649a0e0a79e - ok f5ce3d9f078c4796f9f6e84e27970c82944c3859 - ok 7f42fb42206b5924dd4b769e1c2a3e22fa76eb56 - ok 4aaccecd9afeeac49990aaf45bb7e3c9a4c5613e - ok 8ee68edc2342c64ba629f40c8564664da261a3c8 - Don't we need
mobile/android/base/java/org/mozilla/gecko/CrashReporterActivity.javaandmobile/android/base/java/org/mozilla/gecko/updater/Updater.javatoo?CrashReporterActivityis excluded because we disable crash-reporter at compile-time: https://searchfox.org/mozilla-esr68/source/mobile/android/app/build.gradle#119Updateris a little trickier, but that should never be used because we don't defineMOZ_UPDATERon Android builds.aa6eecff7e7e7b3197b436d5ffa5694f19c6c050 - ok 5f43ed64c8311c80b1125ac34e7c5d251550e759 - ok
b7abcc2f4062fd56869827ecc181e93782ea8127 - A couple of changes from the original patch were dropped, why? {{{
-
result.isOnionHost = this.isOnionHost();
-
result.hasCert = !!this._lastStatus;
// Don't show identity data for pages with an unknown identity or if any // mixed content is loaded (mixed display content is loaded by default). @@ -5757,7 +5769,7 @@ var IdentityHandler = { // hasMatchingOverride does not handle that, so avoid calling it. // Updating the tooltip value in those cases isn't critical. // FIXME: Fixing bug 646690 would probably makes this check unnecessary
- if (this._lastLocation.hostname &&
- if (this._lastLocation.hostname && iData.cert && }}}
These were due to merge conflict. The first diff did not change, but I did miss the additional iData.cert check in the second. Good catch, thanks.
> 8b7e055d36246e6ff8aa66e1aa5554dba676f3cc - ok > 139207e5efb48b308b1fe37489299aa8d7e9c432 - ok > 1ab5a30000edd23dd452c77922e4a20c124583ba - ok > 394b735e8821cd41861b381ee95ff93299bded12 - ok > feb7ccdd6b03c0c476ce5201833e942e33297db5 - ok > 23a394180b896579d8254d158d523ee27a448bca - ok > 9dadbe00e257efa034c3c2323449c7d138b9b427 - ok > 3330fc0b90144b9fccef43aebdfd9e635ed17ee8 - > Is this patch ready? I see it as need_revision. It needs some revisions, but it is currently shipping in alpha, so I think we should be consistent and rebase this patch onto 68esr, too. I need to look at that ticket again. > 8424156c8cb3c39953a1bd8d40cd384e2153c7d8 - ok > > These two patches still need to be rebased, right? > 41b333e3a9d3 Orfox: NetCipher enabled, checks if orbot is installed ?? > 433766efba25 Orfox: add BroadcastReceiver to receive Tor status from Orbot Not anymore. Those were original patches we used when Tor Browser used Orbot instead of a built-in tor. Now we ship our own integrated in the app, so we don't need those patches anymore.-
I'll cherry-pick all of these commits onto a branch based on
tor-browser-68.0esr-9.0-3for the next round of reviews, but this branch is based on acat's30429+5branch, like the previous one for consistency.The new branch for review is:
acat30429+5_tor-browser_android_68esr_54.Trac:
Status: needs_revision to needs_review