Opened 3 years ago

Closed 6 months ago

Last modified 6 months ago

#23719 closed defect (wontfix)

Make sure WebExtensions are spared from JIT disabling in higher security settings (Medium-High)

Reported by: cypherpunks Owned by: acat
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-performance, tbb-9.5a7-want, TorBrowserTeam202006
Cc: fdsfgs@…, tbb-team Actual Points: 0.5
Parent ID: Points: 1
Reviewer: Sponsor:

Description

This could for example negatively affect HTTPS Everywhere's performance. I have however no data on whether JIT is disabled for WebExtensions in this case.

Child Tickets

Change History (37)

comment:1 Changed 3 years ago by tokotoko

Cc: fdsfgs@… added

comment:2 Changed 3 years ago by cypherpunks

Keywords: ff59-esr added

comment:3 Changed 3 years ago by cypherpunks

Keywords: tbb-performance added; ff59-esr removed
Severity: NormalMajor

I just confirmed it, at Medium-High security setting, JIT is disabled for WebExtensions resulting in lower performance.

comment:4 Changed 2 years ago by cypherpunks

Can this be given a higher prio? It's seems to severely affect my browsing as some tabs take a while to even start loading with Safer-Safest (seems related to NoScript doing its work).

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

Status: newneeds_information

Replying to cypherpunks:

Can this be given a higher prio? It's seems to severely affect my browsing as some tabs take a while to even start loading with Safer-Safest (seems related to NoScript doing its work).

Is that reproducible (e.g. by loading a particular URL)? I am curious where the NoScript relation is coming from.

comment:6 in reply to:  5 ; Changed 2 years ago by cypherpunks

Replying to gk:

Is that reproducible (e.g. by loading a particular URL)? I am curious where the NoScript relation is coming from.

Thanks for taking interest in this. It's a bit difficult to reproduce immediately but sometimes when I'm on a page and I ctrl+click on a link so that it opens in a new tab, the cpu usage gets to 100% on one core for about 10sec and it's taking 20sec or so just to starting loading up the page, from the browser console I see NoScript doing its thing (i.e. the CSP hack) during that time. (on 8.0a9 of course) It doesn't happen always so it's difficult to give clear ways to reproduce other than "just browse with safer security setting and ctrl+click on links".

comment:7 in reply to:  6 ; Changed 2 years ago by gk

Replying to cypherpunks:

Replying to gk:

Is that reproducible (e.g. by loading a particular URL)? I am curious where the NoScript relation is coming from.

Thanks for taking interest in this. It's a bit difficult to reproduce immediately but sometimes when I'm on a page and I ctrl+click on a link so that it opens in a new tab, the cpu usage gets to 100% on one core for about 10sec and it's taking 20sec or so just to starting loading up the page, from the browser console I see NoScript doing its thing (i.e. the CSP hack) during that time. (on 8.0a9 of course) It doesn't happen always so it's difficult to give clear ways to reproduce other than "just browse with safer security setting and ctrl+click on links".

Is that on Windows? Or Linux? Or...

comment:8 in reply to:  7 Changed 2 years ago by cypherpunks

Replying to gk:

Is that on Windows? Or Linux? Or...

Yeah Linux.

comment:9 Changed 2 years ago by cypherpunks

Maybe it has nothing to do with JIT after all (though that doesn't mean that this ticket shouldn't be addressed) but such relief came when ye finally catch the fish:

[NoScript] Cannot collect noscript activity data Error: Could not establish connection. Receiving end does not exist.
Stack trace:
collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
 Could not establish connection. Receiving end does not exist. collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
  log.js:12:5
	error moz-extension://[NoScript's UUID]/lib/log.js:12:5
	collectSeen moz-extension://[NoScript's UUID]/bg/main.js:274:10
[NoScript] Cannot collect noscript activity data Error: Could not establish connection. Receiving end does not exist.
Stack trace:
collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
 Could not establish connection. Receiving end does not exist. collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
  log.js:12:5
	error moz-extension://[NoScript's UUID]/lib/log.js:12:5
	collectSeen moz-extension://[NoScript's UUID]/bg/main.js:274:10
[NoScript] Cannot collect noscript activity data Error: Could not establish connection. Receiving end does not exist.
Stack trace:
collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
 Could not establish connection. Receiving end does not exist. collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
  log.js:12:5
	error moz-extension://[NoScript's UUID]/lib/log.js:12:5
	collectSeen moz-extension://[NoScript's UUID]/bg/main.js:274:10
[NoScript] Cannot collect noscript activity data Error: Could not establish connection. Receiving end does not exist.
Stack trace:
collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
 Could not establish connection. Receiving end does not exist. collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
  log.js:12:5
	error moz-extension://[NoScript's UUID]/lib/log.js:12:5
	collectSeen moz-extension://[NoScript's UUID]/bg/main.js:274:10
NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIWebNavigation.loadURIWithOptions]  browser-child.js:359
[NoScript] Cannot collect noscript activity data Error: Could not establish connection. Receiving end does not exist.
Stack trace:
collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
 Could not establish connection. Receiving end does not exist. collectSeen@moz-extension://[NoScript's UUID]/bg/main.js:265:38
  log.js:12:5
	error moz-extension://[NoScript's UUID]/lib/log.js:12:5
	collectSeen moz-extension://[NoScript's UUID]/bg/main.js:274:10
NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIWebNavigation.loadURIWithOptions]  browser-child.js:359

comment:10 Changed 2 years ago by cypherpunks_reply

You can easily observe the increase in extension initialization time with different security slider settings by disabling and enabling extensions in about:addons. During this initialization time the browser UI is unresponsive.

In normal usage (on my machine anyway) this delay mostly lengthens the time during startup between Tor connecting and the browser window appearing.

With the security slider set to standard, uBlock Origin with most of the default filters enabled initializes after about 5 seconds of one cpu core pegged at 100%. With the security slider set to safer, it takes 25 seconds.

I testd with Tor Browser 8.0, Windows 10, a quad-core 3 GHz CPU and plenty of free RAM. Hardware acceleration is disabled and content processes are reduced to 1 although raising them to 2 did not change the browser startup delay or the unresponsive UI in the disable / enable scenario.

comment:11 Changed 16 months ago by tom

the flag for enabling wasm is on the JSContext

https://searchfox.org/mozilla-central/rev/c0ca77697c6868482f30af873ec8069f2c080a34/js/public/ContextOptions.h#24

We would need to push it down to CompartmentOptions. And then we can enable it per-Compartment, and enable it for Chrome but not Content.

comment:12 Changed 13 months ago by tom

The wasm aspect of this work is happening in https://bugzilla.mozilla.org/show_bug.cgi?id=1576254

comment:13 Changed 12 months ago by tom

That bug no has an ESR68 backport of the patch

Version 0, edited 12 months ago by tom (next)

comment:14 Changed 11 months ago by gk

Keywords: TorBrowserTeam201910 GeorgKoppen201910 added
Status: needs_informationnew

Let's try testing that in 9.5a2.

comment:15 Changed 11 months ago by gk

Keywords: GeorgKoppen201911 added; GeorgKoppen201910 removed

Moving my tickets.

comment:16 Changed 11 months ago by pili

Keywords: TorBrowserTeam201911 added; TorBrowserTeam201910 removed

Moving tickets to November 2019

comment:17 Changed 11 months ago by pili

Points: 1

comment:18 Changed 11 months ago by acat

Actual Points: 0.5
Keywords: TorBrowserTeam201911R added; TorBrowserTeam201911 removed
Status: newneeds_review

Applied tom's backported patches: https://github.com/acatarineu/tor-browser/commits/23719 and did a pass of clang-format that fixed some minor style issues.

comment:19 Changed 11 months ago by gk

Do we have a good test making sure the WASM part is actually working as expected?

comment:20 in reply to:  19 ; Changed 11 months ago by gk

Keywords: TorBrowserTeam201911 added; TorBrowserTeam201911R removed
Status: needs_reviewneeds_revision

Replying to gk:

Do we have a good test making sure the WASM part is actually working as expected?

I found some "test": On mobile I get a warning now due to an unresponsive WASM script. That's pretty bad because users can start surfing while HTTPS-E is not ready yet and it gets our fundraising banner in a weird shape (not everything is positioned correctly). I am afraid we can't ship this as-is.

Interestingly, though, we *do have* WASM enabled on the standard level in our alpha and nightly builds and I don't have that issue on that level on that device. Thus, the problem here is not wasm-on-android per se.

comment:22 Changed 11 months ago by tom

I tested it by making a web extension and outputting ('WebAssembly' in this) in a web page and in an extension page.

comment:23 in reply to:  20 Changed 11 months ago by acat

Replying to gk:

Replying to gk:

Do we have a good test making sure the WASM part is actually working as expected?

I found some "test": On mobile I get a warning now due to an unresponsive WASM script. That's pretty bad because users can start surfing while HTTPS-E is not ready yet and it gets our fundraising banner in a weird shape (not everything is positioned correctly). I am afraid we can't ship this as-is.

Interestingly, though, we *do have* WASM enabled on the standard level in our alpha and nightly builds and I don't have that issue on that level on that device. Thus, the problem here is not wasm-on-android per se.

I could reproduce the https everywhere wasm script warning in Fennec with javascript.options.ion = false and javascript.options.baselinejit = false. Without having profiled the extension it looks like the JS code in the wasm path is slower than before when JIT is disabled.

I also tested with a wasm bench function that takes a while (at least 15 seconds) and just receives an integer as argument and the performance seems to be the same independently of ion or baselinejit, as long as wasm is enabled.

comment:24 Changed 11 months ago by pili

Cc: tbb-team added
Owner: changed from tbb-team to acat
Status: needs_revisionassigned

Assigning tickets to acat for the next few months

comment:25 Changed 10 months ago by acat

What should we do about this one? It looks like we would need to enable JIT in webextensions apart from wasm to avoid this issue in HTTPS Everywhere (or try to improve the perf. on the JS side of HTTPS Everywhere, which I don't know if it's possible). I guess doing the wasm patch took some effort for tom, I'm not sure how feasible would it be to do a patch for JIT enabled for webextensions...

comment:26 Changed 10 months ago by gk

Yeah. Let's talk next Monday about it, I am not sure yet.

comment:27 Changed 10 months ago by pili

Keywords: TorBrowserTeam201912 added; TorBrowserTeam201911 removed

Moving tickets to December

comment:28 Changed 10 months ago by pili

Keywords: TorBrowserTeam202001 added; TorBrowserTeam201912 removed

acat is afk in December

comment:29 Changed 10 months ago by starlit

Does this also apply to the asmjs option that has now been disabled/set to FALSE by default? Referencing this ticket: https://trac.torproject.org/projects/tor/ticket/19417

since asmjs enabled is essential to being able to use openpgp.js in an extension. config option is:

javascript.options.asmjs

Last edited 10 months ago by starlit (previous) (diff)

comment:30 Changed 9 months ago by acat

I wanted to test on the same device the patch that enables ion on WebExtensions from https://bugzilla.mozilla.org/show_bug.cgi?id=1599226. I quickly backported it (might have missed something) in branch https://github.com/acatarineu/tor-browser/commits/23719+1, after the wasm patches. The resulting apk is in https://people.torproject.org/~acat/builds/tor-browser-9.5a4-android-armv7-multi-qa_23719+1.apk.

Unfortunately, it seems the patches still do not solve the issue. After testing the tor browser build, I tried some prefs combinations in a regular fennec with https everywhere:

baselinejit = false, ion = false -> too slow, prompt shown
baselinejit = false, ion = true -> too slow, prompt shown
baselinejit = true, ion = false -> fast enough, prompt not shown
baselinejit = true, ion = false -> faster, prompt not shown

Then I checked the code in the patch and realized that the IsIonEnabled function in https://phabricator.services.mozilla.com/D56987 returns

IsBaselineJitEnabled() && prefEnabled;

so it seems for ion to be enabled the baselinejit must also be, which I think makes sense.

So, do we also need to be able to selectively enable the baselinejit on extensions, or would there be a simpler way?

comment:31 in reply to:  29 Changed 9 months ago by acat

Replying to starlit:

Does this also apply to the asmjs option that has now been disabled/set to FALSE by default? Referencing this ticket: https://trac.torproject.org/projects/tor/ticket/19417

since asmjs enabled is essential to being able to use openpgp.js in an extension. config option is:

javascript.options.asmjs

Actually, javascript.options.asmjs is now set to true by default, and disabled in higher security settings. So, with default settings it should be fast enough, and with higher security settings slower, but hopefully still usable.

comment:32 Changed 8 months ago by pili

Keywords: TorBrowserTeam202002 added; TorBrowserTeam202001 removed

Moving tickets to February

comment:33 Changed 7 months ago by sysrqb

Keywords: tbb-9.5a7-want added; GeorgKoppen201911 removed

1599226 landed. We can think about backporting this for 9.5a7. I doubt it'll be clean, but hopefully we won't hit too many conflicts.

comment:34 Changed 7 months ago by pili

Keywords: TorBrowserTeam202003 added; TorBrowserTeam202002 removed

We are no longer in February, moving tickets

comment:35 Changed 6 months ago by pili

Keywords: TorBrowserTeam202006 added; TorBrowserTeam202003 removed

comment:36 in reply to:  33 Changed 6 months ago by sysrqb

Resolution: wontfix
Status: assignedclosed

Replying to sysrqb:

1599226 landed. We can think about backporting this for 9.5a7. I doubt it'll be clean, but hopefully we won't hit too many conflicts.

I'm admitting we won't have time to backport this before we finish the migration to Rapid Release and we get it there.

comment:37 Changed 6 months ago by pili

#33840 is a duplicate

Note: See TracTickets for help on using tickets.