Skip to content

Commit

Permalink
Don't need to restart after changing tor option
Browse files Browse the repository at this point in the history
Browsr commands will be updated after changing tor option.
If user turns it off, tor component will be removed at the next
launching. If there is already opened tor window, it can be used.
But, user can't create new tor window anymore.
  • Loading branch information
simonhong committed Dec 4, 2019
1 parent 744f854 commit 3e01e1f
Show file tree
Hide file tree
Showing 16 changed files with 106 additions and 103 deletions.
24 changes: 24 additions & 0 deletions browser/brave_browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "brave/browser/component_updater/brave_component_updater_delegate.h"
#include "brave/browser/profiles/brave_profile_manager.h"
#include "brave/browser/tor/buildflags.h"
#include "brave/browser/ui/brave_browser_command_controller.h"
#include "brave/components/brave_component_updater/browser/local_data_files_service.h"
#include "brave/components/brave_shields/browser/ad_block_custom_filters_service.h"
#include "brave/components/brave_shields/browser/ad_block_regional_service_manager.h"
Expand All @@ -27,6 +28,7 @@
#include "brave/components/p3a/buildflags.h"
#include "brave/components/p3a/brave_histogram_rewrite.h"
#include "brave/components/p3a/brave_p3a_service.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/common/buildflags.h"
#include "chrome/common/chrome_paths.h"
#include "components/component_updater/component_updater_service.h"
Expand Down Expand Up @@ -57,11 +59,14 @@

#if BUILDFLAG(ENABLE_TOR)
#include "brave/browser/extensions/brave_tor_client_updater.h"
#include "brave/common/tor/pref_names.h"
#endif

#if defined(OS_ANDROID)
#include "chrome/browser/android/chrome_feature_list.h"
#include "chrome/browser/android/component_updater/background_task_update_scheduler.h"
#else
#include "chrome/browser/ui/browser.h"
#endif

BraveBrowserProcessImpl* g_brave_browser_process = nullptr;
Expand Down Expand Up @@ -102,6 +107,17 @@ BraveBrowserProcessImpl::BraveBrowserProcessImpl(StartupData* startup_data)
#endif // BUILDFLAG(BRAVE_P3A_ENABLED)
}

void BraveBrowserProcessImpl::Init() {
BrowserProcessImpl::Init();

#if BUILDFLAG(ENABLE_TOR)
pref_change_registrar_.Add(
tor::prefs::kTorDisabled,
base::Bind(&BraveBrowserProcessImpl::OnTorEnabledChanged,
base::Unretained(this)));
#endif
}

brave_component_updater::BraveComponent::Delegate*
BraveBrowserProcessImpl::brave_component_updater_delegate() {
if (!brave_component_updater_delegate_)
Expand Down Expand Up @@ -246,6 +262,14 @@ BraveBrowserProcessImpl::tor_client_updater() {
brave_component_updater_delegate());
return tor_client_updater_.get();
}

void BraveBrowserProcessImpl::OnTorEnabledChanged() {
// Update all browsers' tor command status.
for (Browser* browser : *BrowserList::GetInstance()) {
static_cast<chrome::BraveBrowserCommandController*>(
browser->command_controller())->UpdateCommandForTor();
}
}
#endif

brave::BraveP3AService* BraveBrowserProcessImpl::brave_p3a_service() {
Expand Down
7 changes: 7 additions & 0 deletions browser/brave_browser_process_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,16 @@ class BraveBrowserProcessImpl : public BrowserProcessImpl {
brave::BraveStatsUpdater* brave_stats_updater();

private:
// BrowserProcessImpl overrides:
void Init() override;

void CreateProfileManager();
void CreateNotificationPlatformBridge();

#if BUILDFLAG(ENABLE_TOR)
void OnTorEnabledChanged();
#endif

BraveComponent::Delegate* brave_component_updater_delegate();

// local_data_files_service_ should always be first because it needs
Expand Down
27 changes: 0 additions & 27 deletions browser/brave_local_state_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#if BUILDFLAG(ENABLE_TOR)
#include "brave/browser/tor/tor_profile_service.h"
#include "brave/common/tor/pref_names.h"
#endif

using BraveLocalStateBrowserTest = InProcessBrowserTest;
Expand All @@ -20,29 +19,3 @@ IN_PROC_BROWSER_TEST_F(BraveLocalStateBrowserTest, BasicTest) {
EXPECT_FALSE(tor::TorProfileService::IsTorDisabled());
#endif
}

#if BUILDFLAG(ENABLE_TOR)
IN_PROC_BROWSER_TEST_F(BraveLocalStateBrowserTest, PRE_TorDisabledTest) {
EXPECT_EQ(tor::TorProfileService::IsTorDisabled(),
tor::TorProfileService::GetTorDisabledPref());

// Set to same value(enable) to next launching prefs, doesn't affect anything.
tor::TorProfileService::SetTorDisabledPref(false);
EXPECT_EQ(tor::TorProfileService::IsTorDisabled(),
tor::TorProfileService::GetTorDisabledPref());

// Check setting to disable to next launching prefs doesn't affect
// IsTorDisabled().
tor::TorProfileService::SetTorDisabledPref(true);
EXPECT_FALSE(tor::TorProfileService::IsTorDisabled());
EXPECT_NE(tor::TorProfileService::IsTorDisabled(),
tor::TorProfileService::GetTorDisabledPref());
}

IN_PROC_BROWSER_TEST_F(BraveLocalStateBrowserTest, TorDisabledTest) {
// Check IsTorDisabled() is changed from previous test run.
EXPECT_TRUE(tor::TorProfileService::IsTorDisabled());
EXPECT_EQ(tor::TorProfileService::IsTorDisabled(),
tor::TorProfileService::GetTorDisabledPref());
}
#endif
2 changes: 1 addition & 1 deletion browser/extensions/brave_tor_client_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ BraveTorClientUpdater::~BraveTorClientUpdater() {
void BraveTorClientUpdater::Register() {
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
if (tor::TorProfileService::GetTorDisabledPref() ||
if (tor::TorProfileService::IsTorDisabled() ||
command_line.HasSwitch(switches::kDisableTorClientUpdaterExtension) ||
registered_) {
return;
Expand Down
2 changes: 0 additions & 2 deletions browser/extensions/brave_tor_client_updater.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ class BraveTorClientUpdater : public BraveComponent {
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);

bool registered() const { return registered_; }

protected:
void OnComponentReady(const std::string& component_id,
const base::FilePath& install_dir,
Expand Down
13 changes: 4 additions & 9 deletions browser/policy/brave_policy_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#if BUILDFLAG(ENABLE_TOR)
#include "brave/browser/tor/tor_profile_service.h"
#include "brave/common/tor/pref_names.h"
#endif

using testing::_;
Expand All @@ -40,7 +39,6 @@ class BravePolicyTest : public InProcessBrowserTest {
};

#if BUILDFLAG(ENABLE_TOR)
#if defined(OS_WIN)
// This policy only exists on Windows.
// Sets the tor policy before the browser is started.
class TorDisabledPolicyBrowserTest : public BravePolicyTest {
Expand All @@ -61,9 +59,8 @@ class TorDisabledPolicyBrowserTest : public BravePolicyTest {

IN_PROC_BROWSER_TEST_F(TorDisabledPolicyBrowserTest, TorDisabledPrefValueTest) {
// When policy is set, explicit setting doesn't change its pref value.
g_browser_process->local_state()->SetBoolean(tor::prefs::kTorDisabled, false);
EXPECT_TRUE(
g_browser_process->local_state()->GetBoolean(tor::prefs::kTorDisabled));
tor::TorProfileService::SetTorDisabled(false);
EXPECT_TRUE(tor::TorProfileService::IsTorDisabled());
}

class TorEnabledPolicyBrowserTest : public BravePolicyTest {
Expand All @@ -84,11 +81,9 @@ class TorEnabledPolicyBrowserTest : public BravePolicyTest {

IN_PROC_BROWSER_TEST_F(TorEnabledPolicyBrowserTest, TorDisabledPrefValueTest) {
// When policy is set, explicit setting doesn't change its pref value.
g_browser_process->local_state()->SetBoolean(tor::prefs::kTorDisabled, true);
EXPECT_FALSE(
g_browser_process->local_state()->GetBoolean(tor::prefs::kTorDisabled));
tor::TorProfileService::SetTorDisabled(true);
EXPECT_FALSE(tor::TorProfileService::IsTorDisabled());
}
#endif // OS_WIN
#endif // ENABLE_TOR

} // namespace policy
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ cr.define('settings', function() {
setHangoutsEnabled(value) {}
setIPFSCompanionEnabled(value) {}
setTorEnabled(value) {}
getTorEnabled() {}
getTorManaged() {}
isTorEnabled() {}
isTorManaged() {}
getRestartNeeded() {}
}

Expand All @@ -41,10 +41,10 @@ cr.define('settings', function() {
setTorEnabled(value) {
chrome.send('setTorEnabled', [value]);
}
getTorEnabled() {
return cr.sendWithPromise('getTorEnabled');
isTorEnabled() {
return cr.sendWithPromise('isTorEnabled');
}
getTorManaged() {
isTorManaged() {
return cr.sendWithPromise('isTorManaged');
}
getRestartNeeded() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,17 @@ Polymer({
this.addWebUIListener('brave-needs-restart-changed', (needsRestart) => {
this.showRestartToast_ = needsRestart
})
this.addWebUIListener('tor-enabled-pref-changed', (enabled) => {
this.addWebUIListener('tor-enabled-changed', (enabled) => {
this.torEnabled_ = enabled
})

this.browserProxy_.getRestartNeeded().then(show => {
this.showRestartToast_ = show;
});
this.browserProxy_.getTorEnabled().then(enabled => {
this.browserProxy_.isTorEnabled().then(enabled => {
this.torEnabled_ = enabled
})
this.browserProxy_.getTorManaged().then(managed => {
this.browserProxy_.isTorManaged().then(managed => {
this.disableTorOption_ = managed
})
},
Expand Down
18 changes: 1 addition & 17 deletions browser/tor/tor_profile_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,32 +56,16 @@ void TorProfileService::RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
const std::string tor_proxy_uri =
std::string(kTorProxyScheme) + std::string(kTorProxyAddress) + ":" + port;
registry->RegisterStringPref(prefs::kTorProxyString, tor_proxy_uri);
// This pref value and current tor enabled state might be different because
// user can change pref value. But, this pref changes doesn't affect current
// tor enabled state. Instead, this pref value uses whether tor component is
// registered or not at startup. Tor component is only registered if this has
// false.
// kTorDisabled could be managed. User can only change it via settings if it's
// not managed. For now, only Windows support tor group policy.
registry->RegisterBooleanPref(prefs::kTorDisabled, false);
}

// static
bool TorProfileService::IsTorDisabled() {
// In test, |g_brave_browser_process| could be null.
if (!g_brave_browser_process) {
return false;
}
return !g_brave_browser_process->tor_client_updater()->registered();
}

// static
bool TorProfileService::GetTorDisabledPref() {
return g_browser_process->local_state()->GetBoolean(prefs::kTorDisabled);
}

// static
void TorProfileService::SetTorDisabledPref(bool disabled) {
void TorProfileService::SetTorDisabled(bool disabled) {
g_browser_process->local_state()->SetBoolean(prefs::kTorDisabled, disabled);
}

Expand Down
3 changes: 1 addition & 2 deletions browser/tor/tor_profile_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ class TorProfileService : public KeyedService {
~TorProfileService() override;

static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
static void SetTorDisabledPref(bool disabled);
static bool GetTorDisabledPref();
static void SetTorDisabled(bool disabled);
static bool IsTorDisabled();

virtual void SetNewTorCircuit(content::WebContents* web_contents) = 0;
Expand Down
4 changes: 2 additions & 2 deletions browser/ui/brave_browser_command_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,13 @@ void BraveBrowserCommandController::UpdateCommandForWebcompatReporter() {
UpdateCommandEnabled(IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER, true);
}

void BraveBrowserCommandController::UpdateCommandForTor() {
#if BUILDFLAG(ENABLE_TOR)
void BraveBrowserCommandController::UpdateCommandForTor() {
const bool is_tor_enabled = !tor::TorProfileService::IsTorDisabled();
UpdateCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE, is_tor_enabled);
UpdateCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR, is_tor_enabled);
#endif
}
#endif

void BraveBrowserCommandController::UpdateCommandForBraveSync() {
UpdateCommandEnabled(IDC_SHOW_BRAVE_SYNC, true);
Expand Down
6 changes: 5 additions & 1 deletion browser/ui/brave_browser_command_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#ifndef BRAVE_BROWSER_UI_BRAVE_BROWSER_COMMAND_CONTROLLER_H_
#define BRAVE_BROWSER_UI_BRAVE_BROWSER_COMMAND_CONTROLLER_H_

#include "brave/browser/tor/buildflags.h"
#include "chrome/browser/ui/browser_command_controller.h"

// This namespace is needed for a chromium_src override
Expand All @@ -15,6 +16,10 @@ class BraveBrowserCommandController : public chrome::BrowserCommandController {
public:
explicit BraveBrowserCommandController(Browser* browser);

#if BUILDFLAG(ENABLE_TOR)
void UpdateCommandForTor();
#endif

private:
// Overriden from CommandUpdater:
bool SupportsCommand(int id) const override;
Expand All @@ -32,7 +37,6 @@ class BraveBrowserCommandController : public chrome::BrowserCommandController {
void UpdateCommandForBraveRewards();
void UpdateCommandForBraveAdblock();
void UpdateCommandForWebcompatReporter();
void UpdateCommandForTor();
void UpdateCommandForBraveSync();
void UpdateCommandForBraveWallet();

Expand Down
9 changes: 3 additions & 6 deletions browser/ui/toolbar/brave_app_menu_model_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,10 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, BasicTest) {
}

#if BUILDFLAG(ENABLE_TOR)
// Tor is enabled by default. Change pref to disable it at next test run.
IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, PRE_TorAppMenuTest) {
tor::TorProfileService::SetTorDisabledPref(true);
}

// If tor is disabled, corresponding menu and commands should be also disabled.
IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, TorAppMenuTest) {
// Tor is enabled by default. Change pref to disable.
tor::TorProfileService::SetTorDisabled(true);

auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
BraveAppMenuModel normal_model(browser_view->toolbar(), browser());
normal_model.Init();
Expand Down
Loading

0 comments on commit 3e01e1f

Please sign in to comment.