Migrate existing tests to new NetworkConfigurationUpdaterImpl.

- Migrate old unit tests of NetworkConfigurationUpdaterImplCros to new NetworkConfigurationUpdaterImpl.

I combined this migration with the following steps to simplify the process:
- Replace username hash in NetworkConfigurationUpdater by User* (was suggested earlier already and simplifies the unit test)
- Add a pure interface to ManagedNetworkConfigurationHandler and a Mock implementation.
- Remove unused NetworkConfigurationUpdaterImplCros

BUG=273102
For API changes:
TBR=dbeam@chromium.org, eroman@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218347 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/login/login_utils_browsertest.cc

@@ -271,16 +271,18 @@ class LoginUtilsTest : public testing::Test,
     test_cros_settings_.reset(new ScopedTestCrosSettings);
     test_user_manager_.reset(new ScopedTestUserManager);
 
+    // IOThread creates ProxyConfigServiceImpl and
+    // BrowserPolicyConnector::Init() creates a NetworkConfigurationUpdater,
+    // which both access NetworkHandler. Thus initialize it here before creating
+    // IOThread and before calling BrowserPolicyConnector::Init().
+    NetworkHandler::Initialize();
+
     browser_process_->SetProfileManager(
         new ProfileManagerWithoutInit(scoped_temp_dir_.path()));
     connector_ = browser_process_->browser_policy_connector();
     connector_->Init(local_state_.Get(),
                      browser_process_->system_request_context());
 
-    // IOThread creates ProxyConfigServiceImpl which in turn needs
-    // NetworkHandler. Thus initialize it here before creating IOThread.
-    NetworkHandler::Initialize();
-
     io_thread_state_.reset(new IOThread(local_state_.Get(),
                                         browser_process_->policy_service(),
                                         NULL, NULL));

chrome/browser/chromeos/net/onc_utils.cc

@@ -9,7 +9,6 @@
 #include "base/logging.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/login/user.h"
-#include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/chromeos/ui_proxy_config.h"
 #include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chromeos/network/network_configuration_handler.h"
@@ -164,23 +163,11 @@ class UserStringSubstitution : public chromeos::onc::StringSubstitution {
   DISALLOW_COPY_AND_ASSIGN(UserStringSubstitution);
 };
 
-const chromeos::User* GetLoggedInUserByHash(const std::string& userhash) {
-  const chromeos::UserList& users =
-      chromeos::UserManager::Get()->GetLoggedInUsers();
-  for (chromeos::UserList::const_iterator it = users.begin(); it != users.end();
-       ++it) {
-    if ((*it)->username_hash() == userhash)
-      return *it;
-  }
-  return NULL;
-}
-
 }  // namespace
 
 void ExpandStringPlaceholdersInNetworksForUser(
-    const std::string& hashed_username,
+    const chromeos::User* user,
     base::ListValue* network_configs) {
-  const chromeos::User* user = GetLoggedInUserByHash(hashed_username);
   if (!user) {
     // In tests no user may be logged in. It's not harmful if we just don't
     // expand the strings.
@@ -190,18 +177,17 @@ void ExpandStringPlaceholdersInNetworksForUser(
   chromeos::onc::ExpandStringsInNetworks(substitution, network_configs);
 }
 
-void ImportNetworksForUser(const std::string& hashed_username,
+void ImportNetworksForUser(const chromeos::User* user,
                            const base::ListValue& network_configs,
                            std::string* error) {
   error->clear();
 
   scoped_ptr<base::ListValue> expanded_networks(network_configs.DeepCopy());
-  ExpandStringPlaceholdersInNetworksForUser(hashed_username,
-                                            expanded_networks.get());
+  ExpandStringPlaceholdersInNetworksForUser(user, expanded_networks.get());
 
   const NetworkProfile* profile =
       NetworkHandler::Get()->network_profile_handler()->GetProfileForUserhash(
-          hashed_username);
+          user->username_hash());
   if (!profile) {
     *error = "User profile doesn't exist.";
     return;

chrome/browser/chromeos/net/onc_utils.h

@@ -16,6 +16,9 @@ class ListValue;
 }
 
 namespace chromeos {
+
+class User;
+
 namespace onc {
 
 // Translates |onc_proxy_settings|, which has to be a valid ONC ProxySettings
@@ -32,10 +35,10 @@ scoped_ptr<base::DictionaryValue> ConvertOncProxySettingsToProxyConfig(
 // implemented, which are replaced by attributes of the logged-in user with
 // |hashed_username|.
 void ExpandStringPlaceholdersInNetworksForUser(
-    const std::string& hashed_username,
+    const chromeos::User* user,
     base::ListValue* network_configs);
 
-void ImportNetworksForUser(const std::string& hashed_username,
+void ImportNetworksForUser(const chromeos::User* user,
                            const base::ListValue& network_configs,
                            std::string* error);
 

chrome/browser/chromeos/policy/network_configuration_updater.h

@@ -9,6 +9,10 @@
 #include "base/memory/scoped_ptr.h"
 #include "net/cert/x509_certificate.h"
 
+namespace chromeos {
+class User;
+}
+
 namespace net {
 class CertTrustAnchorProvider;
 }
@@ -30,10 +34,10 @@ class NetworkConfigurationUpdater {
   // not applied. This function may trigger immediate policy applications.  Web
   // trust isn't given to certificates imported from ONC by default. Setting
   // |allow_trust_certs_from_policy| to true allows giving Web trust to the
-  // certificates that request it. The pointer |user_policy_service| is
-  // stored until UnsetUserPolicyService is called.
+  // certificates that request it. References to |user_policy_service| and
+  // |user| are stored until UnsetUserPolicyService() is called.
   virtual void SetUserPolicyService(bool allow_trusted_certs_from_policy,
-                                    const std::string& hashed_username,
+                                    const chromeos::User* user,
                                     PolicyService* user_policy_service) = 0;
 
   // Unregisters from the PolicyService previously provided by

chrome/browser/chromeos/policy/network_configuration_updater_impl.cc

@@ -10,6 +10,7 @@
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/values.h"
+#include "chrome/browser/chromeos/login/user.h"
 #include "chrome/browser/chromeos/net/onc_utils.h"
 #include "chrome/browser/policy/policy_map.h"
 #include "chromeos/network/managed_network_configuration_handler.h"
@@ -22,12 +23,15 @@ namespace policy {
 
 NetworkConfigurationUpdaterImpl::NetworkConfigurationUpdaterImpl(
     PolicyService* device_policy_service,
+    chromeos::ManagedNetworkConfigurationHandler* network_config_handler,
     scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer)
     : device_policy_change_registrar_(device_policy_service,
                                       PolicyNamespace(POLICY_DOMAIN_CHROME,
                                                       std::string())),
       user_policy_service_(NULL),
       device_policy_service_(device_policy_service),
+      user_(NULL),
+      network_config_handler_(network_config_handler),
       certificate_importer_(certificate_importer.Pass()) {
   device_policy_change_registrar_.Observe(
       key::kDeviceOpenNetworkConfiguration,
@@ -52,11 +56,11 @@ NetworkConfigurationUpdaterImpl::~NetworkConfigurationUpdaterImpl() {
 
 void NetworkConfigurationUpdaterImpl::SetUserPolicyService(
     bool allow_trusted_certs_from_policy,
-    const std::string& hashed_username,
+    const chromeos::User* user,
     PolicyService* user_policy_service) {
   VLOG(1) << "Got user policy service.";
   user_policy_service_ = user_policy_service;
-  hashed_username_ = hashed_username;
+  user_ = user;
   if (allow_trusted_certs_from_policy)
     SetAllowTrustedCertsFromPolicy();
 
@@ -160,14 +164,13 @@ void NetworkConfigurationUpdaterImpl::ApplyNetworkConfiguration(
       certificates, onc_source, web_trust_certs.get());
 
   std::string userhash;
-  if (onc_source == chromeos::onc::ONC_SOURCE_USER_POLICY) {
-    userhash = hashed_username_;
-    chromeos::onc::ExpandStringPlaceholdersInNetworksForUser(hashed_username_,
+  if (onc_source == chromeos::onc::ONC_SOURCE_USER_POLICY && user_) {
+    userhash = user_->username_hash();
+    chromeos::onc::ExpandStringPlaceholdersInNetworksForUser(user_,
                                                              &network_configs);
   }
 
-  chromeos::NetworkHandler::Get()->managed_network_configuration_handler()->
-      SetPolicy(onc_source, userhash, network_configs);
+  network_config_handler_->SetPolicy(onc_source, userhash, network_configs);
 
   if (onc_source == chromeos::onc::ONC_SOURCE_USER_POLICY)
     SetTrustAnchors(web_trust_certs.Pass());

chrome/browser/chromeos/policy/network_configuration_updater_impl.h

@@ -14,6 +14,8 @@ class Value;
 }
 
 namespace chromeos {
+class ManagedNetworkConfigurationHandler;
+
 namespace onc {
 class CertificateImporter;
 }
@@ -31,13 +33,14 @@ class NetworkConfigurationUpdaterImpl : public NetworkConfigurationUpdater,
  public:
   NetworkConfigurationUpdaterImpl(
       PolicyService* device_policy_service,
+      chromeos::ManagedNetworkConfigurationHandler* network_config_handler,
       scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer);
   virtual ~NetworkConfigurationUpdaterImpl();
 
   // NetworkConfigurationUpdater overrides.
   virtual void SetUserPolicyService(
       bool allow_trusted_certs_from_policy,
-      const std::string& hashed_username,
+      const chromeos::User* user,
       PolicyService* user_policy_service) OVERRIDE;
 
   virtual void UnsetUserPolicyService() OVERRIDE;
@@ -68,8 +71,12 @@ class NetworkConfigurationUpdaterImpl : public NetworkConfigurationUpdater,
    // Used to retrieve device policies.
    PolicyService* device_policy_service_;
 
-   // User hash of the user that the user policy applies to.
-   std::string hashed_username_;
+   // The user for whom the user policy will be applied. The User object must be
+   // valid until UnsetUserPolicyService() is called.
+   const chromeos::User* user_;
+
+   // Pointer to the global singleton or a test instance.
+   chromeos::ManagedNetworkConfigurationHandler* network_config_handler_;
 
    scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer_;