Opened 7 weeks ago

Last modified 10 days ago

#31010 needs_revision defect

Rebase Tor Browser mobile/ patches for Firefox ESR 68

Reported by: sysrqb Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile, tbb-9.0-must-nightly, TorBrowserTeam201908
Cc: mcs, brade Actual Points:
Parent ID: #30429 Points:
Reviewer: Sponsor:

Description

This is the Android-specific patches for #30429.

Child Tickets

Attachments (1)

0001-Bug-13252-Do-not-store-data-in-the-app-bundle.patch (22.1 KB) - added by mcs 10 days ago.
ESR68 rebased patch for #13252

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 weeks ago by sysrqb

Copying from the parent ticket:

Replying to 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

comment:2 Changed 7 weeks ago by sysrqb

I successfully built a branch based on acat's 30429+1 branch. The branch with the Android patches is in my repo as branch acat30429+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.

comment:3 Changed 7 weeks ago by gk

I've not checked the rebased commits yet but while looking at #28672 I realized we still have the tor-browser-build patch for #29575, which I think we should move into tor-browser. Could you move android-dependencies.patch over to that repo and integrate it into your rebased patches?

comment:4 Changed 7 weeks ago by gk

Keywords: TorBrowserTeam201907R added; TorBrowserTeam201906R removed

No reviews in June 2019 anymore, moving them.

comment:5 Changed 4 weeks ago by gk

Keywords: tbb-9.0-must-nightly added

Starting with 9.0 keywords

comment:6 Changed 2 weeks ago by gk

Keywords: TorBrowserTeam201908R added; TorBrowserTeam201907R removed

No July any longer.

comment:7 Changed 11 days ago by sysrqb

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 EXITED

Because it is setting JNI_READY, it seems like this is failing after XPCom initialization.

comment:8 Changed 11 days ago by sysrqb

But in the meantime, the current branch I'm using is acat30429+5_tor-browser_android_68esr_39 in my public repo. There are a few fixup commits on that branch, but I think most of the commits are stable.

comment:9 Changed 10 days ago by sysrqb

Cc: mcs brade added

Bisecting leads to this commit (commit d03d360bd773c8d8a97ef1f69e565afcc19796e5 on acat's 30429+5 branch). 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 1

I'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_READY

and I believe that is set at the end of the Initialize() call (after a few layers of indirection, assuming I traced this correctly).

comment:10 in reply to:  9 Changed 10 days ago by mcs

Replying to sysrqb:

Bisecting leads to this commit (commit d03d360bd773c8d8a97ef1f69e565afcc19796e5 on acat's 30429+5 branch). 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 patch is just not correct by itself for Android. What is missing from acat's 30429+5 branch is a patch for #13252, which Kathy and I have been working on along with the updater patches. The #13252 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 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 patch is intended to be inserted into the commit stream immediately after the #11641 patch (69c8b59f9cdc792ee533187723286fdd75dbabd0 on acat's 30429+5 branch). You may get conflicts if you tack it on the end.

Changed 10 days ago by mcs

ESR68 rebased patch for #13252

comment:11 Changed 10 days ago by acat

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.enabled pref seems not to be used anymore
  • app.update.enabled same (see #29611, but not sure how relevant is that for Android)
  • In the code now I only see media.autoplay.enabled.user-gestures-needed instead of media.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"/>
  ++#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?

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, it would be good at some point to solve #30017 and #30606.
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.java and mobile/android/base/java/org/mozilla/gecko/updater/Updater.java too?

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

comment:12 Changed 10 days ago by acat

Keywords: TorBrowserTeam201908 added; TorBrowserTeam201908R removed
Status: newneeds_revision
Note: See TracTickets for help on using tickets.