e10s and 52esr compatibility for tor circuit display

mcs and brade noticed a crash in the circuit display. Also first party key is now part of origin attributes, so that probably needs to operate differently as well.

added TorBrowserTeam201703R component::applications/tor browser ff52-esr owner::tbb-team parent::21201 priority::medium resolution::fixed severity::normal status::closed tbb-7.0-must tbb-torbutton type::defect labels

Trac:
Keywords: N/A deleted, tbb-torbutton added
Component: Applications/Torbutton to Applications/Tor Browser
Status: new to assigned
Owner: N/A to tbb-team

Trac:
Keywords: N/A deleted, TorBrowserTeam201703, tbb-7.0-must, ff52-esr added

Here's a patch for review: https://github.com/arthuredelstein/torbutton/commit/21547 It's on top of some other #21201 (moved) patches, but only the top patch is relevant to this ticket.

Trac:
Status: assigned to needs_review
Keywords: TorBrowserTeam201703 deleted, TorBrowserTeam201703R added

Kathy and I reviewed the code and ran it with a browser built from your tor-browser.git 20680+4 branch. Circuit display seems to work, but we see quite a few messages like this: Torbutton INFO: Failed to get credentials or browser from channel

We modified the code to log the exception and chan.URI.spec. Here is an example: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIInterfaceRequestor.getInterface]" nsresult: "0x80004002 (NS_NOINTERFACE)" location: "JS frame :: chrome://torbutton/content/tor-circuit-display.js :: browserForChannel :: line 163" data: no] uri: https://check.torproject.org/torcheck/img/tor-on.png

The above error occurred while loading https://check.torproject.org/?lang=en_US; we also see similar messages for many "internal" fetches such as OSCP which is not as surprising.

We should figure out a way to suppress the logging for errors that are expected. When we do log, we may want to include the exception as well as chan.URI.spec to make debugging easier.

Replying to mcs:

[snip] We should figure out a way to suppress the logging for errors that are expected. When we do log, we may want to include the exception as well as chan.URI.spec to make debugging easier.

Thanks for reviewing this and you make very good points. Here's my new version, applying your suggestions:

https://github.com/arthuredelstein/torbutton/commits/21547+2

r=brade, r=mcs Thanks for making the improvements. This looks good to us now.

Replying to mcs:

r=brade, r=mcs Thanks for making the improvements. This looks good to us now.

Thanks for the review. Added to my #21201 (moved) branch.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Looking over that patch it seems to me we only care about HTTP traffic nowadays at least if the circuit display concerned. Am I correct? While I am not a big fan of FTP, showing the corresponding circuit still works in current Tor Browser versions.

closed

mentioned in issue #21745 (moved)

moved to tpo/applications/tor-browser#21547 (closed)