Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: Implement Tor profile based on incognito profile #2724

Closed
wants to merge 17 commits into from
Closed

WIP: Implement Tor profile based on incognito profile #2724

wants to merge 17 commits into from

Conversation

yrliou
Copy link
Member

@yrliou yrliou commented Jun 18, 2019

Fix brave/brave-browser#2307

Submitter Checklist:

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

yrliou added 13 commits June 17, 2019 15:20
@yrliou
Implement BraveProfileImpl and TorProfileImpl to support creating a n…
bf3c71c 
…ew OTR

with Tor profile from a regular profile.
@yrliou
Override ProfileManager::IsValidProfile to check tor profile as well.
1ea79a0
@yrliou
Support destroying Tor profile in ProfileDestroyer
038d50c
@yrliou
Destroy Tor profile if needed in ~Browser()
0bff896
@yrliou
Add HasTorContext and GetTorContext in ExtensionBrowserClient
5bb7eb0
@yrliou
Change action of IDC_NEW_OFFTHERECORD_WINDOW_TOR to open the new tor …
4f77a22 
…profile
@yrliou
Create BravePluginServiceFilter to purge plugin list cache for associ…
35069c2 
…ated Tor

profile upon chrome::NOTIFICATION_PLUGIN_ENABLE_STATUS_CHANGED notification.
@yrliou
Post the error notification to associated Tor profile when a global e…
b2cf66e 
…rror has

changed to update error UI in Tor windows as well.
@yrliou
Creates ServiceWorkerTasks for associated Tor profile.
e4dfdc8
@yrliou
Handle downloads for Tor profile:
cdbcea4 
  - Open the download page for tor profiles with downloads in progress.
  - Include non-malicious download count from tor profile when getting
    the count for all profiles.
  - Check Tor profile as well for most recent download item.
  - Check Tor profile as well for in progress downloads in app_controller_mac.
@yrliou
Create BraveProcessManagerDelegate to close the background hosts in tor
74f0315 
profile if its original profile is destroyed.
@yrliou
Create BraveLazyBackgroundTaskQueue to handle Tor profile
651c01f 
  - Create lazy background host when extension loaded
  - Notify queued tasks when a lazy background page has failed to load
@yrliou
Create BraveLazyEventDispatcher to dispatch lazy events to tor contex…
6a3a0c0 
…t as well
@yrliou yrliou added this to the 0.68.x - Nightly milestone Jun 18, 2019
@yrliou yrliou self-assigned this Jun 18, 2019
@yrliou
Copy link
Member Author

yrliou commented Jun 18, 2019 •
edited
Loading

TODO:

  • Try to eliminate more patches if possible
  • make Open Link in Private Window with Tor in context menu to use the new tor profile
    (Addressed in 75216e1)
  • remove unused codes from current tor impl
    (Addressed in 644e1d7)
  • tests

@yrliou yrliou force-pushed the tor_profile_rework_rebase branch from da45906 to b2361ac Compare June 18, 2019 19:36
yrliou added 2 commits June 18, 2019 15:49
@yrliou
Implement open link in Tor profile in context menu
75216e1
@yrliou
Fix launching Tor process behavior when close and open Tor windows ag…
4c5ab07 
…ain.

Guest window is only destroyed when browser shuts down, so we only expect to
kill tor process while browser exits before when Tor profile is based on guest
window. By basing on incognito profile, now our Tor profile will be destroyed
when all Tor windows are closed, so Tor process will be killed when there is
no Tor windows. We need to reset TorLauncherFactory::tor_pid_ when it is killed
so we will trigger launching Tor process when creating Tor profile again. And
reinit TorLauncherFactory tor_launcher_ when needed.
@yrliou yrliou force-pushed the tor_profile_rework_rebase branch 2 times, most recently from d40b183 to da2892b Compare June 18, 2019 22:52
@yrliou
Update avatar menu views
4f90401 
  - Remove Tor button in avatar menu
  - Update text for title card and exit button to use Tor
@yrliou yrliou force-pushed the tor_profile_rework_rebase branch from da2892b to 22588cd Compare June 18, 2019 22:53
@yrliou yrliou force-pushed the tor_profile_rework_rebase branch from 22588cd to 644e1d7 Compare June 18, 2019 22:54
@yrliou
Copy link
Member Author

yrliou commented Jun 18, 2019

@riastradh-brave @bridiver @iefremov @darkdh This is still a WIP that have some todo items listed in above comment, but I would like to ask for feedbacks first.
I think it would be easier to go through each commit instead of looking at all files at the same time, thanks in advance.
cc @bbondy @tomlowenthal

@simonhong
Copy link
Member

simonhong commented Jun 24, 2019 •
edited
Loading

Question - With this PR, we will have two OTR profiles (for private and tor).
Should we need another OTR for Tor instead of sharing one OTR with private and Tor?
Or, should Tor window implementation be coupled with different OTR profile?

@yrliou
Copy link
Member Author

yrliou commented Jun 26, 2019 •
edited
Loading

@simonhong We want to have separate profiles between Private Window with Tor and Private Window, @riastradh-brave please correct me if I'm wrong.

@yrliou yrliou modified the milestones: 0.68.x - Dev, 0.69.x - Nightly Jun 26, 2019
@simonhong
Copy link
Member

@yrliou Yup, I just worried about using two different OTR with one normal profile because upstream uses 1:1 matching between them.
If there is no regression with this, I'm also fine. :)

@durch durch mentioned this pull request Jul 22, 2019
Closed
@yrliou yrliou removed this from the 0.69.x - Beta milestone Aug 20, 2019
@rulatir
Copy link

rulatir commented Aug 23, 2019

Is this PR abandoned?

@szaimen
Copy link

szaimen commented Oct 3, 2019

Is this PR abandoned?

Seems so, because brave/brave-browser#2307 is already closed.

@yrliou
Copy link
Member Author

yrliou commented Oct 3, 2019

This has been superseded by #3319.

@yrliou yrliou closed this Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darkdh darkdh Awaiting requested review from darkdh

@iefremov iefremov Awaiting requested review from iefremov

@bridiver bridiver Awaiting requested review from bridiver bridiver is a code owner

@riastradh-brave riastradh-brave Awaiting requested review from riastradh-brave

Assignees

@yrliou yrliou

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor Tor implementation to retain consistent profile
4 participants
@yrliou @simonhong @rulatir @szaimen