Opened 2 weeks ago

Last modified 42 hours ago

#28640 needs_revision defect

System addon does not override app-profile addon

Reported by: sysrqb Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile, TorBrowserTeam201812, TBA-a3
Cc: igt0, gk, mcs, brade Actual Points:
Parent ID: Points:
Reviewer: Sponsor: Sponsor8

Description

In #25013, torbutton was added as a system addon in TBA, but it seems like the app continues using the version in the profile after the app upgrade. I toggled extensions.logging.enabled, these are some of the log entries.

torbutton is copied into the app's writable storage, but it isn't installed. It also doesn't find the new preferences.json file.

11-28 01:23:40.492  8476  8493 D GeckoProfile: Found profile dir.
11-28 01:23:40.539  8476  8476 D StrictMode:    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
11-28 01:23:40.546  8476  8493 D PostUpdateHandler: Build ID changed since last start: '20181128012042', '20181128011144'
11-28 01:23:40.546  8476  8493 D PostUpdateHandler: Copying system add-ons from APK to dataDir
11-28 01:23:40.548  8476  8493 D PostUpdateHandler: Copying 'features/torbutton@torproject.org.xpi' from APK to dataDir
11-28 01:23:40.552  8476  8493 D PostUpdateHandler: Creating /data/user/0/org.torproject.torbrowser_alpha/features
11-28 01:23:40.558  8476  8493 D GeckoSessInfo: Recording start of session: 1543368220513
11-28 01:23:40.558  8476  8493 D GeckoDistribution: Getting file from distribution.
11-28 01:23:40.558  8476  8493 E GeckoDistribution: Distribution directory exists, but no file named preferences.json
11-28 01:23:40.558  8476  8493 D GeckoSearchEngineManager: Found default engine name in SharedPreferences: DuckDuckGo
11-28 01:23:40.558  8476  8493 D GeckoDistribution: Getting file from distribution.
11-28 01:23:43.111  8476  8496 I Gecko   : 1543368223110        addons.manager  DEBUG   Loaded provider scope for resource://gre/modules/addons/XPIProvider.jsm: ["XPIProvider", "XPIInternal"]
11-28 01:23:43.114  8476  8496 I Gecko   : 1543368223114        addons.manager  DEBUG   Loaded provider scope for resource://gre/modules/LightweightThemeManager.jsm: ["LightweightThemeManager"]
11-28 01:23:43.137  8476  8496 I Gecko   : 1543368223136        addons.manager  DEBUG   Loaded provider scope for resource://gre/modules/addons/GMPProvider.jsm
11-28 01:23:43.137  8476  8496 I Gecko   : 1543368223137        addons.manager  DEBUG   Starting provider: XPIProvider
11-28 01:23:43.137  8476  8496 I Gecko   : 1543368223137        addons.xpi      DEBUG   startup
11-28 01:23:43.139  8476  8496 I Gecko   : 1543368223139        addons.xpi      INFO    SystemAddonInstallLocation directory is missing
11-28 01:23:43.140  8476  8496 I Gecko   : 1543368223139        addons.xpi      DEBUG   checkForChanges
11-28 01:23:43.141  8476  8496 I Gecko   : 1543368223140        addons.xpi      DEBUG   Loaded add-on state: {"app-profile":{"addons":{"{73a6fe31-595d-460b-a920-fcc0f8843232}":{"enabled":true,"lastModifiedTime":1543367768000,"path":"{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi","signedState":2,"version":"10.2.0","type":"webextension","bootstrapped":true,"dependencies":[],"runInSafeMode":false,"hasEmbeddedWebExtension":false},"https-everywhere-eff@eff.org":{"enabled":true,"lastModifiedTime":1543367678000,"path":"https-everywhere-eff@eff.org.xpi","signedState":2,"version":"2018.4.11","type":"webextension","bootstrapped":true,"dependencies":[],"runInSafeMode":false,"hasEmbeddedWebExtension":false},"torbutton@torproject.org":{"enabled":true,"lastModifiedTime":1543367720000,"path":"torbutton@torproject.org.xpi","version":"2.1.1"}},"path":"/data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions"}}
11-28 01:23:43.142  8476  8496 I Gecko   : 1543368223142        addons.xpi      INFO    Mapping {73a6fe31-595d-460b-a920-fcc0f8843232} to /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions/{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi
11-28 01:23:43.143  8476  8496 I Gecko   : 1543368223143        addons.xpi      INFO    Mapping https-everywhere-eff@eff.org to /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions/https-everywhere-eff@eff.org.xpi
11-28 01:23:43.143  8476  8496 I Gecko   : 1543368223143        addons.xpi      INFO    Mapping torbutton@torproject.org to /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions/torbutton@torproject.org.xpi
11-28 01:23:43.144  8476  8496 I Gecko   : 1543368223144        addons.xpi      INFO    Mapping {73a6fe31-595d-460b-a920-fcc0f8843232} to /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions/{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi
11-28 01:23:43.144  8476  8496 I Gecko   : 1543368223144        addons.xpi      INFO    Mapping https-everywhere-eff@eff.org to /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions/https-everywhere-eff@eff.org.xpi
11-28 01:23:43.144  8476  8496 I Gecko   : 1543368223144        addons.xpi      INFO    Mapping torbutton@torproject.org to /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions/torbutton@torproject.org.xpi
11-28 01:23:43.145  8476  8496 I Gecko   : 1543368223145        addons.xpi      DEBUG   Existing add-on {73a6fe31-595d-460b-a920-fcc0f8843232} in app-profile
11-28 01:23:43.145  8476  8496 I Gecko   : 1543368223145        addons.xpi      DEBUG   Existing add-on https-everywhere-eff@eff.org in app-profile
11-28 01:23:43.146  8476  8496 I Gecko   : 1543368223146        addons.xpi      DEBUG   Existing add-on torbutton@torproject.org in app-profile
11-28 01:23:43.148  8476  8496 I Gecko   : 1543368223147        addons.xpi      WARN    List of valid built-in add-ons could not be parsed.: [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsIXPCComponents_Utils.readUTF8URI]"  nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)"  location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: _readAddons :: line 6484"  data: no] Stack trace: _readAddons()@resource://gre/modules/addons/XPIProvider.jsm:6484
11-28 01:23:43.148  8476  8496 I Gecko   : getAddonLocations()@resource://gre/modules/addons/XPIProvider.jsm:6145
11-28 01:23:43.148  8476  8496 I Gecko   : getInstallState()@resource://gre/modules/addons/XPIProvider.jsm:1601
11-28 01:23:43.148  8476  8496 I Gecko   : checkForChanges()@resource://gre/modules/addons/XPIProvider.jsm:3295
11-28 01:23:43.148  8476  8496 I Gecko   : startup()@resource://gre/modules/addons/XPIProvider.jsm:2203
11-28 01:23:43.148  8476  8496 I Gecko   : callProvider()@resource://gre/modules/AddonManager.jsm:258
11-28 01:23:43.148  8476  8496 I Gecko   : _startProvider()@resource://gre/modules/AddonManager.jsm:733
11-28 01:23:43.148  8476  8496 I Gecko   : startup()@resource://gre/modules/AddonManager.jsm:921
11-28 01:23:43.148  8476  8496 I Gecko   : startup()@resource://gre/modules/AddonManager.jsm:3005
11-28 01:23:43.148  8476  8496 I Gecko   : observe()@jar:jar:file:///data/app/org.torproject.torbrowser_alp
11-28 01:23:43.149  8476  8496 I Gecko   : 1543368223149        addons.xpi      DEBUG   getInstallState changed: false, state: {}
11-28 01:23:43.150  8476  8496 I Gecko   : 1543368223150        addons.xpi      INFO    SystemAddonInstallLocation directory is missing
11-28 01:23:43.154  8476  8496 I Gecko   : 1543368223154        addons.xpi      DEBUG   No changes found
11-28 01:23:43.155  8476  8496 I Gecko   : 1543368223155        addons.xpi      DEBUG   Loading bootstrap scope from /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions/https-everywhere-eff@eff.org.xpi
11-28 01:23:43.179  8476  8496 I Gecko   : 1543368223179        addons.xpi      DEBUG   Calling bootstrap method startup on https-everywhere-eff@eff.org version 2018.4.11
11-28 01:23:43.183  8476  8496 I Gecko   : 1543368223183        addons.xpi      DEBUG   Loading bootstrap scope from /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions/{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi
11-28 01:23:43.184  8476  8496 I Gecko   : 1543368223184        addons.xpi      DEBUG   Calling bootstrap method startup on {73a6fe31-595d-460b-a920-fcc0f8843232} version 10.2.0
11-28 01:23:43.187  8476  8496 I Gecko   : 1543368223187        addons.manager  DEBUG   Registering shutdown blocker for XPIProvider
11-28 01:23:43.187  8476  8496 I Gecko   : 1543368223187        addons.manager  DEBUG   Provider finished startup: XPIProvider
11-28 01:23:43.795  8476  8496 I Gecko   : 1543368223795        addons.xpi-utils        DEBUG   Starting async load of XPI database /data/user/0/org.torproject.torbrowser_alpha/files/mozilla/rb02if1q.default/extensions.json
11-28 01:23:44.104  8476  8496 I Gecko   : 1543368224104        addons.xpi-utils        DEBUG   Async JSON file read took 0 MS
11-28 01:23:44.104  8476  8496 I Gecko   : 1543368224104        addons.xpi-utils        DEBUG   Finished async read of XPI database, parsing...
11-28 01:23:44.107  8476  8496 I Gecko   : 1543368224107        addons.xpi-utils        DEBUG   Successfully read XPI database
11-28 01:23:44.124  8476  8496 D GeckoDistribution: Custom distribution directory not found.

Child Tickets

Change History (15)

comment:1 Changed 2 weeks ago by sysrqb

A fresh installation where torbutton is installed as a system addon look like this (I don't see any log entries where it is explicitly installed...but it is and it works):

11-28 02:19:07.403  8977  8994 D GeckoProfile: Created new profile dir.
11-28 02:19:07.406  8977  8994 I GeckoProfile: Enqueuing profile init.
11-28 02:19:07.408  8977  9000 E GeckoLibLoad: Load sqlite done
11-28 02:19:07.408  8977  9000 E GeckoLibLoad: Load nss start
11-28 02:19:07.408  8977  9000 E GeckoLibLoad: Load nss done
11-28 02:19:07.409  8977  8994 D GeckoProfile: Found profile dir.
11-28 02:19:07.409  8977  8994 D GeckoProfile: Attempting to write new client ID
11-28 02:19:07.424  8977  8994 D GeckoDistribution: Creating /data/user/0/org.torproject.torbrowser_alpha/distribution/extensions
11-28 02:19:07.474  8977  8977 D GeckoDistribution: Custom distribution directory not found.
11-28 02:19:07.551  8977  8994 D GeckoDistribution: Getting file from distribution.
11-28 02:19:07.563  8977  8994 D PostUpdateHandler: Build ID changed since last start: '20181128015826', 'null'
11-28 02:19:07.563  8977  8994 D PostUpdateHandler: Copying system add-ons from APK to dataDir
11-28 02:19:07.565  8977  8994 D PostUpdateHandler: Copying 'features/torbutton@torproject.org.xpi' from APK to dataDir
11-28 02:19:07.566  8977  8994 D PostUpdateHandler: Creating /data/user/0/org.torproject.torbrowser_alpha/features
11-28 02:19:07.567  8977  8994 D GeckoSessInfo: Recording start of session: 1543371547435
11-28 02:19:07.567  8977  8994 D GeckoDistribution: Getting file from distribution.
11-28 02:19:07.567  8977  8994 E GeckoDistribution: Distribution preferences.json has no Global entry!
11-28 02:19:07.568  8977  8994 D GeckoApplication: Running post-distribution task: bookmarks.
11-28 02:19:07.568  8977  8994 D GeckoDistribution: Getting file from distribution.
11-28 02:19:07.568  8977  8994 E GeckoDistribution: Distribution directory exists, but no file named bookmarks.json
11-28 02:19:07.738  8977  8994 D GeckoApplication: Running post-distribution task: android preferences.
11-28 02:19:07.738  8977  8994 D GeckoDistribution: Getting file from distribution.
11-28 02:19:07.740  8977  8994 D GeckoDistribution: Getting file from distribution.
11-28 02:19:07.745  8977  8994 D GeckoDistribution: Getting file from distribution.
11-28 02:19:07.746  8977  8994 I GeckoSearchEngineManager: This is Tor Browser. Skipping.
11-28 02:19:07.746  8977  8994 D GeckoSearchEngineManager: Found default engine name in browsersearch.json.
11-28 02:19:07.746  8977  8994 D GeckoDistribution: Getting file from distribution.
11-28 02:19:07.746  8977  8994 E GeckoDistribution: Distribution directory exists, but no file named searchplugins
11-28 02:19:10.293  8977  9000 I Gecko   : 1543371550290        addons.xpi      WARN    List of valid built-in add-ons could not be parsed.: [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsIXPCComponents_Utils.readUTF8URI]"  nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)"  location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: _readAddons :: line 6484"  data: no] Stack trace: _readAddons()@resource://gre/modules/addons/XPIProvider.jsm:6484
11-28 02:19:10.293  8977  9000 I Gecko   : getAddonLocations()@resource://gre/modules/addons/XPIProvider.jsm:6145
11-28 02:19:10.293  8977  9000 I Gecko   : getInstallState()@resource://gre/modules/addons/XPIProvider.jsm:1601
11-28 02:19:10.293  8977  9000 I Gecko   : checkForChanges()@resource://gre/modules/addons/XPIProvider.jsm:3295
11-28 02:19:10.293  8977  9000 I Gecko   : startup()@resource://gre/modules/addons/XPIProvider.jsm:2203
11-28 02:19:10.293  8977  9000 I Gecko   : callProvider()@resource://gre/modules/AddonManager.jsm:258
11-28 02:19:10.293  8977  9000 I Gecko   : _startProvider()@resource://gre/modules/AddonManager.jsm:733
11-28 02:19:10.293  8977  9000 I Gecko   : startup()@resource://gre/modules/AddonManager.jsm:921
11-28 02:19:10.293  8977  9000 I Gecko   : startup()@resource://gre/modules/AddonManager.jsm:3005
11-28 02:19:10.293  8977  9000 I Gecko   : observe()@jar:jar:file:///data/app/org.torproject.torbrowser_alp
11-28 02:19:10.394  8977  9000 D GeckoThread: State changed to PROFILE_READY
11-28 02:19:10.898  8977  9000 D GeckoDistribution: Custom distribution directory not found.

comment:2 Changed 2 weeks ago by sysrqb

Status: newneeds_information

I think it is intentional that the app profile extensions override the system addon: in XPIProvider - at least this is how I am reading it.

I wonder if we should do something slightly hacky. In the PostUpdateHandler, when the system extensions are copied into the features/ dir, we check if the same addon is already installed in the profile, and we uninstall it if it is found. (I don't know how to initiate an uninstall of an addon from Java - the javascript is context sensitive - plus it requires another restart before it is it fully uninstalled, but that's another problem).

The third problem is preferences.json isn't copied from the apk into the app's data directory. I think this is because the the app sets a preference (distribution_state) when initialization completes, and then it never copies new files from newer APKs during an update.

comment:3 Changed 2 weeks ago by sysrqb

Also, in the release announcement, we should say the Security Slider setting will be reset. If the user changed the setting, they should reset it.

comment:4 in reply to:  2 ; Changed 2 weeks ago by gk

Cc: mcs brade added
Keywords: TorBrowserTeam201811 added
Status: needs_informationnew

Replying to sysrqb:

I think it is intentional that the app profile extensions override the system addon: in XPIProvider - at least this is how I am reading it.

That's actually okay I think. The bug here is that the there is an integral extension, which we don't ship anymore compared to the previous bundle, still available after the update and not deleted.

For Linux/Windows we get that essentially for free as the profile is included inside of the bundle and the logic determining which items to remove/replace is running across the profiles on those platforms as well.

For macOS the case is different as we needed to get the profile outside of the bundle directory for code signing purposes (see: #13252, and note that we plan to do so for Linux and Windows, too, as there are a bunch of bugs with our current way of doing things on those platforms, see: #18367 and #18369). We solved that problem but I currently can't find the details, mcs/brade might know. That in turn might help to find a good solution for Android case and/or might help thinking through whether desktop platforms would be affected the same once we transition to a built-in Torbutton for them, too.

I wonder if we should do something slightly hacky. In the PostUpdateHandler, when the system extensions are copied into the features/ dir, we check if the same addon is already installed in the profile, and we uninstall it if it is found.

Sounds fine with me if we don't fine a better plan.

(I don't know how to initiate an uninstall of an addon from Java - the javascript is context sensitive - plus it requires another restart before it is it fully uninstalled, but that's another problem).

Yeah, le't not go that route.

The third problem is preferences.json isn't copied from the apk into the app's data directory. I think this is because the the app sets a preference (distribution_state) when initialization completes, and then it never copies new files from newer APKs during an update.

That's annoying. I think we should make exceptions for the files we care about. Or we could just put some logic in the PostUpdateHandler here as well.

comment:5 in reply to:  4 ; Changed 2 weeks ago by mcs

Replying to gk:

For macOS the case is different as we needed to get the profile outside of the bundle directory for code signing purposes (see: #13252, and note that we plan to do so for Linux and Windows, too, as there are a bunch of bugs with our current way of doing things on those platforms, see: #18367 and #18369). We solved that problem but I currently can't find the details, mcs/brade might know. That in turn might help to find a good solution for Android case and/or might help thinking through whether desktop platforms would be affected the same once we transition to a built-in Torbutton for them, too.

On macOS, when we transitioned from "profile embedded within the app" to the "side-by-side" TorBrowser-Data approach, the normal update mechanism took care of removing all of the extensions from the embedded browser profile. Here is the current #13252 patch for reference:
https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-60.3.0esr-8.5-1&id=86e3b0eabbd3b1f02d755399074e511eb5784aa2

The patch does include some migration code which moves the browser profile over to TorBrowser-Data, but I don't think that code needs to do anything special for the extensions (since they will be removed when the MAR-based update has been applied, and that occurs before the profile migration code runs). If you want to see the migration code, start by looking at migrateOneTorBrowserDataDir() inside toolkit/xre/nsAppRunner.cpp.

We also added some code to help with preferences overrides. From the commit message:

All .js files in distribution/preferences (on Mac OS, Contents/Resources/distribution/preferences) will be copied to the preferences directory within the user's browser profile when the profile is created and each time Tor Browser is updated.

I don't think we rely on that code any longer due to changes made for Firefox ESR 60, but you can see the relevant code by looking at the changes to toolkit/mozapps/extensions/internal/XPIProvider.jsm as part of the #13252 patch.

One crazy idea that might work as a quick-and-dirty solution would be to blocklist the old Torbutton. Using that approach would require using a new extension ID for the system add-on... which is inconvenient. I am sure we will encounter this same issue for desktop (at least on macOS), and we will probably need to add code to toolkit/mozapps/extensions/internal/XPIProvider.jsm that removes all traces of the old profile-based extension.

comment:6 in reply to:  5 Changed 2 weeks ago by sysrqb

Replying to mcs:

Replying to gk:

For macOS the case is different as we needed to get the profile outside of the bundle directory for code signing purposes (see: #13252, and note that we plan to do so for Linux and Windows, too, as there are a bunch of bugs with our current way of doing things on those platforms, see: #18367 and #18369). We solved that problem but I currently can't find the details, mcs/brade might know. That in turn might help to find a good solution for Android case and/or might help thinking through whether desktop platforms would be affected the same once we transition to a built-in Torbutton for them, too.

On macOS, when we transitioned from "profile embedded within the app" to the "side-by-side" TorBrowser-Data approach, the normal update mechanism took care of removing all of the extensions from the embedded browser profile. Here is the current #13252 patch for reference:
https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-60.3.0esr-8.5-1&id=86e3b0eabbd3b1f02d755399074e511eb5784aa2

The patch does include some migration code which moves the browser profile over to TorBrowser-Data, but I don't think that code needs to do anything special for the extensions (since they will be removed when the MAR-based update has been applied, and that occurs before the profile migration code runs). If you want to see the migration code, start by looking at migrateOneTorBrowserDataDir() inside toolkit/xre/nsAppRunner.cpp.

We also added some code to help with preferences overrides. From the commit message:

All .js files in distribution/preferences (on Mac OS, Contents/Resources/distribution/preferences) will be copied to the preferences directory within the user's browser profile when the profile is created and each time Tor Browser is updated.

I don't think we rely on that code any longer due to changes made for Firefox ESR 60, but you can see the relevant code by looking at the changes to toolkit/mozapps/extensions/internal/XPIProvider.jsm as part of the #13252 patch.

Thanks! Part of the problem we're having on Android is the update mechanism is completely different than on desktop - so we don't benefit from the same MAR logic. I do think, and I agree, the torbutton transition should affect Mac OS the say way it affects Android, though.

One crazy idea that might work as a quick-and-dirty solution would be to blocklist the old Torbutton. Using that approach would require using a new extension ID for the system add-on... which is inconvenient. I am sure we will encounter this same issue for desktop (at least on macOS), and we will probably need to add code to toolkit/mozapps/extensions/internal/XPIProvider.jsm that removes all traces of the old profile-based extension.

Interesting idea, and I don't think it's too crazy. One other idea I have is we can conditionally ignore a torbutton@torproject.org xpi during startup if it's not a system addon:

https://gitweb.torproject.org/tor-browser.git/tree/toolkit/mozapps/extensions/internal/XPIProvider.jsm?h=tor-browser-60.3.0esr-8.5-1#n1600

comment:7 Changed 13 days ago by gk

Keywords: TBA-a2 added

comment:8 Changed 13 days ago by gk

Okay, I have something for the Torbutton issue (not sure where you were with the preferences file but it seemed you were in good shape with it). Testing this has been a pain in the butt but I think I now have a setup where I can develop code without rebuilding the whole .apk and test the update case. The following works for me (it still needs to get applied only on Android and not on all platforms):

diff --git a/toolkit/mozapps/extensions/internal/XPIProvider.jsm b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
index a8be063ddbde..20544a3d37d4 100644
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -1601,6 +1601,13 @@ var XPIStates = {
       for (let [id, file] of location.getAddonLocations(true)) {
         knownIds.delete(id);
 
+        if (id === "torbutton@torproject.org" &&
+            location.name === KEY_APP_PROFILE) {
+          location.uninstallAddon(id);
+          changed = true;
+          continue;
+        }
+
         let xpiState = loc.get(id);
         if (!xpiState) {
           logger.debug("New add-on ${id} in ${location}", {id, location: location.name});
-- 
2.19.2

I *think* that should be all we need but XPIProvider.jsm code is quite complex. Thus, I could have missed some bit. I chose uninstallAddon() as this does a variety of things which could be useful, including deleting the file and I set changed to true in order to rebuild all databases if needed and get rid of old state. The log shows that Torbutton is taking from the update and no old one is initialized. It seems HTTPS-E and NoScript are not showing up in the menu anymore but that has happened on my phone before randomly. So, might just be a correlation.

Feel free to use it if you think it's worthwhile. :)

comment:9 in reply to:  8 ; Changed 12 days ago by sysrqb

Replying to gk:

I *think* that should be all we need but XPIProvider.jsm code is quite complex. Thus, I could have missed some bit. I chose uninstallAddon() as this does a variety of things which could be useful, including deleting the file and I set changed to true in order to rebuild all databases if needed and get rid of old state. The log shows that Torbutton is taking from the update and no old one is initialized. It seems HTTPS-E and NoScript are not showing up in the menu anymore but that has happened on my phone before randomly. So, might just be a correlation.

Feel free to use it if you think it's worthwhile. :)

Thanks! I like this patch better than patching every place in XPIProvider where it reads the extensions on disk. I have a branch for this, but I am also see the Https-E and NoScript issue. The branch is 28640 in my user repo.

comment:10 in reply to:  9 Changed 12 days ago by sysrqb

Status: newneeds_review

Replying to sysrqb:

Replying to gk:

I *think* that should be all we need but XPIProvider.jsm code is quite complex. Thus, I could have missed some bit. I chose uninstallAddon() as this does a variety of things which could be useful, including deleting the file and I set changed to true in order to rebuild all databases if needed and get rid of old state. The log shows that Torbutton is taking from the update and no old one is initialized. It seems HTTPS-E and NoScript are not showing up in the menu anymore but that has happened on my phone before randomly. So, might just be a correlation.

Feel free to use it if you think it's worthwhile. :)

Thanks! I like this patch better than patching every place in XPIProvider where it reads the extensions on disk. I have a branch for this, but I am also see the Https-E and NoScript issue. The branch is 28640 in my user repo.

Okay, I have a better patch now - same branch (sorry, rebased). This branch should provide a better experience on fresh installations with loading about:tor when the user closes the onboard screen.

Preliminary testing looks good, and upgrades are successful. The one remaining bug I see occasionally is the onboarding screen is shown after restarting the app. That still needs some more investigation, but I don't think it's critical or a blocker.

comment:11 Changed 12 days ago by gk

Status: needs_reviewneeds_revision

Thanks for the work, sysrqb! Nicely done. I picked the patches more or less as-is to get the build going. The result of that is 4 commits on tor-browser-60.3.0esr-8.5-1 (commit 62ec69b09ff02649b3eab64a08fa87549c0bfc16, 20f13426dec48e7b3b1df2cbd6cc41b3432fa267, bd3d1614a144377b32f5f548ce235c8ae0fb01d8, and 10ca4718681668db7b0bdba0e3dd207dbe352cd8).

Review comments:

5f3ee64a5dcfeb884c56ee12124d0ad6166badff - I don't think there is a need to invoke all the preprocessing machinery here. We could do like we did in #27763 I think, just using AppConstants.platform === "android" (which we already have in XPIInstall.jsm)

d43b65d65952ac47f6cd4cc12237dbd4a51fa36b - okay

d56596ea32fd4499a0d6656fa5e80689110da263 - okay

c32f80a0ad33140d1058df9845cbea92938c5c10 - this needs some considerable clean-up. I think we should make the result another commit starting with "Bug 28507". That way all the things are grouped together and easier to find.

Could you try making the summaries of your commit message shorter (I know it is hard sometimes!)? Having those super long lines makes it harder to read through the log in a terminal (I generally found https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html to be reasonable with good arguments but that's just a personal opinion).

comment:12 Changed 6 days ago by gk

Keywords: TorBrowserTeam201812 added; TorBrowserTeam201811 removed

Moving items to December

comment:13 Changed 45 hours ago by gk

Keywords: TBA-a3 added

Setting tag for third Tor Browser for Android alpha milestone.

comment:14 Changed 44 hours ago by gk

Keywords: TBA-a2 removed

We are beyond TBA-a2, TBA-a3 is the new black.

comment:15 Changed 42 hours ago by gk

Sponsor: Sponsor8

+ 2 for sponsor8.

Note: See TracTickets for help on using tickets.