Skip to content

GitLab

T
tangled
Commit a9376cda authored by lukasza's avatar lukasza Committed by Commit bot
Browse files
Options

Merged PolicyServiceWatcher into PolicyWatcher. 

BUG=368321

Review URL: https://codereview.chromium.org/867463006

Cr-Commit-Position: refs/heads/master@{#313570}
parent f96a3d3a
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 246 additions and 370 deletions
remoting/host/policy_hack/policy_service_watcher.cc deleted 100644 → 0
View file @ f96a3d3a
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "remoting/host/policy_hack/policy_service_watcher.h"
#include "base/files/file_path.h"
#include "base/values.h"
#include "components/policy/core/common/async_policy_loader.h"
#include "components/policy/core/common/async_policy_provider.h"
#include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/policy_service_impl.h"
#include "components/policy/core/common/schema.h"
#include "components/policy/core/common/schema_registry.h"
#include "policy/policy_constants.h"
#if defined(OS_CHROMEOS)
#include "content/public/browser/browser_thread.h"
#elif 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"
#include "components/policy/core/common/preferences_mac.h"
#elif defined(OS_POSIX) && !defined(OS_ANDROID)
#include "components/policy/core/common/config_dir_policy_loader.h"
#endif
using namespace policy;
namespace remoting {
namespace policy_hack {
namespace {
PolicyNamespace GetPolicyNamespace() {
return PolicyNamespace(POLICY_DOMAIN_CHROME, std::string());
}
} // namespace
PolicyServiceWatcher::PolicyServiceWatcher(
const scoped_refptr<base::SingleThreadTaskRunner>&
policy_service_task_runner,
PolicyService* policy_service)
: PolicyWatcher(policy_service_task_runner) {
policy_service_ = policy_service;
}
PolicyServiceWatcher::PolicyServiceWatcher(
const scoped_refptr<base::SingleThreadTaskRunner>&
policy_service_task_runner,
scoped_ptr<PolicyService> owned_policy_service,
scoped_ptr<ConfigurationPolicyProvider> owned_policy_provider,
scoped_ptr<SchemaRegistry> owned_schema_registry)
: PolicyWatcher(policy_service_task_runner),
owned_schema_registry_(owned_schema_registry.Pass()),
owned_policy_provider_(owned_policy_provider.Pass()),
owned_policy_service_(owned_policy_service.Pass()) {
policy_service_ = owned_policy_service_.get();
}
PolicyServiceWatcher::~PolicyServiceWatcher() {
if (owned_policy_provider_) {
owned_policy_provider_->Shutdown();
}
}
void PolicyServiceWatcher::OnPolicyUpdated(const PolicyNamespace& ns,
const PolicyMap& previous,
const PolicyMap& current) {
scoped_ptr<base::DictionaryValue> policy_dict(new base::DictionaryValue());
for (PolicyMap::const_iterator it = current.begin(); it != current.end();
it++) {
// TODO(lukasza): Use policy::Schema::Normalize() for schema verification.
policy_dict->Set(it->first, it->second.value->DeepCopy());
}
UpdatePolicies(policy_dict.get());
}
void PolicyServiceWatcher::OnPolicyServiceInitialized(PolicyDomain domain) {
PolicyNamespace ns = GetPolicyNamespace();
const PolicyMap& current = policy_service_->GetPolicies(ns);
OnPolicyUpdated(ns, current, current);
}
void PolicyServiceWatcher::StartWatchingInternal() {
// Listen for future policy changes.
policy_service_->AddObserver(POLICY_DOMAIN_CHROME, this);
// Process current policy state.
if (policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME)) {
OnPolicyServiceInitialized(POLICY_DOMAIN_CHROME);
}
}
void PolicyServiceWatcher::StopWatchingInternal() {
policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, this);
}
scoped_ptr<PolicyServiceWatcher> PolicyServiceWatcher::CreateFromPolicyLoader(
const scoped_refptr<base::SingleThreadTaskRunner>&
policy_service_task_runner,
scoped_ptr<AsyncPolicyLoader> async_policy_loader) {
// TODO(lukasza): Schema below should ideally only cover Chromoting-specific
// policies (expecting perf and maintanability improvement, but no functional
// impact).
Schema schema = Schema::Wrap(GetChromeSchemaData());
scoped_ptr<SchemaRegistry> schema_registry(new SchemaRegistry());
schema_registry->RegisterComponent(GetPolicyNamespace(), schema);
scoped_ptr<AsyncPolicyProvider> policy_provider(new AsyncPolicyProvider(
schema_registry.get(), async_policy_loader.Pass()));
policy_provider->Init(schema_registry.get());
PolicyServiceImpl::Providers providers;
providers.push_back(policy_provider.get());
scoped_ptr<PolicyService> policy_service(new PolicyServiceImpl(providers));
return make_scoped_ptr(new PolicyServiceWatcher(
policy_service_task_runner, 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) {
#if defined(OS_CHROMEOS)
DCHECK(policy_service);
return make_scoped_ptr(new PolicyServiceWatcher(
content::BrowserThread::GetMessageLoopProxyForThread(
content::BrowserThread::UI),
policy_service));
#elif defined(OS_WIN)
DCHECK(!policy_service);
static const wchar_t kRegistryKey[] = L"SOFTWARE\\Policies\\Google\\Chrome";
return PolicyServiceWatcher::CreateFromPolicyLoader(
network_task_runner,
PolicyLoaderWin::Create(network_task_runner, kRegistryKey));
#elif defined(OS_MACOSX)
CFStringRef bundle_id = CFSTR("com.google.Chrome");
DCHECK(!policy_service);
return PolicyServiceWatcher::CreateFromPolicyLoader(
network_task_runner,
make_scoped_ptr(new PolicyLoaderMac(
network_task_runner,
policy::PolicyLoaderMac::GetManagedPolicyPath(bundle_id),
new MacPreferences(), bundle_id)));
#elif defined(OS_POSIX) && !defined(OS_ANDROID)
DCHECK(!policy_service);
// Always read the Chrome policies (even on Chromium) so that policy
// enforcement can't be bypassed by running Chromium.
static const base::FilePath::CharType kPolicyDir[] =
FILE_PATH_LITERAL("/etc/opt/chrome/policies");
return PolicyServiceWatcher::CreateFromPolicyLoader(
network_task_runner, make_scoped_ptr(new ConfigDirPolicyLoader(
network_task_runner, base::FilePath(kPolicyDir),
POLICY_SCOPE_MACHINE)));
#else
#error OS that is not yet supported by PolicyWatcher code.
#endif
}
} // namespace policy_hack
} // namespace remoting
remoting/host/policy_hack/policy_service_watcher.h deleted 100644 → 0
View file @ f96a3d3a
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef REMOTING_HOST_POLICY_HACK_POLICY_SERVICE_WATCHER_H_
#define REMOTING_HOST_POLICY_HACK_POLICY_SERVICE_WATCHER_H_
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "components/policy/core/common/policy_service.h"
#include "remoting/host/policy_hack/policy_watcher.h"
namespace base {
class SingleThreadTaskRunner;
} // namespace base
namespace policy {
class AsyncPolicyLoader;
class ConfigurationPolicyProvider;
class SchemaRegistry;
} // namespace policy
namespace remoting {
namespace policy_hack {
// TODO(lukasza): Merge PolicyServiceWatcher with PolicyWatcher class.
// PolicyServiceWatcher is a concrete implementation of PolicyWatcher that wraps
// an instance of PolicyService.
class PolicyServiceWatcher : public PolicyWatcher,
public policy::PolicyService::Observer {
public:
// Constructor for the case when |policy_service| is borrowed.
//
// |policy_service_task_runner| is the task runner where it is safe
// to call |policy_service| methods and where we expect to get callbacks
// from |policy_service|.
PolicyServiceWatcher(const scoped_refptr<base::SingleThreadTaskRunner>&
policy_service_task_runner,
policy::PolicyService* policy_service);
// Constructor for the case when |policy_service| is owned (and uses also
// owned |owned_policy_provider| and |owned_schema_registry|.
//
// |policy_service_task_runner| is the task runner where it is safe
// to call |policy_service| methods and where we expect to get callbacks
// from |policy_service|.
PolicyServiceWatcher(
const scoped_refptr<base::SingleThreadTaskRunner>&
policy_service_task_runner,
scoped_ptr<policy::PolicyService> owned_policy_service,
scoped_ptr<policy::ConfigurationPolicyProvider> owned_policy_provider,
scoped_ptr<policy::SchemaRegistry> owned_schema_registry);
~PolicyServiceWatcher() override;
// Creates PolicyServiceWatcher that wraps the owned |async_policy_loader|
// with an appropriate PolicySchema.
//
// |policy_service_task_runner| is passed through to the constructor
// of PolicyServiceWatcher.
static scoped_ptr<PolicyServiceWatcher> CreateFromPolicyLoader(
const scoped_refptr<base::SingleThreadTaskRunner>&
policy_service_task_runner,
scoped_ptr<policy::AsyncPolicyLoader> async_policy_loader);
// PolicyService::Observer interface.
void OnPolicyUpdated(const policy::PolicyNamespace& ns,
const policy::PolicyMap& previous,
const policy::PolicyMap& current) override;
void OnPolicyServiceInitialized(policy::PolicyDomain domain) override;
protected:
// PolicyWatcher overrides.
void StartWatchingInternal() override;
void StopWatchingInternal() override;
private:
policy::PolicyService* policy_service_;
// Order of fields below is important to ensure destruction takes object
// dependencies into account:
// - |owned_policy_service_| uses |owned_policy_provider_|
// - |owned_policy_provider_| uses |owned_schema_registry_|
scoped_ptr<policy::SchemaRegistry> owned_schema_registry_;
scoped_ptr<policy::ConfigurationPolicyProvider> owned_policy_provider_;
scoped_ptr<policy::PolicyService> owned_policy_service_;
DISALLOW_COPY_AND_ASSIGN(PolicyServiceWatcher);
};
} // namespace policy_hack
} // namespace remoting
#endif // REMOTING_HOST_POLICY_HACK_POLICY_SERVICE_WATCHER_H_
remoting/host/policy_hack/policy_watcher.cc
View file @ a9376cda
This diff is collapsed.
remoting/host/policy_hack/policy_watcher.h
View file @ a9376cda
......@@ -6,48 +6,45 @@
#define REMOTING_HOST_POLICY_HACK_POLICY_WATCHER_H_
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "base/values.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "components/policy/core/common/policy_service.h"
namespace base {
class DictionaryValue;
class SingleThreadTaskRunner;
class TimeDelta;
class WaitableEvent;
} // namespace base
namespace policy {
class PolicyService;
class AsyncPolicyLoader;
class ConfigurationPolicyProvider;
class SchemaRegistry;
} // namespace policy
namespace remoting {
namespace policy_hack {
// Watches for changes to the managed remote access host policies.
// If StartWatching() has been called, then before this object can be deleted,
// StopWatching() have completed (the provided |done| event must be signaled).
class PolicyWatcher {
// 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 {
public:
// Called first with all policies, and subsequently with any changed policies.
typedef base::Callback<void(scoped_ptr<base::DictionaryValue>)>
PolicyUpdatedCallback;
// TODO(lukasza): PolicyErrorCallback never gets called by
// PolicyServiceWatcher. Need to either 1) remove error-handling from
// PolicyWatcher or 2) add error-handling around PolicyService
// 2a) Add policy name/type validation via policy::Schema::Normalize.
// 2b) Consider exposing parsing errors from policy::ConfigDirPolicyLoader.
// TODO(lukasza): PolicyErrorCallback never gets called by PolicyWatcher.
// Need to either 1) remove error-handling from PolicyWatcher or 2) add
// error-handling around PolicyService 2a) Add policy name/type validation via
// policy::Schema::Normalize. 2b) Consider exposing parsing errors from
// policy::ConfigDirPolicyLoader.
// Called after detecting malformed policies.
typedef base::Callback<void()> PolicyErrorCallback;
// Derived classes specify which |task_runner| should be used for calling
// their StartWatchingInternal and StopWatchingInternal methods.
// Derived classes promise back to call UpdatePolicies and other instance
// methods on the same |task_runner|.
explicit PolicyWatcher(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
virtual ~PolicyWatcher();
~PolicyWatcher() override;
// This guarantees that the |policy_updated_callback| is called at least once
// with the current policies. After that, |policy_updated_callback| will be
......@@ -94,12 +91,11 @@ class PolicyWatcher {
policy::PolicyService* policy_service,
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
protected:
virtual void StartWatchingInternal() = 0;
virtual void StopWatchingInternal() = 0;
private:
friend class PolicyWatcherTest;
// Used to check if the class is on the right thread.
bool OnPolicyWatcherThread() const;
bool OnPolicyServiceThread() const;
// Takes the policy dictionary from the OS specific store and extracts the
// relevant policies.
......@@ -114,14 +110,36 @@ class PolicyWatcher {
// The counter is reset whenever policy has been successfully read.
void SignalTransientPolicyError();
friend class PolicyWatcherTest;
// |policy_service_task_runner| is the task runner where it is safe
// 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,
scoped_ptr<policy::SchemaRegistry> owned_schema_registry);
// Returns a DictionaryValue containing the default values for each policy.
const base::DictionaryValue& Defaults() const;
// Creates PolicyWatcher that wraps the owned |async_policy_loader| with an
// appropriate PolicySchema.
//
// |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);
private:
void StopWatchingOnPolicyWatcherThread();
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
// PolicyService::Observer interface.
void OnPolicyUpdated(const policy::PolicyNamespace& ns,
const policy::PolicyMap& previous,
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_;
......@@ -129,10 +147,21 @@ class PolicyWatcher {
scoped_ptr<base::DictionaryValue> old_policies_;
scoped_ptr<base::DictionaryValue> default_values_;
// TODO(lukasza): Remove - components/policy filters out mistyped values.
scoped_ptr<base::DictionaryValue> bad_type_values_;
// Allows us to cancel any inflight FileWatcher events or scheduled reloads.
base::WeakPtrFactory<PolicyWatcher> weak_factory_;
policy::PolicyService* policy_service_;
// Order of fields below is important to ensure destruction takes object
// dependencies into account:
// - |owned_policy_service_| uses |owned_policy_provider_|
// - |owned_policy_provider_| uses |owned_schema_registry_|
scoped_ptr<policy::SchemaRegistry> owned_schema_registry_;
scoped_ptr<policy::ConfigurationPolicyProvider> owned_policy_provider_;
scoped_ptr<policy::PolicyService> owned_policy_service_;
DISALLOW_COPY_AND_ASSIGN(PolicyWatcher);
};
} // namespace policy_hack
......
remoting/host/policy_hack/policy_watcher_unittest.cc
View file @ a9376cda
......@@ -12,7 +12,6 @@
#include "policy/policy_constants.h"
#include "remoting/host/dns_blackhole_checker.h"
#include "remoting/host/policy_hack/mock_policy_callback.h"
#include "remoting/host/policy_hack/policy_service_watcher.h"
#include "remoting/host/policy_hack/policy_watcher.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -35,7 +34,7 @@ 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_ = PolicyServiceWatcher::CreateFromPolicyLoader(
policy_watcher_ = PolicyWatcher::CreateFromPolicyLoader(
message_loop_proxy_, make_scoped_ptr(policy_loader_));
nat_true_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal, true);
......
remoting/remoting_host_srcs.gypi
View file @ a9376cda
......@@ -189,8 +189,6 @@
'host/pin_hash.h',
'host/policy_hack/policy_watcher.cc',
'host/policy_hack/policy_watcher.h',
'host/policy_hack/policy_service_watcher.cc',
'host/policy_hack/policy_service_watcher.h',
'host/register_support_host_request.cc',
'host/register_support_host_request.h',
'host/remote_input_filter.cc',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment