Opened 14 months ago

Closed 8 weeks ago

#21850 closed defect (fixed)

Update #16940 patch for e10s (about:tbupdate)

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff52-esr, tbb-e10s, tbb-7.0-must, TorBrowserTeam201803R
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

+  // When electrolysis is enabled we will need to adopt an architecture that is
+  // more similar to the one that is used for about:home (see AboutHomeListener
+  // in this file and browser/modules/AboutHome.jsm).

Child Tickets

Change History (22)

comment:1 Changed 14 months ago by gk

Keywords: tbb-7.0-must-alpha added; tbb-7.0-must removed

Getting this on our radar for alpha release in less than two weeks.

comment:2 Changed 13 months ago by gk

Keywords: TorBrowserTeam201705 added; TorBrowserTeam201704 removed

Moving our tickets to May 2017.

comment:3 Changed 12 months ago by gk

Keywords: tbb-7.0-must added; tbb-7.0-must-alpha removed

We are beyond the alpha testing. Moving tickets for tbb-7.0-must.

comment:4 Changed 12 months ago by cypherpunks

Keywords: tbb-e10s added

comment:5 Changed 12 months ago by gk

Keywords: TorBrowserTeam201706 added; TorBrowserTeam201705 removed

Moving our tickets to June.

comment:6 Changed 11 months ago by mcs

Summary: Update #16940 patch for e10sUpdate #16940 patch for e10s (about:tbupdate)

comment:7 Changed 11 months ago by gk

Keywords: TorBrowserTeam201707 added; TorBrowserTeam201706 removed

Moving Tickets to July 2017.

comment:8 Changed 10 months ago by gk

Keywords: TorBrowserTeam201708 added; TorBrowserTeam201707 removed

Moving our Tickets to August.

comment:10 Changed 9 months ago by gk

Keywords: TorBrowserTeam201709 added; TorBrowserTeam201708 removed

Items for September 2017.

comment:11 Changed 8 months ago by gk

Keywords: TorBrowserTeam201710 added; TorBrowserTeam201709 removed

Items for October 2017

comment:12 Changed 7 months ago by gk

Keywords: TorBrowserTeam201711 added; TorBrowserTeam201710 removed

Moving tickets over to November.

comment:13 Changed 6 months ago by gk

Moving tickets to December 2017

comment:14 Changed 6 months ago by gk

Keywords: TorBrowserTeam201712 added; TorBrowserTeam201711 removed

Moving tickets to December 2017, for realz.

comment:15 Changed 4 months ago by gk

Keywords: TorBrowserTeam201801 added; TorBrowserTeam201712 removed

Moving tickets to 2018.

comment:16 Changed 4 months ago by mcs

Keywords: TorBrowserTeam201801R added; TorBrowserTeam201801 removed
Status: newneeds_review

While the existing code functions correctly with or without electrolysis enabled, we improved it (see the check in comment for details). The fixup patch is here:

https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug21850-01&id=3d3272e7b56ed23c18f3ba1f3fbb1821118e164c

comment:17 Changed 4 months ago by mcs

We forgot about comment:9. Here is an additional fixup patch that forces about:tbupdate to always be loaded in a content process:
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug21850-01&id=7a7c6b380c95a00f3091aa8f79c24bd76a0a2cbc

It seems safer to do this and should not cause any harm (we already force the same behavior for about:tor).

comment:18 Changed 4 months ago by gk

Keywords: TorBrowserTeam201802R added; TorBrowserTeam201801R removed

Moving reviews to February.

comment:19 Changed 3 months ago by gk

Keywords: TorBrowserTeam201803R added; TorBrowserTeam201802R removed

Moving our reviews to March 2018

comment:20 Changed 8 weeks ago by gk

Status: needs_reviewneeds_information

Nice work! One thing I don't understand: why do you have

+if (AppConstants.TOR_BROWSER_UPDATE) {
+  XPCOMUtils.defineLazyModuleGetter(this, "AboutTBUpdate",
+    "resource:///modules/AboutTBUpdate.jsm");
+}

in browser.js? It seems you don't use anything from that module there.

comment:21 in reply to:  20 Changed 8 weeks ago by mcs

Status: needs_informationneeds_review

Replying to gk:

Nice work! One thing I don't understand: why do you have

+if (AppConstants.TOR_BROWSER_UPDATE) {
+  XPCOMUtils.defineLazyModuleGetter(this, "AboutTBUpdate",
+    "resource:///modules/AboutTBUpdate.jsm");
+}

in browser.js? It seems you don't use anything from that module there.

You are correct; that part of the patch is not needed. Here is a revised (and rebased) patch:
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug21850-02

comment:22 Changed 8 weeks ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks, looks good now. \o/ Applied to tor-browser-52.7.3esr-8.0-1 (commit 7719a132533d0e245602b9aee496875a8cb03cd5).

Note: See TracTickets for help on using tickets.