Move ownership of the ComponentCloudPolicyService to the broker.

The ComponentCloudPolicyService needs to work with the SchemaRegistry,
so it was initially owned by the DeviceLocalAccountPolicyProvider
(which always has a valid reference to the registry). However, this
means that the ComponentCloudPolicyService can't precache any policies
until the session is started.

To enable precaching at enrollment time and at the login screen, the
ComponentCloudPolicyService has been moved to the broker instead.
This allows some code simplifications too, but requires changing the
ownership of the SchemaRegistry for device local accounts.

This CL is just moving code around; a subsequent CL will trigger
the precaching.

TBR=zelidrag@chromium.org
BUG=224596

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

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

chrome/browser/chromeos/policy/device_local_account_browsertest.cc

@@ -421,10 +421,13 @@ class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest,
     EXPECT_EQ(chromeos::User::USER_TYPE_PUBLIC_ACCOUNT, user->GetType());
   }
 
-  base::FilePath GetCacheDirectoryForAccountID(const std::string& account_id) {
+  base::FilePath GetExtensionCacheDirectoryForAccountID(
+      const std::string& account_id) {
     base::FilePath extension_cache_root_dir;
-    PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
-                     &extension_cache_root_dir);
+    if (!PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_EXTENSIONS,
+                          &extension_cache_root_dir)) {
+      ADD_FAILURE();
+    }
     return extension_cache_root_dir.Append(
         base::HexEncode(account_id.c_str(), account_id.size()));
   }
@@ -432,7 +435,7 @@ class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest,
   base::FilePath GetCacheCRXFile(const std::string& account_id,
                                  const std::string& id,
                                  const std::string& version) {
-    return GetCacheDirectoryForAccountID(account_id)
+    return GetExtensionCacheDirectoryForAccountID(account_id)
         .Append(base::StringPrintf("%s-%s.crx", id.c_str(), version.c_str()));
   }
 
@@ -764,7 +767,7 @@ IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, ExtensionsCached) {
   // Pre-populate the device local account's extension cache with a hosted app
   // and an extension.
   EXPECT_TRUE(base::CreateDirectory(
-      GetCacheDirectoryForAccountID(kAccountId1)));
+      GetExtensionCacheDirectoryForAccountID(kAccountId1)));
   base::FilePath test_dir;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
   const base::FilePath cached_hosted_app =

chrome/browser/chromeos/policy/device_local_account_policy_provider.cc

@@ -5,19 +5,16 @@
 #include "chrome/browser/chromeos/policy/device_local_account_policy_provider.h"
 
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
 #include "chrome/browser/chromeos/policy/device_local_account_external_data_manager.h"
 #include "chromeos/dbus/power_policy_controller.h"
 #include "components/policy/core/common/cloud/cloud_policy_core.h"
 #include "components/policy/core/common/cloud/cloud_policy_service.h"
+#include "components/policy/core/common/cloud/component_cloud_policy_service.h"
 #include "components/policy/core/common/policy_bundle.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_namespace.h"
-#include "components/policy/core/common/policy_switches.h"
-#include "content/public/browser/browser_thread.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "policy/policy_constants.h"
 
 namespace policy {
@@ -98,18 +95,13 @@ DeviceLocalAccountPolicyProvider::Create(
   return provider.Pass();
 }
 
-void DeviceLocalAccountPolicyProvider::Init(SchemaRegistry* schema_registry) {
-  ConfigurationPolicyProvider::Init(schema_registry);
-  MaybeCreateComponentPolicyService();
-}
-
 bool DeviceLocalAccountPolicyProvider::IsInitializationComplete(
     PolicyDomain domain) const {
   if (domain == POLICY_DOMAIN_CHROME)
     return store_initialized_;
   if (ComponentCloudPolicyService::SupportsDomain(domain) &&
-      component_policy_service_) {
-    return component_policy_service_->is_initialized();
+      GetBroker() && GetBroker()->component_policy_service()) {
+    return GetBroker()->component_policy_service()->is_initialized();
   }
   return true;
 }
@@ -126,38 +118,18 @@ void DeviceLocalAccountPolicyProvider::RefreshPolicies() {
   }
 }
 
-void DeviceLocalAccountPolicyProvider::Shutdown() {
-  component_policy_service_.reset();
-  ConfigurationPolicyProvider::Shutdown();
-}
-
 void DeviceLocalAccountPolicyProvider::OnPolicyUpdated(
     const std::string& user_id) {
-  if (user_id == user_id_) {
-    MaybeCreateComponentPolicyService();
+  if (user_id == user_id_)
     UpdateFromBroker();
-  }
 }
 
 void DeviceLocalAccountPolicyProvider::OnDeviceLocalAccountsChanged() {
-  MaybeCreateComponentPolicyService();
   UpdateFromBroker();
 }
 
-void DeviceLocalAccountPolicyProvider::OnBrokerShutdown(
-    DeviceLocalAccountPolicyBroker* broker) {
-  if (broker->user_id() == user_id_) {
-    // The |component_policy_service_| relies on the broker's CloudPolicyCore,
-    // so destroy it if the broker is going away.
-    component_policy_service_.reset();
-  }
-}
-
-void DeviceLocalAccountPolicyProvider::OnComponentCloudPolicyUpdated() {
-  UpdateFromBroker();
-}
-
-DeviceLocalAccountPolicyBroker* DeviceLocalAccountPolicyProvider::GetBroker() {
+DeviceLocalAccountPolicyBroker* DeviceLocalAccountPolicyProvider::GetBroker()
+    const {
   return service_->GetBrokerForUser(user_id_);
 }
 
@@ -176,6 +148,9 @@ void DeviceLocalAccountPolicyProvider::UpdateFromBroker() {
       bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
           .CopyFrom(broker->core()->store()->policy_map());
       external_data_manager_ = broker->external_data_manager();
+
+      if (broker->component_policy_service())
+        bundle->MergeFrom(broker->component_policy_service()->policy());
     } else {
       // Wait for the refresh to finish.
       return;
@@ -187,9 +162,6 @@ void DeviceLocalAccountPolicyProvider::UpdateFromBroker() {
     bundle->CopyFrom(policies());
   }
 
-  if (component_policy_service_)
-    bundle->MergeFrom(component_policy_service_->policy());
-
   // Apply overrides.
   if (chrome_policy_overrides_) {
     PolicyMap& chrome_policy =
@@ -206,35 +178,4 @@ void DeviceLocalAccountPolicyProvider::UpdateFromBroker() {
   UpdatePolicy(bundle.Pass());
 }
 
-void DeviceLocalAccountPolicyProvider::MaybeCreateComponentPolicyService() {
-  if (component_policy_service_)
-    return;  // Already started.
-
-  if (CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDisableComponentCloudPolicy)) {
-    // Disabled via the command line.
-    return;
-  }
-
-  DeviceLocalAccountPolicyBroker* broker = GetBroker();
-  if (!broker || !schema_registry())
-    return;  // Missing broker or not initialized yet.
-
-  scoped_ptr<ResourceCache> resource_cache(
-      new ResourceCache(broker->GetComponentPolicyCachePath(),
-                        content::BrowserThread::GetMessageLoopProxyForThread(
-                            content::BrowserThread::FILE)));
-
-  component_policy_service_.reset(new ComponentCloudPolicyService(
-      this,
-      schema_registry(),
-      broker->core(),
-      resource_cache.Pass(),
-      service_->request_context(),
-      content::BrowserThread::GetMessageLoopProxyForThread(
-          content::BrowserThread::FILE),
-      content::BrowserThread::GetMessageLoopProxyForThread(
-          content::BrowserThread::IO)));
-}
-
 }  // namespace policy

chrome/browser/chromeos/policy/device_local_account_policy_provider.h

@@ -14,8 +14,6 @@
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/policy/device_local_account_external_data_manager.h"
 #include "chrome/browser/chromeos/policy/device_local_account_policy_service.h"
-#include "components/policy/core/common/cloud/component_cloud_policy_service.h"
-#include "components/policy/core/common/cloud/resource_cache.h"
 #include "components/policy/core/common/configuration_policy_provider.h"
 
 namespace policy {
@@ -26,11 +24,11 @@ class PolicyMap;
 // DeviceLocalAccountPolicyService. Note that this implementation keeps
 // functioning when the device-local account disappears from
 // DeviceLocalAccountPolicyService. The current policy will be kept in that case
-// and RefreshPolicies becomes a no-op.
+// and RefreshPolicies becomes a no-op. Policies for any installed extensions
+// will be kept as well in that case.
 class DeviceLocalAccountPolicyProvider
     : public ConfigurationPolicyProvider,
-      public DeviceLocalAccountPolicyService::Observer,
-      public ComponentCloudPolicyService::Delegate {
+      public DeviceLocalAccountPolicyService::Observer {
  public:
   DeviceLocalAccountPolicyProvider(
       const std::string& user_id,
@@ -46,23 +44,16 @@ class DeviceLocalAccountPolicyProvider
       DeviceLocalAccountPolicyService* service);
 
   // ConfigurationPolicyProvider:
-  virtual void Init(SchemaRegistry* registry) OVERRIDE;
   virtual bool IsInitializationComplete(PolicyDomain domain) const OVERRIDE;
   virtual void RefreshPolicies() OVERRIDE;
-  virtual void Shutdown() OVERRIDE;
 
   // DeviceLocalAccountPolicyService::Observer:
   virtual void OnPolicyUpdated(const std::string& user_id) OVERRIDE;
   virtual void OnDeviceLocalAccountsChanged() OVERRIDE;
-  virtual void OnBrokerShutdown(
-      DeviceLocalAccountPolicyBroker* broker) OVERRIDE;
-
-  // ComponentCloudPolicyService::Delegate:
-  virtual void OnComponentCloudPolicyUpdated() OVERRIDE;
 
  private:
   // Returns the broker for |user_id_| or NULL if not available.
-  DeviceLocalAccountPolicyBroker* GetBroker();
+  DeviceLocalAccountPolicyBroker* GetBroker() const;
 
   // Handles completion of policy refreshes and triggers the update callback.
   // |success| is true if the policy refresh was successful.
@@ -72,10 +63,6 @@ class DeviceLocalAccountPolicyProvider
   // policy from the broker if available or keeping the current policy.
   void UpdateFromBroker();
 
-  // Creates the |component_policy_service_| if it hasn't been created yet
-  // and all the dependencies are in place.
-  void MaybeCreateComponentPolicyService();
-
   const std::string user_id_;
   scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager_;
 
@@ -89,8 +76,6 @@ class DeviceLocalAccountPolicyProvider
   bool store_initialized_;
   bool waiting_for_policy_refresh_;
 
-  scoped_ptr<ComponentCloudPolicyService> component_policy_service_;
-
   base::WeakPtrFactory<DeviceLocalAccountPolicyProvider> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountPolicyProvider);

chrome/browser/chromeos/policy/device_local_account_policy_service.h

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
@@ -21,6 +22,8 @@
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "components/policy/core/common/cloud/cloud_policy_core.h"
 #include "components/policy/core/common/cloud/cloud_policy_store.h"
+#include "components/policy/core/common/cloud/component_cloud_policy_service.h"
+#include "components/policy/core/common/schema_registry.h"
 
 namespace base {
 class SequencedTaskRunner;
@@ -44,8 +47,12 @@ class DeviceManagementService;
 
 // The main switching central that downloads, caches, refreshes, etc. policy for
 // a single device-local account.
-class DeviceLocalAccountPolicyBroker {
+class DeviceLocalAccountPolicyBroker
+    : public CloudPolicyStore::Observer,
+      public ComponentCloudPolicyService::Delegate {
  public:
+  // |policy_update_callback| will be invoked to notify observers that the
+  // policy for |account| has been updated.
   // |task_runner| is the runner for policy refresh tasks.
   DeviceLocalAccountPolicyBroker(
       const DeviceLocalAccount& account,
@@ -53,8 +60,9 @@ class DeviceLocalAccountPolicyBroker {
       scoped_ptr<DeviceLocalAccountPolicyStore> store,
       scoped_refptr<DeviceLocalAccountExternalDataManager>
           external_data_manager,
+      const base::Closure& policy_updated_callback,
       const scoped_refptr<base::SequencedTaskRunner>& task_runner);
-  ~DeviceLocalAccountPolicyBroker();
+  virtual ~DeviceLocalAccountPolicyBroker();
 
   // Initialize the broker, loading its |store_|.
   void Initialize();
@@ -74,6 +82,12 @@ class DeviceLocalAccountPolicyBroker {
     return external_data_manager_;
   }
 
+  ComponentCloudPolicyService* component_policy_service() const {
+    return component_policy_service_.get();
+  }
+
+  SchemaRegistry* schema_registry() { return &schema_registry_; }
+
   // Fire up the cloud connection for fetching policy for the account from the
   // cloud if this is an enterprise-managed device.
   void ConnectIfPossible(
@@ -88,20 +102,28 @@ class DeviceLocalAccountPolicyBroker {
   // empty string if the policy is not present.
   std::string GetDisplayName() const;
 
-  // Returns a directory where component policy for this account can be cached.
-  // The DeviceLocalAccountPolicyService takes care of cleaning up caches of
-  // accounts that have been removed.
-  base::FilePath GetComponentPolicyCachePath() const;
+  // CloudPolicyStore::Observer:
+  virtual void OnStoreLoaded(CloudPolicyStore* store) OVERRIDE;
+  virtual void OnStoreError(CloudPolicyStore* store) OVERRIDE;
+
+  // ComponentCloudPolicyService::Delegate:
+  virtual void OnComponentCloudPolicyUpdated() OVERRIDE;
 
  private:
+  void CreateComponentCloudPolicyService(
+      const scoped_refptr<net::URLRequestContextGetter>& request_context);
+
   const std::string account_id_;
   const std::string user_id_;
   const base::FilePath component_policy_cache_path_;
+  SchemaRegistry schema_registry_;
   const scoped_ptr<DeviceLocalAccountPolicyStore> store_;
   scoped_refptr<DeviceLocalAccountExternalDataManager> external_data_manager_;
   scoped_refptr<chromeos::DeviceLocalAccountExternalPolicyLoader>
       extension_loader_;
   CloudPolicyCore core_;
+  scoped_ptr<ComponentCloudPolicyService> component_policy_service_;
+  base::Closure policy_update_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountPolicyBroker);
 };
@@ -110,7 +132,7 @@ class DeviceLocalAccountPolicyBroker {
 // The actual policy blobs are brokered by session_manager (to prevent file
 // manipulation), and we're making signature checks on the policy blobs to
 // ensure they're issued by the device owner.
-class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
+class DeviceLocalAccountPolicyService {
  public:
   // Interface for interested parties to observe policy changes.
   class Observer {
@@ -122,9 +144,6 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
 
     // The list of accounts has been updated.
     virtual void OnDeviceLocalAccountsChanged() = 0;
-
-    // The given |broker| is about to be destroyed.
-    virtual void OnBrokerShutdown(DeviceLocalAccountPolicyBroker* broker) {}
   };
 
   DeviceLocalAccountPolicyService(
@@ -153,15 +172,9 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
   // |user_id|.
   bool IsPolicyAvailableForUser(const std::string& user_id);
 
-  scoped_refptr<net::URLRequestContextGetter> request_context() const;
-
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  // CloudPolicyStore::Observer:
-  virtual void OnStoreLoaded(CloudPolicyStore* store) OVERRIDE;
-  virtual void OnStoreError(CloudPolicyStore* store) OVERRIDE;
-
  private:
   typedef std::map<std::string, DeviceLocalAccountPolicyBroker*>
       PolicyBrokerMap;
@@ -203,6 +216,9 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
   // Find the broker for a given |store|. Returns NULL if |store| is unknown.
   DeviceLocalAccountPolicyBroker* GetBrokerForStore(CloudPolicyStore* store);
 
+  // Notifies the |observers_| that the policy for |user_id| has changed.
+  void NotifyPolicyUpdated(const std::string& user_id);
+
   ObserverList<Observer, true> observers_;
 
   chromeos::SessionManagerClient* session_manager_client_;
@@ -220,12 +236,12 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
 
   // Orphaned extension caches are removed at startup. This tracks the status of
   // that process.
-  enum OrphanCacheDeletionState {
+  enum OrphanExtensionCacheDeletionState {
     NOT_STARTED,
     IN_PROGRESS,
     DONE,
   };
-  OrphanCacheDeletionState orphan_cache_deletion_state_;
+  OrphanExtensionCacheDeletionState orphan_extension_cache_deletion_state_;
 
   // Account IDs whose extension cache directories are busy, either because a
   // broker for the account has not shut down completely yet or because the
@@ -242,8 +258,8 @@ class DeviceLocalAccountPolicyService : public CloudPolicyStore::Observer {
   const scoped_ptr<chromeos::CrosSettings::ObserverSubscription>
       local_accounts_subscription_;
 
-  // Path to the directory that contains the cached policies for components
-  // for device local accounts.
+  // Path to the directory that contains the cached policy for components
+  // for device-local accounts.
   base::FilePath component_policy_cache_root_;
 
   base::WeakPtrFactory<DeviceLocalAccountPolicyService> weak_factory_;

chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc

@@ -405,7 +405,9 @@ TEST_F(DeviceLocalAccountPolicyServiceTest, FetchPolicy) {
                        device_policy_.policy_data().device_id(),
                        _))
       .WillOnce(SaveArg<6>(&request));
-  EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_));
+  // This will be called twice, because the ComponentCloudPolicyService will
+  // also become ready after flushing all the pending tasks.
+  EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_)).Times(2);
   broker->core()->client()->FetchPolicy();
   FlushDeviceSettings();
   Mock::VerifyAndClearExpectations(&service_observer_);
@@ -449,7 +451,9 @@ TEST_F(DeviceLocalAccountPolicyServiceTest, RefreshPolicy) {
       .WillOnce(mock_device_management_service_.SucceedJob(response));
   EXPECT_CALL(mock_device_management_service_, StartJob(_, _, _, _, _, _, _));
   EXPECT_CALL(*this, OnRefreshDone(true)).Times(1);
-  EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_));
+  // This will be called twice, because the ComponentCloudPolicyService will
+  // also become ready after flushing all the pending tasks.
+  EXPECT_CALL(service_observer_, OnPolicyUpdated(account_1_user_id_)).Times(2);
   broker->core()->service()->RefreshPolicy(
       base::Bind(&DeviceLocalAccountPolicyServiceTest::OnRefreshDone,
                  base::Unretained(this)));

chrome/browser/policy/schema_registry_service_factory.cc

@@ -11,8 +11,51 @@
 #include "components/policy/core/common/schema_registry.h"
 #include "content/public/browser/browser_context.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_platform_part_chromeos.h"
+#include "chrome/browser/chromeos/login/users/user.h"
+#include "chrome/browser/chromeos/login/users/user_manager.h"
+#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
+#include "chrome/browser/chromeos/policy/device_local_account_policy_service.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/profiles/profile.h"
+#endif
+
 namespace policy {
 
+#if defined(OS_CHROMEOS)
+namespace {
+
+DeviceLocalAccountPolicyBroker* GetBroker(content::BrowserContext* context) {
+  Profile* profile = Profile::FromBrowserContext(context);
+
+  if (chromeos::ProfileHelper::IsSigninProfile(profile))
+    return NULL;
+
+  if (!chromeos::UserManager::IsInitialized()) {
+    // Bail out in unit tests that don't have a UserManager.
+    return NULL;
+  }
+
+  chromeos::UserManager* user_manager = chromeos::UserManager::Get();
+  chromeos::User* user = user_manager->GetUserByProfile(profile);
+  if (!user)
+    return NULL;
+
+  BrowserPolicyConnectorChromeOS* connector =
+      g_browser_process->platform_part()->browser_policy_connector_chromeos();
+  DeviceLocalAccountPolicyService* service =
+      connector->GetDeviceLocalAccountPolicyService();
+  if (!service)
+    return NULL;
+
+  return service->GetBrokerForUser(user->email());
+}
+
+}  // namespace
+#endif  // OS_CHROMEOS
+
 // static
 SchemaRegistryServiceFactory* SchemaRegistryServiceFactory::GetInstance() {
   return Singleton<SchemaRegistryServiceFactory>::get();
@@ -60,7 +103,23 @@ SchemaRegistryServiceFactory::CreateForContextInternal(
     CombinedSchemaRegistry* global_registry) {
   DCHECK(!context->IsOffTheRecord());
   DCHECK(registries_.find(context) == registries_.end());
-  scoped_ptr<SchemaRegistry> registry(new SchemaRegistry);
+
+  scoped_ptr<SchemaRegistry> registry;
+
+#if defined(OS_CHROMEOS)
+  DeviceLocalAccountPolicyBroker* broker = GetBroker(context);
+  if (broker) {
+    // The SchemaRegistry for a device-local account is owned by its
+    // DeviceLocalAccountPolicyBroker, which uses the registry to fetch and
+    // cache policy even if there is no active session for that account.
+    // Use a ForwardingSchemaRegistry that wraps this SchemaRegistry.
+    registry.reset(new ForwardingSchemaRegistry(broker->schema_registry()));
+  }
+#endif
+
+  if (!registry)
+    registry.reset(new SchemaRegistry);
+
   scoped_ptr<SchemaRegistryService> service(new SchemaRegistryService(
       registry.Pass(), chrome_schema, global_registry));
   registries_[context] = service.get();

chrome/browser/policy/schema_registry_service_factory.h

@@ -7,8 +7,8 @@
 
 #include <map>
 
-#include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "components/keyed_service/content/browser_context_keyed_base_factory.h"

chrome/browser/policy/test/policy_testserver.py

@@ -443,10 +443,12 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
       self.server.UpdateStateKeys(token_info['device_token'],
                                   key_update_request.server_backed_state_key)
 
-    # If this is a publicaccount request then get the username now and use it
-    # in every PolicyFetchResponse produced. This is required to validate
-    # policy for extensions in public accounts.
-    username = self.server.GetPolicies().get('policy_user', None)
+    # If this is a |publicaccount| request, get the |username| now and use
+    # it in every PolicyFetchResponse produced. This is required to validate
+    # policy for extensions in device-local accounts.
+    # Unfortunately, the |username| can't be obtained from |msg| because that
+    # requires interacting with GAIA.
+    username = None
     for request in msg.policy_request.request:
       if request.policy_type == 'google/chromeos/publicaccount':
         username = request.settings_entity_id
@@ -461,7 +463,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
               'google/chrome/user',
               'google/ios/user')):
         fetch_response = response.policy_response.response.add()
-        self.ProcessCloudPolicy(request, token_info, fetch_response)
+        self.ProcessCloudPolicy(request, token_info, fetch_response, username)
       elif request.policy_type == 'google/chrome/extension':
         self.ProcessCloudPolicyForExtensions(
             request, response.policy_response, token_info, username)
@@ -637,7 +639,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
       settings.__getattribute__(field.name).CopyFrom(policy_message)
 
   def ProcessCloudPolicyForExtensions(self, request, response, token_info,
-                                      username):
+                                      username=None):
     """Handles a request for policy for extensions.
 
     A request for policy for extensions is slightly different from the other
@@ -649,7 +651,7 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
       response: The DevicePolicyResponse message for the response. Multiple
       PolicyFetchResponses will be appended to this message.
       token_info: The token extracted from the request.
-      username: The username for the response.
+      username: The username for the response. May be None.
     """
     # Send one PolicyFetchResponse for each extension that has
     # configuration data at the server.
@@ -760,8 +762,6 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     if username:
       policy_data.username = username
-    elif msg.policy_type == 'google/chromeos/publicaccount':
-      policy_data.username = msg.settings_entity_id
     else:
       # For regular user/device policy, there is no way for the testserver to
       # know the user name belonging to the GAIA auth token we received (short

chromeos/chromeos_paths.h

@@ -38,11 +38,11 @@ enum {
   DIR_DEVICE_LOCAL_ACCOUNT_EXTERNAL_DATA,  // Directory where external data
                                            // referenced by policies is cached
                                            // for device-local accounts.
-  DIR_DEVICE_LOCAL_ACCOUNT_COMPONENT_POLICY,  // Directory where policy for the
-                                              // components of device-local
-                                              // account is stored. Currently
-                                              // this is used for policy for
-                                              // extensions.
+  DIR_DEVICE_LOCAL_ACCOUNT_COMPONENT_POLICY,  // Directory where policy for
+                                              // components is stored for
+                                              // device-local accounts.
+                                              // Currently this is used for
+                                              // policy for extensions.
   PATH_END
 };