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

Make Tor profile an OTR profile associated with the original profile #7069

Merged
merged 12 commits into from
Nov 19, 2020
Merged

Make Tor profile an OTR profile associated with the original profile #7069

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
55afff5
Make Tor profile an OTR profile associated with the original profile
darkdh Nov 6, 2020
fde6209
Brave profile manager cleanup:
darkdh Nov 10, 2020
7b1d402
Migrate legacy tor session profile
darkdh Nov 11, 2020
b03d2c4
Fix tests
darkdh Nov 12, 2020
dfd40f7
Move KillTor() to TorProfileManager
darkdh Nov 12, 2020
834a46b
Make sure all tor profile have proper proxy config
darkdh Nov 12, 2020
df4842a
Remove brave/browser/profiles deps for brave/browser/tor
darkdh Nov 13, 2020
b82905c
Introduce MockTorLauncherFactory to test cases like CloseLastTorWindow
darkdh Nov 17, 2020
cbf1323
Remove IsTorProfile
darkdh Nov 18, 2020
78f81ae
Address review feedback from yrliou
darkdh Nov 18, 2020
8e4ef4a
Remove redundant profile check
darkdh Nov 18, 2020
c4ad0bd
Add kSessionProfileDir
darkdh Nov 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion browser/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,6 @@ source_set("browser_process") {

deps += [
"//base",
"//brave/browser/profiles",
"//brave/common",
"//brave/components/gemini/browser",
"//components/country_codes",
Expand Down
11 changes: 11 additions & 0 deletions browser/brave_browser_main_parts.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,17 @@ void BraveBrowserMainParts::PostBrowserStart() {
tor_legacy_path, base::DoNothing(),
ProfileMetrics::DELETE_PROFILE_SETTINGS);
}
for (Profile* profile : profile_manager->GetLoadedProfiles()) {
const base::FilePath tor_legacy_session_path =
profile->GetPath()
.AppendASCII("session_profiles")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about making session_profiles as a constant instead of hard-cording?
It looks like some places used hard coded string.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only used for migration

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also hard coded below.

bool IsSessionProfilePath(const base::FilePath& path) {
  return path.DirName().BaseName() ==
         base::FilePath(FILE_PATH_LITERAL("session_profiles"));
}

Copy link
Member Author

@darkdh darkdh Nov 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in c4ad0bd

.Append(tor::kTorProfileDir);
if (base::PathExists(tor_legacy_session_path)) {
profile_manager->MaybeScheduleProfileForDeletion(
tor_legacy_session_path, base::DoNothing(),
ProfileMetrics::DELETE_PROFILE_SETTINGS);
}
}
#endif

#if !defined(OS_ANDROID)
Expand Down
5 changes: 2 additions & 3 deletions browser/brave_content_browser_client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "brave/browser/brave_browser_process_impl.h"
#include "brave/browser/net/brave_proxying_url_loader_factory.h"
#include "brave/browser/net/brave_proxying_web_socket.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/common/pref_names.h"
#include "brave/common/webui_url_constants.h"
#include "brave/components/binance/browser/buildflags/buildflags.h"
Expand Down Expand Up @@ -476,7 +475,7 @@ BraveContentBrowserClient::CreateThrottlesForNavigation(
std::unique_ptr<content::NavigationThrottle> tor_navigation_throttle =
tor::TorNavigationThrottle::MaybeCreateThrottleFor(handle,
TorProfileServiceFactory::GetForContext(context),
brave::IsTorProfile(context));
context->IsTor());
if (tor_navigation_throttle)
throttles.push_back(std::move(tor_navigation_throttle));
std::unique_ptr<tor::OnionLocationNavigationThrottleDelegate>
Expand All @@ -487,7 +486,7 @@ BraveContentBrowserClient::CreateThrottlesForNavigation(
tor::OnionLocationNavigationThrottle::MaybeCreateThrottleFor(
handle, TorProfileServiceFactory::IsTorDisabled(),
std::move(onion_location_navigation_throttle_delegate),
brave::IsTorProfile(context));
context->IsTor());
if (onion_location_navigation_throttle)
throttles.push_back(std::move(onion_location_navigation_throttle));
#endif
Expand Down
4 changes: 2 additions & 2 deletions browser/brave_tab_helpers.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "brave/browser/brave_stats/brave_stats_tab_helper.h"
#include "brave/browser/ephemeral_storage/ephemeral_storage_tab_helper.h"
#include "brave/browser/farbling/farbling_tab_helper.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/browser/ui/bookmark/brave_bookmark_tab_helper.h"
#include "brave/components/brave_ads/browser/ads_tab_helper.h"
#include "brave/components/brave_perf_predictor/browser/buildflags.h"
Expand All @@ -22,6 +21,7 @@
#include "brave/components/ipfs/buildflags/buildflags.h"
#include "brave/components/speedreader/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/web_contents.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/widevine/cdm/buildflags.h"
Expand Down Expand Up @@ -122,7 +122,7 @@ void AttachTabHelpers(content::WebContents* web_contents) {

#if BUILDFLAG(ENABLE_TOR)
tor::TorTabHelper::MaybeCreateForWebContents(
web_contents, brave::IsTorProfile(web_contents->GetBrowserContext()));
web_contents, web_contents->GetBrowserContext()->IsTor());
tor::OnionLocationTabHelper::CreateForWebContents(web_contents);
#endif

Expand Down
5 changes: 1 addition & 4 deletions browser/extensions/api/binance_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <vector>

#include "base/environment.h"
#include "brave/browser/profiles/profile_util.h"

#include "brave/common/extensions/api/binance.h"
#include "brave/common/pref_names.h"
Expand All @@ -34,9 +33,7 @@ BinanceService* GetBinanceService(content::BrowserContext* context) {

bool IsBinanceAPIAvailable(content::BrowserContext* context) {
Profile* profile = Profile::FromBrowserContext(context);
return !brave::IsTorProfile(profile) &&
!profile->IsIncognitoProfile() &&
!profile->IsGuestSession();
return !profile->IsIncognitoProfile() && !profile->IsGuestSession();
}

} // namespace
Expand Down
3 changes: 1 addition & 2 deletions browser/extensions/api/brave_together_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <memory>

#include "base/environment.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/components/brave_together/browser/regions.h"
#include "brave/components/ntp_widget_utils/browser/ntp_widget_utils_region.h"
#include "chrome/browser/profiles/profile.h"
Expand All @@ -21,7 +20,7 @@ ExtensionFunction::ResponseAction
BraveTogetherIsSupportedFunction::Run() {
Profile* profile = Profile::FromBrowserContext(browser_context());

if (brave::IsTorProfile(profile)) {
if (profile->IsTor()) {
return RespondNow(Error("Not available in Tor profile"));
}

Expand Down
13 changes: 5 additions & 8 deletions browser/extensions/api/brave_wallet_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/values.h"
#include "brave/browser/extensions/brave_wallet_util.h"
#include "brave/browser/infobars/crypto_wallets_infobar_delegate.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/common/extensions/api/brave_wallet.h"
#include "brave/components/brave_wallet/brave_wallet_constants.h"
#include "brave/components/brave_wallet/pref_names.h"
Expand Down Expand Up @@ -57,9 +56,8 @@ BraveWalletPromptToEnableWalletFunction::Run() {
brave_wallet::PromptToEnableWallet::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params.get());

Profile* profile = Profile::FromBrowserContext(browser_context());
if (brave::IsTorProfile(profile)) {
return RespondNow(Error("Not available in Tor profile"));
if (browser_context()->IsTor()) {
return RespondNow(Error("Not available in Tor context"));
}

// Get web contents for this tab
Expand Down Expand Up @@ -94,9 +92,8 @@ BraveWalletPromptToEnableWalletFunction::Run() {

ExtensionFunction::ResponseAction
BraveWalletReadyFunction::Run() {
Profile* profile = Profile::FromBrowserContext(browser_context());
if (brave::IsTorProfile(profile)) {
return RespondNow(Error("Not available in Tor profile"));
if (browser_context()->IsTor()) {
return RespondNow(Error("Not available in Tor context"));
}

auto* service = GetBraveWalletService(browser_context());
Expand Down Expand Up @@ -143,7 +140,7 @@ BraveWalletShouldPromptForSetupFunction::Run() {
ExtensionFunction::ResponseAction
BraveWalletShouldCheckForDappsFunction::Run() {
Profile* profile = Profile::FromBrowserContext(browser_context());
if (brave::IsTorProfile(profile)) {
if (browser_context()->IsTor()) {
return RespondNow(OneArgument(
std::make_unique<base::Value>(false)));
}
Expand Down
3 changes: 1 addition & 2 deletions browser/extensions/brave_extension_service.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <string>

#include "base/one_shot_event.h"
#include "brave/browser/profiles/profile_util.h"
#include "chrome/browser/extensions/api/content_settings/content_settings_service.h"
#include "chrome/browser/profiles/profile.h"
#include "extensions/browser/extension_action.h"
Expand Down Expand Up @@ -43,7 +42,7 @@ void BraveExtensionService::AddComponentExtension(const Extension* extension) {
// the extension when BraveActionViewController is queried about the
// visibility of the action.
if ((extension->id() == brave_rewards_extension_id) &&
(profile_->IsGuestSession() || brave::IsTorProfile(profile_))) {
(profile_->IsGuestSession() || profile_->IsTor())) {
extensions::ExtensionActionManager* extension_action_manager =
ExtensionActionManager::Get(profile_);
extensions::ExtensionAction* action =
Expand Down
12 changes: 0 additions & 12 deletions browser/extensions/brave_extensions_browser_client_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@

#include "base/macros.h"
#include "brave/browser/extensions/brave_extensions_browser_api_provider.h"
#include "brave/browser/profiles/profile_util.h"
#include "chrome/browser/extensions/chrome_component_extension_resource_manager.h"
#include "components/prefs/pref_service.h"

namespace extensions {

Expand All @@ -25,14 +23,4 @@ BraveExtensionsBrowserClientImpl::BraveExtensionsBrowserClientImpl() {
->GetTemplateReplacementsForExtension(""));
}

bool BraveExtensionsBrowserClientImpl::AreExtensionsDisabled(
const base::CommandLine& command_line,
content::BrowserContext* context) {
if (brave::IsTorProfile(context))
return true;

return ChromeExtensionsBrowserClient::AreExtensionsDisabled(command_line,
context);
}

} // namespace extensions
3 changes: 0 additions & 3 deletions browser/extensions/brave_extensions_browser_client_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ class BraveExtensionsBrowserClientImpl : public ChromeExtensionsBrowserClient {
BraveExtensionsBrowserClientImpl();
~BraveExtensionsBrowserClientImpl() override = default;

bool AreExtensionsDisabled(const base::CommandLine& command_line,
content::BrowserContext* context) override;

DISALLOW_COPY_AND_ASSIGN(BraveExtensionsBrowserClientImpl);
};

Expand Down
2 changes: 0 additions & 2 deletions browser/extensions/brave_webtorrent_navigation_throttle_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

#include "base/command_line.h"
#include "base/files/scoped_temp_dir.h"
#include "brave/browser/profiles/brave_profile_manager.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/common/pref_names.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/load_error_reporter.h"
Expand Down
4 changes: 1 addition & 3 deletions browser/gemini/gemini_service_factory.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include "brave/browser/gemini/gemini_service_factory.h"

#include "brave/browser/profiles/profile_util.h"
#include "brave/components/gemini/browser/gemini_service.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
Expand All @@ -18,8 +17,7 @@ GeminiServiceFactory* GeminiServiceFactory::GetInstance() {

// static
GeminiService* GeminiServiceFactory::GetForProfile(Profile* profile) {
if (brave::IsTorProfile(profile) ||
profile->IsIncognitoProfile() ||
if (profile->IsIncognitoProfile() ||
profile->IsGuestSession()) {
return nullptr;
}
Expand Down
3 changes: 1 addition & 2 deletions browser/geolocation/brave_geolocation_permission_context_delegate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <utility>

#include "brave/browser/profiles/profile_util.h"
#include "chrome/browser/profiles/profile.h"
#include "components/content_settings/core/common/content_settings.h"
#include "components/permissions/permission_request_id.h"
Expand All @@ -30,7 +29,7 @@ bool BraveGeolocationPermissionContextDelegate::DecidePermission(
bool user_gesture,
permissions::BrowserPermissionCallback* callback,
permissions::GeolocationPermissionContext* context) {
if (brave::IsTorProfile(profile_)) {
if (profile_->IsTor()) {
std::move(*callback).Run(ContentSetting::CONTENT_SETTING_BLOCK);
return true;
}
Expand Down
31 changes: 12 additions & 19 deletions browser/ipfs/content_browser_client_helper_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,16 @@

#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h"
#include "brave/browser/profiles/brave_profile_manager.h"
#include "brave/browser/profiles/brave_unittest_profile_manager.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/components/ipfs/features.h"
#include "brave/components/ipfs/ipfs_constants.h"
#include "brave/components/ipfs/ipfs_gateway.h"
#include "brave/components/ipfs/pref_names.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "chrome/browser/prefs/browser_prefs.h"
#include "chrome/common/channel_info.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/scoped_testing_local_state.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/mock_navigation_handle.h"
Expand All @@ -34,6 +30,8 @@

namespace {

constexpr char kTestProfileName[] = "TestProfile";

const GURL& GetIPFSURI() {
static const GURL ipfs_url(
"ipfs://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq/wiki/"
Expand Down Expand Up @@ -77,34 +75,30 @@ namespace ipfs {

class ContentBrowserClientHelperUnitTest : public testing::Test {
public:
ContentBrowserClientHelperUnitTest()
: local_state_(TestingBrowserProcess::GetGlobal()) {}
ContentBrowserClientHelperUnitTest() = default;
~ContentBrowserClientHelperUnitTest() override = default;

void SetUp() override {
feature_list_.InitAndEnableFeature(ipfs::features::kIpfsFeature);
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());

TestingBrowserProcess::GetGlobal()->SetProfileManager(
new BraveUnittestProfileManager(temp_dir_.GetPath()));
ProfileManager* profile_manager = g_browser_process->profile_manager();
ASSERT_TRUE(profile_manager);
TestingBrowserProcess* browser_process = TestingBrowserProcess::GetGlobal();
profile_manager_.reset(new TestingProfileManager(browser_process));
ASSERT_TRUE(profile_manager_->SetUp());

profile_ = profile_manager_->CreateTestingProfile(kTestProfileName);

profile_ = profile_manager->GetProfile(
temp_dir_.GetPath().AppendASCII(TestingProfile::kTestUserProfileDir));
web_contents_ =
content::WebContentsTester::CreateTestWebContents(profile_, nullptr);
}

void TearDown() override {
web_contents_.reset();
TestingBrowserProcess::GetGlobal()->SetProfileManager(nullptr);
profile_ = nullptr;
profile_manager_->DeleteTestingProfile(kTestProfileName);
}

content::WebContents* web_contents() { return web_contents_.get(); }

base::ScopedTempDir* temp_dir() { return &temp_dir_; }

// Helper that creates simple test guest profile.
std::unique_ptr<TestingProfile> CreateGuestProfile() {
TestingProfile::Builder profile_builder;
Expand All @@ -120,11 +114,10 @@ class ContentBrowserClientHelperUnitTest : public testing::Test {

private:
content::BrowserTaskEnvironment task_environment_;
base::ScopedTempDir temp_dir_;
ScopedTestingLocalState local_state_;
content::RenderViewHostTestEnabler test_render_host_factories_;
std::unique_ptr<content::WebContents> web_contents_;
Profile* profile_;
std::unique_ptr<TestingProfileManager> profile_manager_;
base::test::ScopedFeatureList feature_list_;

DISALLOW_COPY_AND_ASSIGN(ContentBrowserClientHelperUnitTest);
Expand Down
5 changes: 2 additions & 3 deletions browser/p3a/p3a_core_metrics.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include "base/metrics/histogram_macros.h"
#include "base/time/time.h"
#include "brave/browser/profiles/profile_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_list.h"
Expand Down Expand Up @@ -44,7 +43,7 @@ enum class WindowUsageStats {

const char* GetPrefNameForProfile(Profile* profile) {
if (profile->IsIncognitoProfile() &&
!brave::IsTorProfile(profile)) {
!profile->IsTor()) {
return kLastTimeIncognitoUsed;
}
return nullptr;
Expand Down Expand Up @@ -130,7 +129,7 @@ void BraveWindowTracker::RegisterPrefs(PrefRegistrySimple* registry) {
}

void BraveWindowTracker::OnBrowserAdded(Browser* browser) {
if (brave::IsTorProfile(browser->profile())) {
if (browser->profile()->IsTor()) {
local_state_->SetBoolean(kTorUsed, true);
return;
}
Expand Down
12 changes: 7 additions & 5 deletions browser/profiles/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import("//brave/components/tor/buildflags/buildflags.gni")
import("//components/gcm_driver/config.gni")

source_set("profiles") {
Expand Down Expand Up @@ -26,8 +27,6 @@ source_set("profiles") {
deps = [
"//base",
"//brave/browser/gcm_driver",
"//brave/browser/tor",
"//brave/browser/translate/buildflags",
"//brave/common:pref_names",
"//brave/components/brave_ads/browser",
"//brave/components/brave_rewards/browser",
Expand All @@ -38,9 +37,12 @@ source_set("profiles") {
"//chrome/common",
"//components/gcm_driver:gcm_buildflags",
"//components/prefs",
"//components/safe_browsing/core/common:safe_browsing_prefs",
"//components/translate/core/browser",
"//third_party/blink/public/common",
"//ui/base",
]

if (enable_tor) {
deps += [
"//brave/browser/tor",
]
}
}
Loading