diff options
author | sergeyu <sergeyu@chromium.org> | 2015-01-30 15:33:25 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-01-30 23:34:24 +0000 |
commit | b2ae7e31aeeaa8f6dcffc4d2c859beea16ae97c2 (patch) | |
tree | 77526ad1266e6ebd263d1ab6a2c56a4342101d70 | |
parent | f53150c7287e14d357dd2671ab5a0bacae443b82 (diff) | |
download | chromium_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.cc | 15 | ||||
-rw-r--r-- | remoting/host/it2me/it2me_host.h | 4 | ||||
-rw-r--r-- | remoting/host/policy_watcher.cc | 101 | ||||
-rw-r--r-- | remoting/host/policy_watcher.h | 43 | ||||
-rw-r--r-- | remoting/host/policy_watcher_unittest.cc | 42 | ||||
-rw-r--r-- | remoting/host/remoting_me2me_host.cc | 37 |
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) { |