Skip to content

Commit

Permalink
Don't use pref for IsTorDisabled()
Browse files Browse the repository at this point in the history
To check whether tor is enabled or not, tor component registration is
used instead of pref value because user can change pref via settings.
With this, we don't need to another prefs to check user changed or not.
  • Loading branch information
simonhong committed Dec 4, 2019
1 parent 7fafcb9 commit 744f854
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 123 deletions.
5 changes: 0 additions & 5 deletions browser/brave_browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@

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

#if defined(OS_ANDROID)
Expand Down Expand Up @@ -101,10 +100,6 @@ BraveBrowserProcessImpl::BraveBrowserProcessImpl(StartupData* startup_data)
brave_p3a_service();
brave::SetupHistogramsBraveization();
#endif // BUILDFLAG(BRAVE_P3A_ENABLED)

#if BUILDFLAG(ENABLE_TOR)
tor::TorProfileService::InitializeTorPrefs();
#endif
}

brave_component_updater::BraveComponent::Delegate*
Expand Down
22 changes: 11 additions & 11 deletions browser/brave_local_state_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,31 @@ IN_PROC_BROWSER_TEST_F(BraveLocalStateBrowserTest, BasicTest) {
#if BUILDFLAG(ENABLE_TOR)
// Tor is enabled by default.
EXPECT_FALSE(tor::TorProfileService::IsTorDisabled());
EXPECT_FALSE(tor::TorProfileService::IsTorDisabledAtNextLaunching());
EXPECT_FALSE(tor::TorProfileService::IsTorDisabledChanged());
#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::SetTorDisabledAtNextLaunching(false);
EXPECT_FALSE(tor::TorProfileService::IsTorDisabled());
EXPECT_FALSE(tor::TorProfileService::IsTorDisabledAtNextLaunching());
EXPECT_FALSE(tor::TorProfileService::IsTorDisabledChanged());
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::SetTorDisabledAtNextLaunching(true);
tor::TorProfileService::SetTorDisabledPref(true);
EXPECT_FALSE(tor::TorProfileService::IsTorDisabled());
EXPECT_TRUE(tor::TorProfileService::IsTorDisabledAtNextLaunching());
EXPECT_TRUE(tor::TorProfileService::IsTorDisabledChanged());
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_FALSE(tor::TorProfileService::IsTorDisabledAtNextLaunching());
EXPECT_FALSE(tor::TorProfileService::IsTorDisabledChanged());
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::IsTorDisabled() ||
if (tor::TorProfileService::GetTorDisabledPref() ||
command_line.HasSwitch(switches::kDisableTorClientUpdaterExtension) ||
registered_) {
return;
Expand Down
2 changes: 2 additions & 0 deletions browser/extensions/brave_tor_client_updater.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ 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
16 changes: 4 additions & 12 deletions browser/policy/brave_policy_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,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(tor::TorProfileService::IsTorDisabled());
EXPECT_TRUE(
g_browser_process->local_state()->GetBoolean(tor::prefs::kTorDisabled));
}

class TorEnabledPolicyBrowserTest : public BravePolicyTest {
Expand All @@ -84,17 +85,8 @@ 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(tor::TorProfileService::IsTorDisabled());
}

// W/o TorDisabled group policy, kTorDisabled pref value should be false.
IN_PROC_BROWSER_TEST_F(NoTorPolicyBrowserTest,
DefaultTorDisabledPrefValueTest) {
EXPECT_FALSE(tor::TorProfileService::IsTorDisabled());

// If policy is not set, explicit setting change its pref value.
g_browser_process->local_state()->SetBoolean(tor::prefs::kTorDisabled, true);
EXPECT_TRUE(tor::TorProfileService::IsTorDisabled());
EXPECT_FALSE(
g_browser_process->local_state()->GetBoolean(tor::prefs::kTorDisabled));
}
#endif // OS_WIN
#endif // ENABLE_TOR
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Polymer({
this.addWebUIListener('brave-needs-restart-changed', (needsRestart) => {
this.showRestartToast_ = needsRestart
})
this.addWebUIListener('tor-enabled-changed', (enabled) => {
this.addWebUIListener('tor-enabled-pref-changed', (enabled) => {
this.torEnabled_ = enabled
})

Expand Down
52 changes: 15 additions & 37 deletions browser/tor/tor_profile_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,55 +56,33 @@ 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);
// kTorDisabled is managed prefs.
// User can only change it via settiongs option if it's not managed.
// 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);
// kTorDisabledAtNextLaunching has the value that user changed via settings.
// If this value is changed, it is exposed to settings page. At the next
// launching, this value is copied to kTorDisabled.
registry->RegisterBooleanPref(prefs::kTorDisabledAtNextLaunching, false);
}

// static
void TorProfileService::InitializeTorPrefs() {
auto* state = g_browser_process->local_state();
if (state->FindPreference(
prefs::kTorDisabledAtNextLaunching)->IsDefaultValue()) {
return;
bool TorProfileService::IsTorDisabled() {
// In test, |g_brave_browser_process| could be null.
if (!g_brave_browser_process) {
return false;
}

state->SetBoolean(prefs::kTorDisabled,
state->GetBoolean(prefs::kTorDisabledAtNextLaunching));
state->ClearPref(prefs::kTorDisabledAtNextLaunching);
return !g_brave_browser_process->tor_client_updater()->registered();
}

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

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

// static
bool TorProfileService::IsTorDisabledChanged() {
auto* state = g_browser_process->local_state();
if (state->FindPreference(
prefs::kTorDisabledAtNextLaunching)->IsDefaultValue()) {
return false;
}

return state->GetBoolean(prefs::kTorDisabledAtNextLaunching) !=
state->GetBoolean(prefs::kTorDisabled);
}

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

std::string TorProfileService::GetTorProxyURI() {
Expand Down
7 changes: 3 additions & 4 deletions browser/tor/tor_profile_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class PrefRegistrySyncable;
}

class PrefRegistrySimple;
class BraveAppMenuBrowserTestWithTorDisabled;

namespace tor {

Expand All @@ -39,11 +40,9 @@ class TorProfileService : public KeyedService {
~TorProfileService() override;

static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
static void SetTorDisabledAtNextLaunching(bool disabled);
static void SetTorDisabledPref(bool disabled);
static bool GetTorDisabledPref();
static bool IsTorDisabled();
static bool IsTorDisabledAtNextLaunching();
static bool IsTorDisabledChanged();
static void InitializeTorPrefs();

virtual void SetNewTorCircuit(content::WebContents* web_contents) = 0;
virtual std::unique_ptr<net::ProxyConfigService>
Expand Down
35 changes: 9 additions & 26 deletions browser/ui/toolbar/brave_app_menu_model_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
#include "brave/components/brave_rewards/browser/buildflags/buildflags.h"
#include "brave/components/brave_sync/buildflags/buildflags.h"
#include "brave/components/brave_wallet/browser/buildflags/buildflags.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_browser_main.h"
#include "chrome/browser/chrome_browser_main_extra_parts.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_window.h"
Expand All @@ -29,6 +27,10 @@
#include "content/public/browser/notification_service.h"
#include "content/public/test/test_utils.h"

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

using BraveAppMenuBrowserTest = InProcessBrowserTest;

IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, BasicTest) {
Expand Down Expand Up @@ -135,32 +137,13 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, BasicTest) {
}

#if BUILDFLAG(ENABLE_TOR)
class ChromeBrowserMainExtraPartsTor : public ChromeBrowserMainExtraParts {
public:
ChromeBrowserMainExtraPartsTor() = default;

// ChromeBrowserMainExtraParts:
void PostProfileInit() override {
g_browser_process->local_state()->SetBoolean(tor::prefs::kTorDisabled,
true);
}

private:
DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsTor);
};

class BraveAppMenuBrowserTestWithTorDisabledPolicy
: public InProcessBrowserTest {
public:
void CreatedBrowserMainParts(content::BrowserMainParts* parts) override {
static_cast<ChromeBrowserMainParts*>(parts)->AddParts(
new ChromeBrowserMainExtraPartsTor);
}
};
// 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(BraveAppMenuBrowserTestWithTorDisabledPolicy,
TorDisabledTest) {
IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, TorAppMenuTest) {
auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
BraveAppMenuModel normal_model(browser_view->toolbar(), browser());
normal_model.Init();
Expand Down
36 changes: 16 additions & 20 deletions browser/ui/webui/settings/brave_default_extensions_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@ void BraveDefaultExtensionsHandler::RegisterMessages() {
web_ui()->RegisterMessageCallback(
"setTorEnabled",
base::BindRepeating(
&BraveDefaultExtensionsHandler::SetTorEnabledAtNextLaunching,
&BraveDefaultExtensionsHandler::SetTorEnabledPref,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"getTorEnabled",
base::BindRepeating(&BraveDefaultExtensionsHandler::IsTorEnabled,
base::BindRepeating(&BraveDefaultExtensionsHandler::GetTorEnabledPref,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"isTorManaged",
Expand All @@ -109,8 +109,8 @@ void BraveDefaultExtensionsHandler::InitializePrefCallbacks() {
#if BUILDFLAG(ENABLE_TOR)
local_state_change_registrar_.Init(g_brave_browser_process->local_state());
local_state_change_registrar_.Add(
tor::prefs::kTorDisabledAtNextLaunching,
base::Bind(&BraveDefaultExtensionsHandler::OnTorEnabledChanged,
tor::prefs::kTorDisabled,
base::Bind(&BraveDefaultExtensionsHandler::OnTorEnabledPrefChanged,
base::Unretained(this)));
#endif
}
Expand All @@ -128,7 +128,8 @@ bool BraveDefaultExtensionsHandler::IsRestartNeeded() {
return true;

#if BUILDFLAG(ENABLE_TOR)
if (tor::TorProfileService::IsTorDisabledChanged())
if (tor::TorProfileService::IsTorDisabled() !=
tor::TorProfileService::GetTorDisabledPref())
return true;
#endif

Expand Down Expand Up @@ -234,36 +235,31 @@ void BraveDefaultExtensionsHandler::SetMediaRouterEnabled(
}

#if BUILDFLAG(ENABLE_TOR)
void BraveDefaultExtensionsHandler::SetTorEnabledAtNextLaunching(
void BraveDefaultExtensionsHandler::SetTorEnabledPref(
const base::ListValue* args) {
CHECK_EQ(args->GetSize(), 1U);
bool enabled;
args->GetBoolean(0, &enabled);

AllowJavascript();
tor::TorProfileService::SetTorDisabledAtNextLaunching(!enabled);
tor::TorProfileService::SetTorDisabledPref(!enabled);
}

void BraveDefaultExtensionsHandler::IsTorEnabled(
void BraveDefaultExtensionsHandler::GetTorEnabledPref(
const base::ListValue* args) {
CHECK_EQ(args->GetSize(), 1U);
bool enabled = false;
if (tor::TorProfileService::IsTorDisabledChanged()) {
enabled = !tor::TorProfileService::IsTorDisabledAtNextLaunching();
} else {
enabled = !tor::TorProfileService::IsTorDisabled();
}

AllowJavascript();
ResolveJavascriptCallback(args->GetList()[0], base::Value(enabled));
ResolveJavascriptCallback(
args->GetList()[0],
base::Value(!tor::TorProfileService::GetTorDisabledPref()));
}

void BraveDefaultExtensionsHandler::OnTorEnabledChanged() {
void BraveDefaultExtensionsHandler::OnTorEnabledPrefChanged() {
OnRestartNeededChanged();

if (IsJavascriptAllowed()) {
FireWebUIListener("tor-enabled-changed",
base::Value(!tor::TorProfileService::IsTorDisabled()));
FireWebUIListener(
"tor-enabled-pref-changed",
base::Value(!tor::TorProfileService::GetTorDisabledPref()));
}
}

Expand Down
6 changes: 3 additions & 3 deletions browser/ui/webui/settings/brave_default_extensions_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ class BraveDefaultExtensionsHandler : public settings::SettingsPageUIHandler {
void SetMediaRouterEnabled(const base::ListValue* args);
void SetBraveWalletEnabled(const base::ListValue* args);
#if BUILDFLAG(ENABLE_TOR)
void SetTorEnabledAtNextLaunching(const base::ListValue* args);
void IsTorEnabled(const base::ListValue* args);
void OnTorEnabledChanged();
void SetTorEnabledPref(const base::ListValue* args);
void GetTorEnabledPref(const base::ListValue* args);
void OnTorEnabledPrefChanged();
void IsTorManaged(const base::ListValue* args);
#endif

Expand Down
2 changes: 0 additions & 2 deletions common/tor/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ namespace prefs {

const char kTorProxyString[] = "tor.tor_proxy_string";
const char kTorDisabled[] = "tor.tor_disabled";
const char kTorDisabledAtNextLaunching[] =
"tor.tor_disabled_at_next_launching";

} // namespace prefs
} // namespace tor
1 change: 0 additions & 1 deletion common/tor/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ namespace prefs {

extern const char kTorProxyString[];
extern const char kTorDisabled[];
extern const char kTorDisabledAtNextLaunching[];

} // namespace prefs
} // namespace tor
Expand Down

0 comments on commit 744f854

Please sign in to comment.