summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsergeyu <sergeyu@chromium.org>2015-01-30 15:33:25 -0800
committerCommit bot <commit-bot@chromium.org>2015-01-30 23:34:24 +0000
commitb2ae7e31aeeaa8f6dcffc4d2c859beea16ae97c2 (patch)
tree77526ad1266e6ebd263d1ab6a2c56a4342101d70
parentf53150c7287e14d357dd2671ab5a0bacae443b82 (diff)
downloadchromium_src-b2ae7e31aeeaa8f6dcffc4d2c859beea16ae97c2.zip
chromium_src-b2ae7e31aeeaa8f6dcffc4d2c859beea16ae97c2.tar.gz
chromium_src-b2ae7e31aeeaa8f6dcffc4d2c859beea16ae97c2.tar.bz2
Cleanup threading in PolicyWatcher.
Previously PolicyWatcher allowed initialization on a thread different from the one it runs on. It's no longer necessary. Also for It2Me it was initialized on UI thread, but was given task runner for the network thread, which causes the DCHECK in the linked bug. Now PolicyWatcher is marked as NonThreadSafe. It's initialized on UI thread for It2Me host and on NetworkThread for the me2me host. Also the file thread is used for blocking operation when reading policies. BUG=453615 Review URL: https://codereview.chromium.org/886913002 Cr-Commit-Position: refs/heads/master@{#314026}
-rw-r--r--remoting/host/it2me/it2me_host.cc15
-rw-r--r--remoting/host/it2me/it2me_host.h4
-rw-r--r--remoting/host/policy_watcher.cc101
-rw-r--r--remoting/host/policy_watcher.h43
-rw-r--r--remoting/host/policy_watcher_unittest.cc42
-rw-r--r--remoting/host/remoting_me2me_host.cc37
6 files changed, 65 insertions, 177 deletions
diff --git a/remoting/host/it2me/it2me_host.cc b/remoting/host/it2me/it2me_host.cc
index 2834dcd..22a9790 100644
--- a/remoting/host/it2me/it2me_host.cc
+++ b/remoting/host/it2me/it2me_host.cc
@@ -298,14 +298,6 @@ void It2MeHost::ShutdownOnUiThread() {
desktop_environment_factory_.reset();
// Stop listening for policy updates.
- if (policy_watcher_.get()) {
- policy_watcher_->StopWatching(
- base::Bind(&It2MeHost::OnPolicyWatcherShutdown, this));
- return;
- }
-}
-
-void It2MeHost::OnPolicyWatcherShutdown() {
policy_watcher_.reset();
}
@@ -356,8 +348,7 @@ void It2MeHost::OnClientDisconnected(const std::string& jid) {
}
void It2MeHost::OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies) {
- // The policy watcher runs on the |ui_task_runner| on ChromeOS and the
- // |network_task_runner| on other platforms.
+ // The policy watcher runs on the |ui_task_runner|.
if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
host_context_->network_task_runner()->PostTask(
FROM_HERE,
@@ -532,10 +523,12 @@ scoped_refptr<It2MeHost> It2MeHostFactory::CreateIt2MeHost(
base::WeakPtr<It2MeHost::Observer> observer,
const XmppSignalStrategy::XmppServerConfig& xmpp_server_config,
const std::string& directory_bot_jid) {
+ DCHECK(context->ui_task_runner()->BelongsToCurrentThread());
+
scoped_ptr<It2MeConfirmationDialogFactory> confirmation_dialog_factory(
new It2MeConfirmationDialogFactory());
scoped_ptr<PolicyWatcher> policy_watcher =
- PolicyWatcher::Create(policy_service_, context->network_task_runner());
+ PolicyWatcher::Create(policy_service_, context->file_task_runner());
return new It2MeHost(context.Pass(), policy_watcher.Pass(),
confirmation_dialog_factory.Pass(),
observer, xmpp_server_config, directory_bot_jid);
diff --git a/remoting/host/it2me/it2me_host.h b/remoting/host/it2me/it2me_host.h
index c7fa60e..cbc835c 100644
--- a/remoting/host/it2me/it2me_host.h
+++ b/remoting/host/it2me/it2me_host.h
@@ -132,10 +132,6 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
// the UI thread.
void ShutdownOnUiThread();
- // Called when |policy_watcher_| has stopped listening for changes and it is
- // safe to delete it.
- void OnPolicyWatcherShutdown();
-
// Called when initial policies are read, and when they change.
void OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies);
diff --git a/remoting/host/policy_watcher.cc b/remoting/host/policy_watcher.cc
index 9c71cb2..8ef59a7 100644
--- a/remoting/host/policy_watcher.cc
+++ b/remoting/host/policy_watcher.cc
@@ -26,9 +26,7 @@
#include "base/json/json_reader.h"
#endif
-#if defined(OS_CHROMEOS)
-#include "content/public/browser/browser_thread.h"
-#elif defined(OS_WIN)
+#if defined(OS_WIN)
#include "components/policy/core/common/policy_loader_win.h"
#elif defined(OS_MACOSX)
#include "components/policy/core/common/policy_loader_mac.h"
@@ -93,13 +91,10 @@ policy::PolicyNamespace GetPolicyNamespace() {
void PolicyWatcher::StartWatching(
const PolicyUpdatedCallback& policy_updated_callback,
const PolicyErrorCallback& policy_error_callback) {
- if (!OnPolicyServiceThread()) {
- policy_service_task_runner_->PostTask(
- FROM_HERE,
- base::Bind(&PolicyWatcher::StartWatching, base::Unretained(this),
- policy_updated_callback, policy_error_callback));
- return;
- }
+ DCHECK(CalledOnValidThread());
+ DCHECK(!policy_updated_callback.is_null());
+ DCHECK(!policy_error_callback.is_null());
+ DCHECK(policy_updated_callback_.is_null());
policy_updated_callback_ = policy_updated_callback;
policy_error_callback_ = policy_error_callback;
@@ -113,26 +108,9 @@ void PolicyWatcher::StartWatching(
}
}
-void PolicyWatcher::StopWatching(const base::Closure& stopped_callback) {
- policy_service_task_runner_->PostTaskAndReply(
- FROM_HERE, base::Bind(&PolicyWatcher::StopWatchingOnPolicyServiceThread,
- base::Unretained(this)),
- stopped_callback);
-}
-
-void PolicyWatcher::StopWatchingOnPolicyServiceThread() {
- policy_service_->RemoveObserver(policy::POLICY_DOMAIN_CHROME, this);
- policy_updated_callback_.Reset();
- policy_error_callback_.Reset();
-}
-
-bool PolicyWatcher::OnPolicyServiceThread() const {
- return policy_service_task_runner_->BelongsToCurrentThread();
-}
-
void PolicyWatcher::UpdatePolicies(
const base::DictionaryValue* new_policies_raw) {
- DCHECK(OnPolicyServiceThread());
+ DCHECK(CalledOnValidThread());
transient_policy_error_retry_counter_ = 0;
@@ -176,14 +154,11 @@ void PolicyWatcher::SignalTransientPolicyError() {
}
PolicyWatcher::PolicyWatcher(
- const scoped_refptr<base::SingleThreadTaskRunner>&
- policy_service_task_runner,
policy::PolicyService* policy_service,
scoped_ptr<policy::PolicyService> owned_policy_service,
scoped_ptr<policy::ConfigurationPolicyProvider> owned_policy_provider,
scoped_ptr<policy::SchemaRegistry> owned_schema_registry)
- : policy_service_task_runner_(policy_service_task_runner),
- transient_policy_error_retry_counter_(0),
+ : transient_policy_error_retry_counter_(0),
old_policies_(new base::DictionaryValue()),
default_values_(new base::DictionaryValue()),
policy_service_(policy_service),
@@ -222,6 +197,11 @@ PolicyWatcher::PolicyWatcher(
}
PolicyWatcher::~PolicyWatcher() {
+ // Stop observing |policy_service_| if StartWatching() has been called.
+ if (!policy_updated_callback_.is_null()) {
+ policy_service_->RemoveObserver(policy::POLICY_DOMAIN_CHROME, this);
+ }
+
if (owned_policy_provider_) {
owned_policy_provider_->Shutdown();
}
@@ -245,8 +225,6 @@ void PolicyWatcher::OnPolicyServiceInitialized(policy::PolicyDomain domain) {
}
scoped_ptr<PolicyWatcher> PolicyWatcher::CreateFromPolicyLoader(
- const scoped_refptr<base::SingleThreadTaskRunner>&
- policy_service_task_runner,
scoped_ptr<policy::AsyncPolicyLoader> async_policy_loader) {
// TODO(lukasza): Schema below should ideally only cover Chromoting-specific
// policies (expecting perf and maintanability improvement, but no functional
@@ -268,49 +246,46 @@ scoped_ptr<PolicyWatcher> PolicyWatcher::CreateFromPolicyLoader(
new policy::PolicyServiceImpl(providers));
policy::PolicyService* borrowed_policy_service = policy_service.get();
- return make_scoped_ptr(new PolicyWatcher(
- policy_service_task_runner, borrowed_policy_service,
- policy_service.Pass(), policy_provider.Pass(), schema_registry.Pass()));
+ return make_scoped_ptr(
+ new PolicyWatcher(borrowed_policy_service, policy_service.Pass(),
+ policy_provider.Pass(), schema_registry.Pass()));
}
scoped_ptr<PolicyWatcher> PolicyWatcher::Create(
policy::PolicyService* policy_service,
- const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner) {
+ const scoped_refptr<base::SingleThreadTaskRunner>& file_task_runner) {
#if defined(OS_CHROMEOS)
+ // On Chrome OS the PolicyService is owned by the browser.
DCHECK(policy_service);
return make_scoped_ptr(
- new PolicyWatcher(content::BrowserThread::GetMessageLoopProxyForThread(
- content::BrowserThread::UI),
- policy_service, nullptr, nullptr, nullptr));
-#elif defined(OS_WIN)
+ new PolicyWatcher(policy_service, nullptr, nullptr, nullptr));
+#else // !defined(OS_CHROMEOS)
DCHECK(!policy_service);
- // Always read the Chrome policies (even on Chromium) so that policy
- // enforcement can't be bypassed by running Chromium.
- // Note that this comment applies to all of Win/Mac/Posix branches below.
- static const wchar_t kRegistryKey[] = L"SOFTWARE\\Policies\\Google\\Chrome";
- return PolicyWatcher::CreateFromPolicyLoader(
- network_task_runner,
- policy::PolicyLoaderWin::Create(network_task_runner, kRegistryKey));
+
+ // Create platform-specific PolicyLoader. Always read the Chrome policies
+ // (even on Chromium) so that policy enforcement can't be bypassed by running
+ // Chromium.
+ scoped_ptr<policy::AsyncPolicyLoader> policy_loader;
+#if defined(OS_WIN)
+ policy_loader = policy::PolicyLoaderWin::Create(
+ file_task_runner, L"SOFTWARE\\Policies\\Google\\Chrome");
#elif defined(OS_MACOSX)
CFStringRef bundle_id = CFSTR("com.google.Chrome");
- DCHECK(!policy_service);
- return PolicyWatcher::CreateFromPolicyLoader(
- network_task_runner,
- make_scoped_ptr(new policy::PolicyLoaderMac(
- network_task_runner,
- policy::PolicyLoaderMac::GetManagedPolicyPath(bundle_id),
- new MacPreferences(), bundle_id)));
+ policy_loader.reset(new policy::PolicyLoaderMac(
+ file_task_runner,
+ policy::PolicyLoaderMac::GetManagedPolicyPath(bundle_id),
+ new MacPreferences(), bundle_id));
#elif defined(OS_POSIX) && !defined(OS_ANDROID)
- DCHECK(!policy_service);
- static const base::FilePath::CharType kPolicyDir[] =
- FILE_PATH_LITERAL("/etc/opt/chrome/policies");
- return PolicyWatcher::CreateFromPolicyLoader(
- network_task_runner, make_scoped_ptr(new policy::ConfigDirPolicyLoader(
- network_task_runner, base::FilePath(kPolicyDir),
- policy::POLICY_SCOPE_MACHINE)));
+ policy_loader.reset(new policy::ConfigDirPolicyLoader(
+ file_task_runner,
+ base::FilePath(FILE_PATH_LITERAL("/etc/opt/chrome/policies")),
+ policy::POLICY_SCOPE_MACHINE));
#else
#error OS that is not yet supported by PolicyWatcher code.
#endif
+
+ return PolicyWatcher::CreateFromPolicyLoader(policy_loader.Pass());
+#endif // !(OS_CHROMEOS)
}
} // namespace remoting
diff --git a/remoting/host/policy_watcher.h b/remoting/host/policy_watcher.h
index 513c4c1..b4d6dda 100644
--- a/remoting/host/policy_watcher.h
+++ b/remoting/host/policy_watcher.h
@@ -9,6 +9,7 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
+#include "base/threading/non_thread_safe.h"
#include "components/policy/core/common/policy_service.h"
namespace base {
@@ -24,11 +25,9 @@ class SchemaRegistry;
namespace remoting {
-// Watches for changes to the managed remote access host policies. If
-// StartWatching() has been called, then before this object can be deleted,
-// StopWatching() has to be completed (the provided |done| event must be
-// signaled).
-class PolicyWatcher : public policy::PolicyService::Observer {
+// Watches for changes to the managed remote access host policies.
+class PolicyWatcher : public policy::PolicyService::Observer,
+ public base::NonThreadSafe {
public:
// Called first with all policies, and subsequently with any changed policies.
typedef base::Callback<void(scoped_ptr<base::DictionaryValue>)>
@@ -60,42 +59,32 @@ class PolicyWatcher : public policy::PolicyService::Observer {
// for policy changes and will call |policy_updated_callback| when the error
// is recovered from and may call |policy_error_callback| when new errors are
// found.
- //
- // See |Create| method's description for comments about which thread will
- // be used to run the callbacks.
virtual void StartWatching(
const PolicyUpdatedCallback& policy_updated_callback,
const PolicyErrorCallback& policy_error_callback);
- // Should be called after StartWatching() before the object is deleted. Calls
- // should wait for |stopped_callback| to be called before deleting it.
- virtual void StopWatching(const base::Closure& stopped_callback);
-
// Specify a |policy_service| to borrow (on Chrome OS, from the browser
// process) or specify nullptr to internally construct and use a new
- // PolicyService (on other OS-es).
+ // PolicyService (on other OS-es). PolicyWatcher must be used on the thread on
+ // which it is created. |policy_service| is called on the same thread.
//
- // When |policy_service| is null, then |task_runner| is used for reading the
- // policy from files / registry / preferences. PolicyUpdatedCallback and
- // PolicyErrorCallback will be called on the same |task_runner|.
- // |task_runner| should be of TYPE_IO type.
+ // When |policy_service| is null, then |file_task_runner| is used for reading
+ // the policy from files / registry / preferences (which are blocking
+ // operations). |file_task_runner| should be of TYPE_IO type.
//
- // When |policy_service| is specified then |task_runner| argument is ignored
- // and 1) BrowserThread::UI is used for PolicyUpdatedCallback and
+ // When |policy_service| is specified then |file_task_runner| argument is
+ // ignored and 1) BrowserThread::UI is used for PolicyUpdatedCallback and
// PolicyErrorCallback and 2) BrowserThread::FILE is used for reading the
// policy from files / registry / preferences (although (2) is just an
// implementation detail and should likely be ignored outside of
// PolicyWatcher).
static scoped_ptr<PolicyWatcher> Create(
policy::PolicyService* policy_service,
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
+ const scoped_refptr<base::SingleThreadTaskRunner>& file_task_runner);
private:
friend class PolicyWatcherTest;
- // Used to check if the class is on the right thread.
- bool OnPolicyServiceThread() const;
-
// Takes the policy dictionary from the OS specific store and extracts the
// relevant policies.
void UpdatePolicies(const base::DictionaryValue* new_policy);
@@ -113,8 +102,6 @@ class PolicyWatcher : public policy::PolicyService::Observer {
// to call |policy_service_| methods and where we expect to get callbacks
// from |policy_service_|.
PolicyWatcher(
- const scoped_refptr<base::SingleThreadTaskRunner>&
- policy_service_task_runner,
policy::PolicyService* policy_service,
scoped_ptr<policy::PolicyService> owned_policy_service,
scoped_ptr<policy::ConfigurationPolicyProvider> owned_policy_provider,
@@ -126,8 +113,6 @@ class PolicyWatcher : public policy::PolicyService::Observer {
// |policy_service_task_runner| is passed through to the constructor of
// PolicyWatcher.
static scoped_ptr<PolicyWatcher> CreateFromPolicyLoader(
- const scoped_refptr<base::SingleThreadTaskRunner>&
- policy_service_task_runner,
scoped_ptr<policy::AsyncPolicyLoader> async_policy_loader);
// PolicyService::Observer interface.
@@ -136,10 +121,6 @@ class PolicyWatcher : public policy::PolicyService::Observer {
const policy::PolicyMap& current) override;
void OnPolicyServiceInitialized(policy::PolicyDomain domain) override;
- void StopWatchingOnPolicyServiceThread();
-
- scoped_refptr<base::SingleThreadTaskRunner> policy_service_task_runner_;
-
PolicyUpdatedCallback policy_updated_callback_;
PolicyErrorCallback policy_error_callback_;
int transient_policy_error_retry_counter_;
diff --git a/remoting/host/policy_watcher_unittest.cc b/remoting/host/policy_watcher_unittest.cc
index 7c2af04..850048c 100644
--- a/remoting/host/policy_watcher_unittest.cc
+++ b/remoting/host/policy_watcher_unittest.cc
@@ -42,8 +42,8 @@ class PolicyWatcherTest : public testing::Test {
// Retaining a raw pointer to keep control over policy contents.
policy_loader_ = new policy::FakeAsyncPolicyLoader(message_loop_proxy_);
- policy_watcher_ = PolicyWatcher::CreateFromPolicyLoader(
- message_loop_proxy_, make_scoped_ptr(policy_loader_));
+ policy_watcher_ =
+ PolicyWatcher::CreateFromPolicyLoader(make_scoped_ptr(policy_loader_));
nat_true_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal, true);
nat_false_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal,
@@ -138,13 +138,6 @@ class PolicyWatcherTest : public testing::Test {
base::RunLoop().RunUntilIdle();
}
- void StopWatching() {
- EXPECT_CALL(*this, PostPolicyWatcherShutdown()).Times(1);
- policy_watcher_->StopWatching(base::Bind(
- &PolicyWatcherTest::PostPolicyWatcherShutdown, base::Unretained(this)));
- base::RunLoop().RunUntilIdle();
- }
-
void SetPolicies(const base::DictionaryValue& dict) {
// Copy |dict| into |policy_bundle|.
policy::PolicyNamespace policy_namespace =
@@ -257,7 +250,6 @@ TEST_F(PolicyWatcherTest, None) {
SetPolicies(empty_);
StartWatching();
- StopWatching();
}
TEST_F(PolicyWatcherTest, NatTrue) {
@@ -266,7 +258,6 @@ TEST_F(PolicyWatcherTest, NatTrue) {
SetPolicies(nat_true_);
StartWatching();
- StopWatching();
}
TEST_F(PolicyWatcherTest, NatFalse) {
@@ -275,7 +266,6 @@ TEST_F(PolicyWatcherTest, NatFalse) {
SetPolicies(nat_false_);
StartWatching();
- StopWatching();
}
TEST_F(PolicyWatcherTest, NatOne) {
@@ -284,7 +274,6 @@ TEST_F(PolicyWatcherTest, NatOne) {
SetPolicies(nat_one_);
StartWatching();
- StopWatching();
}
TEST_F(PolicyWatcherTest, DomainEmpty) {
@@ -293,7 +282,6 @@ TEST_F(PolicyWatcherTest, DomainEmpty) {
SetPolicies(domain_empty_);
StartWatching();
- StopWatching();
}
TEST_F(PolicyWatcherTest, DomainFull) {
@@ -302,7 +290,6 @@ TEST_F(PolicyWatcherTest, DomainFull) {
SetPolicies(domain_full_);
StartWatching();
- StopWatching();
}
TEST_F(PolicyWatcherTest, NatNoneThenTrue) {
@@ -312,7 +299,6 @@ TEST_F(PolicyWatcherTest, NatNoneThenTrue) {
SetPolicies(empty_);
StartWatching();
SetPolicies(nat_true_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, NatNoneThenTrueThenTrue) {
@@ -323,7 +309,6 @@ TEST_F(PolicyWatcherTest, NatNoneThenTrueThenTrue) {
StartWatching();
SetPolicies(nat_true_);
SetPolicies(nat_true_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, NatNoneThenTrueThenTrueThenFalse) {
@@ -338,7 +323,6 @@ TEST_F(PolicyWatcherTest, NatNoneThenTrueThenTrueThenFalse) {
SetPolicies(nat_true_);
SetPolicies(nat_true_);
SetPolicies(nat_false_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, NatNoneThenFalse) {
@@ -351,7 +335,6 @@ TEST_F(PolicyWatcherTest, NatNoneThenFalse) {
SetPolicies(empty_);
StartWatching();
SetPolicies(nat_false_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, NatNoneThenFalseThenTrue) {
@@ -366,7 +349,6 @@ TEST_F(PolicyWatcherTest, NatNoneThenFalseThenTrue) {
StartWatching();
SetPolicies(nat_false_);
SetPolicies(nat_true_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, ChangeOneRepeatedlyThenTwo) {
@@ -389,7 +371,6 @@ TEST_F(PolicyWatcherTest, ChangeOneRepeatedlyThenTwo) {
SetPolicies(nat_false_domain_full_);
SetPolicies(nat_false_domain_empty_);
SetPolicies(nat_true_domain_full_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, FilterUnknownPolicies) {
@@ -401,7 +382,6 @@ TEST_F(PolicyWatcherTest, FilterUnknownPolicies) {
StartWatching();
SetPolicies(unknown_policies_);
SetPolicies(empty_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, DebugOverrideNatPolicy) {
@@ -416,7 +396,6 @@ TEST_F(PolicyWatcherTest, DebugOverrideNatPolicy) {
SetPolicies(nat_true_and_overridden_);
StartWatching();
- StopWatching();
}
TEST_F(PolicyWatcherTest, PairingFalseThenTrue) {
@@ -432,7 +411,6 @@ TEST_F(PolicyWatcherTest, PairingFalseThenTrue) {
StartWatching();
SetPolicies(pairing_false_);
SetPolicies(pairing_true_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, GnubbyAuth) {
@@ -448,7 +426,6 @@ TEST_F(PolicyWatcherTest, GnubbyAuth) {
StartWatching();
SetPolicies(gnubby_auth_false_);
SetPolicies(gnubby_auth_true_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, Relay) {
@@ -464,7 +441,6 @@ TEST_F(PolicyWatcherTest, Relay) {
StartWatching();
SetPolicies(relay_false_);
SetPolicies(relay_true_);
- StopWatching();
}
TEST_F(PolicyWatcherTest, UdpPortRange) {
@@ -480,7 +456,6 @@ TEST_F(PolicyWatcherTest, UdpPortRange) {
StartWatching();
SetPolicies(port_range_full_);
SetPolicies(port_range_empty_);
- StopWatching();
}
const int kMaxTransientErrorRetries = 5;
@@ -490,7 +465,6 @@ TEST_F(PolicyWatcherTest, SingleTransientErrorDoesntTriggerErrorCallback) {
StartWatching();
SignalTransientErrorForTest();
- StopWatching();
}
TEST_F(PolicyWatcherTest, MultipleTransientErrorsTriggerErrorCallback) {
@@ -500,7 +474,6 @@ TEST_F(PolicyWatcherTest, MultipleTransientErrorsTriggerErrorCallback) {
for (int i = 0; i < kMaxTransientErrorRetries; i++) {
SignalTransientErrorForTest();
}
- StopWatching();
}
TEST_F(PolicyWatcherTest, PolicyUpdateResetsTransientErrorsCounter) {
@@ -516,7 +489,6 @@ TEST_F(PolicyWatcherTest, PolicyUpdateResetsTransientErrorsCounter) {
for (int i = 0; i < (kMaxTransientErrorRetries - 1); i++) {
SignalTransientErrorForTest();
}
- StopWatching();
}
// Unit tests cannot instantiate PolicyWatcher on ChromeOS
@@ -571,16 +543,6 @@ TEST_F(PolicyWatcherTest, TestRealChromotingPolicy) {
run_loop.RunUntilIdle();
}
- {
- base::RunLoop run_loop;
- PolicyWatcher* raw_policy_watcher = policy_watcher.release();
- raw_policy_watcher->StopWatching(
- base::Bind(base::IgnoreResult(
- &base::SequencedTaskRunner::DeleteSoon<PolicyWatcher>),
- task_runner, FROM_HERE, raw_policy_watcher));
- run_loop.RunUntilIdle();
- }
-
// Today, the only verification offered by this test is:
// - Manual verification of policy values dumped by OnPolicyUpdatedDumpPolicy
// - Automated verification that nothing crashed
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 07f41c5..aa0bf6f 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -277,8 +277,6 @@ class HostProcess : public ConfigWatcher::Delegate,
void ShutdownOnNetworkThread();
- void OnPolicyWatcherShutdown();
-
#if defined(OS_WIN)
// Initializes the pairing registry on Windows. This should be invoked on the
// network thread.
@@ -555,7 +553,7 @@ void HostProcess::OnConfigUpdated(
// loading from policy verifications and move |policy_watcher_|
// initialization to StartOnNetworkThread().
policy_watcher_ =
- PolicyWatcher::Create(nullptr, context_->network_task_runner());
+ PolicyWatcher::Create(nullptr, context_->file_task_runner());
policy_watcher_->StartWatching(
base::Bind(&HostProcess::OnPolicyUpdate, base::Unretained(this)),
base::Bind(&HostProcess::OnPolicyError, base::Unretained(this)));
@@ -958,11 +956,7 @@ bool HostProcess::ApplyConfig(const base::DictionaryValue& config) {
}
void HostProcess::OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies) {
- if (!context_->network_task_runner()->BelongsToCurrentThread()) {
- context_->network_task_runner()->PostTask(FROM_HERE, base::Bind(
- &HostProcess::OnPolicyUpdate, this, base::Passed(&policies)));
- return;
- }
+ DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
bool restart_required = false;
restart_required |= OnHostDomainPolicyUpdate(policies.get());
@@ -985,12 +979,9 @@ void HostProcess::OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies) {
}
void HostProcess::OnPolicyError() {
- context_->network_task_runner()->PostTask(
- FROM_HERE,
- base::Bind(
- &HostProcess::ShutdownHost,
- this,
- kInvalidHostConfigurationExitCode));
+ DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
+
+ ShutdownHost(kInvalidHostConfigurationExitCode);
}
void HostProcess::ApplyHostDomainPolicy() {
@@ -1448,27 +1439,17 @@ void HostProcess::ShutdownOnNetworkThread() {
shutdown_watchdog_->Arm();
config_watcher_.reset();
+ policy_watcher_.reset();
- if (policy_watcher_.get()) {
- policy_watcher_->StopWatching(
- base::Bind(&HostProcess::OnPolicyWatcherShutdown, this));
- } else {
- OnPolicyWatcherShutdown();
- }
+ // Complete the rest of shutdown on the main thread.
+ context_->ui_task_runner()->PostTask(
+ FROM_HERE, base::Bind(&HostProcess::ShutdownOnUiThread, this));
} else {
// This method is only called in STOPPING_TO_RESTART and STOPPING states.
NOTREACHED();
}
}
-void HostProcess::OnPolicyWatcherShutdown() {
- policy_watcher_.reset();
-
- // Complete the rest of shutdown on the main thread.
- context_->ui_task_runner()->PostTask(
- FROM_HERE, base::Bind(&HostProcess::ShutdownOnUiThread, this));
-}
-
void HostProcess::OnCrash(const std::string& function_name,
const std::string& file_name,
const int& line_number) {