Add policy domain for signin screen apps

chrome/browser/extensions/api/storage/managed_value_store_cache.cc

Index: chrome/browser/extensions/api/storage/managed_value_store_cache.cc
diff --git a/chrome/browser/extensions/api/storage/managed_value_store_cache.cc b/chrome/browser/extensions/api/storage/managed_value_store_cache.cc
index 46e3228024ba118e4818ef786d492e5da18e390e..465ddbabced6eb7e5d5d98351270743c9aafdf4d 100644
--- a/chrome/browser/extensions/api/storage/managed_value_store_cache.cc
+++ b/chrome/browser/extensions/api/storage/managed_value_store_cache.cc
@@ -21,7 +21,6 @@
 #include "chrome/browser/policy/schema_registry_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/storage/storage_schema_manifest_handler.h"
-#include "components/policy/core/common/policy_namespace.h"
 #include "components/policy/core/common/schema.h"
 #include "components/policy/core/common/schema_map.h"
 #include "components/policy/core/common/schema_registry.h"
@@ -67,7 +66,7 @@ const ValueStoreFactory::ModelType kManagedModelType =
 class ManagedValueStoreCache::ExtensionTracker
     : public ExtensionRegistryObserver {
  public:
-  explicit ExtensionTracker(Profile* profile);
+  ExtensionTracker(Profile* profile, policy::PolicyDomain policy_domain);
   ~ExtensionTracker() override {}
 
  private:
@@ -96,6 +95,7 @@ class ManagedValueStoreCache::ExtensionTracker
   void Register(const policy::ComponentMap* components);
 
   Profile* profile_;
+  policy::PolicyDomain policy_domain_;
   ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver>
       extension_registry_observer_;
   policy::SchemaRegistry* schema_registry_;
@@ -104,11 +104,15 @@ class ManagedValueStoreCache::ExtensionTracker
   DISALLOW_COPY_AND_ASSIGN(ExtensionTracker);
 };
 
-ManagedValueStoreCache::ExtensionTracker::ExtensionTracker(Profile* profile)
+ManagedValueStoreCache::ExtensionTracker::ExtensionTracker(
+    Profile* profile,
+    policy::PolicyDomain policy_domain)
     : profile_(profile),
+      policy_domain_(policy_domain),
       extension_registry_observer_(this),
-      schema_registry_(policy::SchemaRegistryServiceFactory::GetForContext(
-                           profile)->registry()),
+      schema_registry_(
+          policy::SchemaRegistryServiceFactory::GetForContext(profile)
+              ->registry()),
       weak_factory_(this) {
   extension_registry_observer_.Add(ExtensionRegistry::Get(profile_));
   // Load schemas when the extension system is ready. It might be ready now.
@@ -141,8 +145,8 @@ void ManagedValueStoreCache::ExtensionTracker::OnExtensionUninstalled(
   if (!ExtensionSystem::Get(profile_)->ready().is_signaled())
     return;
   if (extension && UsesManagedStorage(extension)) {
-    schema_registry_->UnregisterComponent(policy::PolicyNamespace(
-        policy::POLICY_DOMAIN_EXTENSIONS, extension->id()));
+    schema_registry_->UnregisterComponent(
+        policy::PolicyNamespace(policy_domain_, extension->id()));
   }
 }
 
@@ -214,16 +218,16 @@ void ManagedValueStoreCache::ExtensionTracker::LoadSchemasOnBlockingPool(
 void ManagedValueStoreCache::ExtensionTracker::Register(
     const policy::ComponentMap* components) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  schema_registry_->RegisterComponents(policy::POLICY_DOMAIN_EXTENSIONS,
-                                       *components);
+  schema_registry_->RegisterComponents(policy_domain_, *components);
 
   // The first SetReady() call is performed after the ExtensionSystem is ready,
   // even if there are no managed extensions. It will trigger a loading of the
   // initial policy for any managed extensions, and eventually the PolicyService
-  // will become ready for POLICY_DOMAIN_EXTENSIONS, and
+  // will become ready for policy for extensions, and
   // OnPolicyServiceInitialized() will be invoked.
   // Subsequent calls to SetReady() are ignored.
   schema_registry_->SetReady(policy::POLICY_DOMAIN_EXTENSIONS);

[Andrew T Wilson (Slow), 2016/10/21 13:56:30]

Why are we doing this instead of SetReady(policy_domain_) ?

[emaxx, 2016/10/21 14:33:10]

That's an unfortunate side effect of introducing a new policy domain.
The problem was that without marking _both_ extension-related domains as ready,
the whole schema registry would stay non-ready and therefore block the observers
in the policy stack:
https://cs.chromium.org/chromium/src/components/policy/core/common/schema_reg...

[emaxx, 2016/10/21 14:59:39]

I added a comment that should clarify this tricky place.

+  schema_registry_->SetReady(policy::POLICY_DOMAIN_SIGNIN_EXTENSIONS);
 }
 
 ManagedValueStoreCache::ManagedValueStoreCache(
@@ -231,6 +235,7 @@ ManagedValueStoreCache::ManagedValueStoreCache(
     const scoped_refptr<ValueStoreFactory>& factory,
     const scoped_refptr<SettingsObserverList>& observers)
     : profile_(Profile::FromBrowserContext(context)),
+      policy_domain_(policy::POLICY_DOMAIN_EXTENSIONS),

[Andrew T Wilson (Slow), 2016/10/21 13:56:30]

I guess the idea is that eventually the caller will specify this in the
constructor? Because otherwise keeping this in a member variable makes no sense.

[emaxx, 2016/10/21 14:33:10]

Yes, it was planned either to add a constructor argument or to introduce a new
static method and call it here (I currently chose the second approach).

       policy_service_(
           policy::ProfilePolicyConnectorFactory::GetForBrowserContext(context)
               ->policy_service()),
@@ -238,14 +243,12 @@ ManagedValueStoreCache::ManagedValueStoreCache(
       observers_(observers) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  policy_service_->AddObserver(policy::POLICY_DOMAIN_EXTENSIONS, this);
+  policy_service_->AddObserver(policy_domain_, this);
 
-  extension_tracker_.reset(new ExtensionTracker(profile_));
+  extension_tracker_.reset(new ExtensionTracker(profile_, policy_domain_));
 
-  if (policy_service_->IsInitializationComplete(
-          policy::POLICY_DOMAIN_EXTENSIONS)) {
-    OnPolicyServiceInitialized(policy::POLICY_DOMAIN_EXTENSIONS);
-  }
+  if (policy_service_->IsInitializationComplete(policy_domain_))
+    OnPolicyServiceInitialized(policy_domain_);
 }
 
 ManagedValueStoreCache::~ManagedValueStoreCache() {
@@ -256,7 +259,7 @@ ManagedValueStoreCache::~ManagedValueStoreCache() {
 
 void ManagedValueStoreCache::ShutdownOnUI() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  policy_service_->RemoveObserver(policy::POLICY_DOMAIN_EXTENSIONS, this);
+  policy_service_->RemoveObserver(policy_domain_, this);
   extension_tracker_.reset();
 }
 
@@ -283,23 +286,22 @@ void ManagedValueStoreCache::OnPolicyServiceInitialized(
     policy::PolicyDomain domain) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  if (domain != policy::POLICY_DOMAIN_EXTENSIONS)
+  if (domain != policy_domain_)
     return;
 
   // The PolicyService now has all the initial policies ready. Send policy
   // for all the managed extensions to their backing stores now.
   policy::SchemaRegistry* registry =
       policy::SchemaRegistryServiceFactory::GetForContext(profile_)->registry();
-  const policy::ComponentMap* map = registry->schema_map()->GetComponents(
-      policy::POLICY_DOMAIN_EXTENSIONS);
+  const policy::ComponentMap* map =
+      registry->schema_map()->GetComponents(policy_domain_);
   if (!map)
     return;
 
   const policy::PolicyMap empty_map;
   for (policy::ComponentMap::const_iterator it = map->begin();
        it != map->end(); ++it) {
-    const policy::PolicyNamespace ns(policy::POLICY_DOMAIN_EXTENSIONS,
-                                     it->first);
+    const policy::PolicyNamespace ns(policy_domain_, it->first);
     // If there is no policy for |ns| then this will clear the previous store,
     // if there is one.
     OnPolicyUpdated(ns, empty_map, policy_service_->GetPolicies(ns));
@@ -311,8 +313,7 @@ void ManagedValueStoreCache::OnPolicyUpdated(const policy::PolicyNamespace& ns,
                                              const policy::PolicyMap& current) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  if (!policy_service_->IsInitializationComplete(
-           policy::POLICY_DOMAIN_EXTENSIONS)) {
+  if (!policy_service_->IsInitializationComplete(policy_domain_)) {
     // OnPolicyUpdated is called whenever a policy changes, but it doesn't
     // mean that all the policy providers are ready; wait until we get the
     // final policy values before passing them to the store.