[tor-bugs] #15197 [Tor Browser]: Rebase Tor Browser patches to ESR 45

[Tor Bug Tracker & Wiki]

#15197: Rebase Tor Browser patches to ESR 45
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:
     Type:  task                                 |  arthuredelstein
 Priority:  Very High                            |         Status:
Component:  Tor Browser                          |  needs_revision
 Severity:  Critical                             |      Milestone:
 Keywords:  ff45-esr, TorBrowserTeam201604,      |        Version:
  tbb-6.0a5                                      |     Resolution:
Parent ID:                                       |  Actual Points:
 Reviewer:                                       |         Points:
                                                 |        Sponsor:
                                                 |  SponsorU
-------------------------------------------------+-------------------------

Comment (by arthuredelstein):

 Replying to [comment:18 gk]:
 > Here is the first part of the review up to and not including
 > {{{
 > W 415f86f Bug #6539: Isolate the Image Cache per url bar domain.
 > }}}
 > I started from 10d37d6 and looked at the corresponding commits on
 15197+7 that were not made by Kathy and Mark assuming they doublecheck
 their patches themselves and you checked the rebased updater patches.
 Commits I have nothing to say about are omitted.

 Thanks for the reviews!

 > e839bf9af83f995c36a4d1a7295869dee68c13b0 and
 > 79a6c2a9971a596511b5902b44e074b825c5f465 and
 > 4b81ffadd3ccdc95b208b9594231c5d8b09ff09d => into one .mozconfig related
 commit?

 Done

 > We should try upstreaming 527211deedc9fef0afdad2b3fdc64c82f6a05cd2
 again. We have a patch and I guess Mozilla would take it after we reopen
 the bug

 Good idea. I was only able to

 > 14d63523794c82c83bfa145262d9ec46b4a18bba
 > {{{
 >   _pluginEnableButton: null,
 >   _pluginHeader: null,
 > }}}
 > landed in gSearchView but should be in gListView I guess

 Done.

 > ae367aad222b70d37e5de936dd594e9cdc554ef1 -- could you put the block in
 AsnycResolveExtended() again after `if (mNotifyResolution) {}`? Might be
 helpful for debugging things (observer notification gets still fired even
 if DNS resolution does not get through)?

 Fixed.

 > 246c62e001eefe5864d705610bfafd74dd8e54ce -- okay (a bunch of trailing
 whitespaces (might be in the original patch) + an additional newline:
 > {{{
 > -    if (mNumHalfOpenConns < parallelSpeculativeConnectLimit &&
 > +
 > +    if (ent->SupportsPipelining()) {
 > }}}
 > )

 Fixed.

 > 094a68930f2d0f17ddc7a50339cc19a4bae44e6c -- okay but it seems to be not
 needed anymore, see: https://bugzilla.mozilla.org/show_bug.cgi?id=962334,
 no?

 You're right -- I will drop this.

 > 850e2636cd7526cf9631ac629b1f5c45148e98c1 -- okay (why is this still
 needed after
 > https://bugzilla.mozilla.org/show_bug.cgi?id=429070 got fixed years
 ago?)

 I agree -- I will drop this patch and the corresponding regression test.

 > 308d80c19a1ad61f03ca7fecaf1102ee32659b76 -- okay (just superfluous
 whitespace in the test)

 Fixed.

 > 8f1395bc9c13b0ccdf61343a91d413a2bad30f6c -- okay (just superfluous
 whitespace in the test)

 Fixed.

 > 49072ce9ec279ab6c5f7903dcfa4142abae7ba48 -- okay (just superfluous
 whitespace in the test)

 I merged this patch with our patch for #16315: this consolidates a single
 test file into one commit, and also deals with the superfluous whitespace.

 > caebb2528e34d7ab6edf0bfd32503bc303755a8f -- we really need #include
 "nsIURL.h" in this commit?

 Yes, it appears to be necessary given the line
 {{{
 +  nsCOMPtr<nsIURL> url = do_QueryInterface(aFirstPartyURI);
 }}}
 later in the commit. I tried compiling without it the #include, but that
 failed.

 > 09e5754cf7748c35305d5e2d72a50de7ae0a1b1f -- okay (missing newline after
 > {{{
 >     +    extension.Append(nsPrintfCString("%s@", cacheDomain.get()));
 > }}}
 > )

 I didn't add an extra newline because the following lines are closely
 related.

 > Could you address Mark's question in #18632?

 Yes, I will address those questions there.

 The branch after these changes is at
 https://github.com/arthuredelstein/tor-browser/commits/15197+8

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/15197#comment:35>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online