r137470 - trunk/src/chrome/browser/policy

0 views
Skip to first unread message

joaod...@chromium.org

unread,
May 16, 2012, 2:56:23 PM5/16/12
to chromium...@chromium.org
Author: joaod...@chromium.org
Date: Wed May 16 11:56:23 2012
New Revision: 137470

Log:
Revert "Refactored NetworkConfigurationUpdater to read policy from the PolicyService."

This reverts commit e9c5d5d43ceb19a46f8a76d1bd7ae5aaa669c1c5.

TBR=fisc...@chromium.org
BUG=None
TEST=vmtests go green

Review URL: https://chromiumcodereview.appspot.com/10386176

Modified:
trunk/src/chrome/browser/policy/browser_policy_connector.cc
trunk/src/chrome/browser/policy/network_configuration_updater.cc
trunk/src/chrome/browser/policy/network_configuration_updater.h
trunk/src/chrome/browser/policy/network_configuration_updater_unittest.cc

Modified: trunk/src/chrome/browser/policy/browser_policy_connector.cc
==============================================================================
--- trunk/src/chrome/browser/policy/browser_policy_connector.cc (original)
+++ trunk/src/chrome/browser/policy/browser_policy_connector.cc Wed May 16 11:56:23 2012
@@ -144,7 +144,7 @@
if (command_line->HasSwitch(switches::kEnableONCPolicy)) {
network_configuration_updater_.reset(
new NetworkConfigurationUpdater(
- g_browser_process->policy_service(),
+ managed_cloud_provider_.get(),
chromeos::CrosLibrary::Get()->GetNetworkLibrary()));
}


Modified: trunk/src/chrome/browser/policy/network_configuration_updater.cc
==============================================================================
--- trunk/src/chrome/browser/policy/network_configuration_updater.cc (original)
+++ trunk/src/chrome/browser/policy/network_configuration_updater.cc Wed May 16 11:56:23 2012
@@ -4,10 +4,6 @@

#include "chrome/browser/policy/network_configuration_updater.h"

-#include <string>
-
-#include "base/bind.h"
-#include "base/bind_helpers.h"
#include "chrome/browser/chromeos/cros/network_library.h"
#include "chrome/browser/policy/policy_map.h"
#include "policy/policy_constants.h"
@@ -18,52 +14,44 @@
"{\"NetworkConfigurations\":[],\"Certificates\":[]}";

NetworkConfigurationUpdater::NetworkConfigurationUpdater(
- PolicyService* policy_service,
+ ConfigurationPolicyProvider* provider,
chromeos::NetworkLibrary* network_library)
- : policy_change_registrar_(
- policy_service, POLICY_DOMAIN_CHROME, std::string()),
- network_library_(network_library) {
+ : network_library_(network_library) {
DCHECK(network_library_);
- policy_change_registrar_.Observe(
- key::kDeviceOpenNetworkConfiguration,
- base::Bind(&NetworkConfigurationUpdater::ApplyNetworkConfiguration,
- base::Unretained(this),
- chromeos::NetworkUIData::ONC_SOURCE_DEVICE_POLICY,
- &device_network_config_));
- policy_change_registrar_.Observe(
- key::kOpenNetworkConfiguration,
- base::Bind(&NetworkConfigurationUpdater::ApplyNetworkConfiguration,
- base::Unretained(this),
- chromeos::NetworkUIData::ONC_SOURCE_USER_POLICY,
- &user_network_config_));
-
- // Apply the current values immediately.
- const PolicyMap& policies = policy_service->GetPolicies(POLICY_DOMAIN_CHROME,
- std::string());
- ApplyNetworkConfiguration(
- chromeos::NetworkUIData::ONC_SOURCE_DEVICE_POLICY,
- &device_network_config_,
- NULL,
- policies.GetValue(key::kDeviceOpenNetworkConfiguration));
- ApplyNetworkConfiguration(
- chromeos::NetworkUIData::ONC_SOURCE_USER_POLICY,
- &user_network_config_,
- NULL,
- policies.GetValue(key::kOpenNetworkConfiguration));
+ provider_registrar_.Init(provider, this);
+ Update();
}

NetworkConfigurationUpdater::~NetworkConfigurationUpdater() {}

+void NetworkConfigurationUpdater::OnUpdatePolicy(
+ ConfigurationPolicyProvider* provider) {
+ Update();
+}
+
+void NetworkConfigurationUpdater::Update() {
+ ConfigurationPolicyProvider* provider = provider_registrar_.provider();
+ const PolicyMap& policy = provider->policies().Get(POLICY_DOMAIN_CHROME, "");
+
+ ApplyNetworkConfiguration(policy, key::kDeviceOpenNetworkConfiguration,
+ chromeos::NetworkUIData::ONC_SOURCE_DEVICE_POLICY,
+ &device_network_config_);
+ ApplyNetworkConfiguration(policy, key::kOpenNetworkConfiguration,
+ chromeos::NetworkUIData::ONC_SOURCE_USER_POLICY,
+ &user_network_config_);
+}
+
void NetworkConfigurationUpdater::ApplyNetworkConfiguration(
+ const PolicyMap& policy_map,
+ const char* policy_name,
chromeos::NetworkUIData::ONCSource onc_source,
- std::string* cached_value,
- const base::Value* previous,
- const base::Value* current) {
+ std::string* cached_value) {
std::string new_network_config;
- if (current != NULL) {
+ const base::Value* value = policy_map.GetValue(policy_name);
+ if (value != NULL) {
// If the policy is not a string, we issue a warning, but still clear the
// network configuration.
- if (!current->GetAsString(&new_network_config))
+ if (!value->GetAsString(&new_network_config))
LOG(WARNING) << "Invalid network configuration.";
}


Modified: trunk/src/chrome/browser/policy/network_configuration_updater.h
==============================================================================
--- trunk/src/chrome/browser/policy/network_configuration_updater.h (original)
+++ trunk/src/chrome/browser/policy/network_configuration_updater.h Wed May 16 11:56:23 2012
@@ -9,11 +9,7 @@
#include <string>

#include "chrome/browser/chromeos/cros/network_ui_data.h"
-#include "chrome/browser/policy/policy_service.h"
-
-namespace base {
-class Value;
-}
+#include "chrome/browser/policy/configuration_policy_provider.h"

namespace chromeos {
class NetworkLibrary;
@@ -25,26 +21,33 @@

// Keeps track of the network configuration policy settings and updates the
// network definitions whenever the configuration changes.
-class NetworkConfigurationUpdater {
+class NetworkConfigurationUpdater
+ : public ConfigurationPolicyProvider::Observer {
public:
- NetworkConfigurationUpdater(PolicyService* policy_service,
+ NetworkConfigurationUpdater(ConfigurationPolicyProvider* provider,
chromeos::NetworkLibrary* network_library);
virtual ~NetworkConfigurationUpdater();

+ // ConfigurationPolicyProvider::Observer:
+ virtual void OnUpdatePolicy(ConfigurationPolicyProvider* provider) OVERRIDE;
+
// Empty network configuration blob.
static const char kEmptyConfiguration[];

private:
+ // Grabs network configuration from policy and applies it.
+ void Update();
+
// Extracts ONC string from |policy_map| and pushes the configuration to
// |network_library_| if it's different from |*cached_value| (which is
// updated).
- void ApplyNetworkConfiguration(chromeos::NetworkUIData::ONCSource onc_source,
- std::string* cached_value,
- const base::Value* previous,
- const base::Value* current);
+ void ApplyNetworkConfiguration(const PolicyMap& policy_map,
+ const char* policy_name,
+ chromeos::NetworkUIData::ONCSource onc_source,
+ std::string* cached_value);

- // Wraps the policy service we read network configuration from.
- PolicyChangeRegistrar policy_change_registrar_;
+ // Wraps the provider we read network configuration from.
+ ConfigurationPolicyObserverRegistrar provider_registrar_;

// Network library to write network configuration to.
chromeos::NetworkLibrary* network_library_;

Modified: trunk/src/chrome/browser/policy/network_configuration_updater_unittest.cc
==============================================================================
--- trunk/src/chrome/browser/policy/network_configuration_updater_unittest.cc (original)
+++ trunk/src/chrome/browser/policy/network_configuration_updater_unittest.cc Wed May 16 11:56:23 2012
@@ -4,11 +4,9 @@

#include "chrome/browser/policy/network_configuration_updater.h"

-#include "base/memory/scoped_ptr.h"
#include "chrome/browser/chromeos/cros/mock_network_library.h"
#include "chrome/browser/policy/mock_configuration_policy_provider.h"
#include "chrome/browser/policy/policy_map.h"
-#include "chrome/browser/policy/policy_service_impl.h"
#include "policy/policy_constants.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -27,9 +25,6 @@
virtual void SetUp() OVERRIDE {
EXPECT_CALL(network_library_, LoadOncNetworks(_, "", _, _))
.WillRepeatedly(Return(true));
- PolicyServiceImpl::Providers providers;
- providers.push_back(&provider_);
- policy_service_.reset(new PolicyServiceImpl(providers));
}

// Maps configuration policy name to corresponding ONC source.
@@ -44,7 +39,6 @@

chromeos::MockNetworkLibrary network_library_;
MockConfigurationPolicyProvider provider_;
- scoped_ptr<PolicyServiceImpl> policy_service_;
};

TEST_P(NetworkConfigurationUpdaterTest, InitialUpdate) {
@@ -57,12 +51,12 @@
LoadOncNetworks(kFakeONC, "", NameToONCSource(GetParam()), _))
.WillOnce(Return(true));

- NetworkConfigurationUpdater updater(policy_service_.get(), &network_library_);
+ NetworkConfigurationUpdater updater(&provider_, &network_library_);
Mock::VerifyAndClearExpectations(&network_library_);
}

TEST_P(NetworkConfigurationUpdaterTest, PolicyChange) {
- NetworkConfigurationUpdater updater(policy_service_.get(), &network_library_);
+ NetworkConfigurationUpdater updater(&provider_, &network_library_);

// We should update if policy changes.
EXPECT_CALL(network_library_,
Reply all
Reply to author
Forward
0 new messages