Policy providers all get a SchemaRegistry to work with.

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 c1957f301b47c70a2a1b7e3e110d5be58ded9e3f..26ef353a2f97cbebfc5d3cbd125e24dec56628eb 100644
--- a/chrome/browser/extensions/api/storage/managed_value_store_cache.cc
+++ b/chrome/browser/extensions/api/storage/managed_value_store_cache.cc
@@ -17,17 +17,21 @@
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
-#include "chrome/browser/policy/policy_domain_descriptor.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
+#include "chrome/browser/policy/schema_registry.h"
+#include "chrome/browser/policy/schema_registry_service.h"
+#include "chrome/browser/policy/schema_registry_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/value_store/value_store_change.h"
 #include "chrome/common/extensions/api/storage.h"
 #include "chrome/common/extensions/api/storage/storage_schema_manifest_handler.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_set.h"
+#include "components/policy/core/common/policy_namespace.h"
 #include "components/policy/core/common/schema.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_source.h"
@@ -66,15 +70,17 @@ class ManagedValueStoreCache::ExtensionTracker
                        const content::NotificationDetails& details) OVERRIDE;
 
  private:
-  // Loads the schemas of the |extensions| and passes a PolicyDomainDescriptor
-  // to RegisterDomain().
+  bool IsEnterpriseExtension(const Extension* extension) const;
+
+  // Loads the schemas of the |extensions| and passes a ComponentMap to
+  // Register().
   static void LoadSchemas(scoped_ptr<ExtensionSet> extensions,
                           base::WeakPtr<ExtensionTracker> self);
-  void RegisterDomain(
-      scoped_refptr<const policy::PolicyDomainDescriptor> descriptor);
+  void Register(const policy::ComponentMap* components);
 
   Profile* profile_;
   content::NotificationRegistrar registrar_;
+  policy::SchemaRegistry* schema_registry_;
   base::WeakPtrFactory<ExtensionTracker> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionTracker);
@@ -82,6 +88,8 @@ class ManagedValueStoreCache::ExtensionTracker
 
 ManagedValueStoreCache::ExtensionTracker::ExtensionTracker(Profile* profile)
     : profile_(profile),
+      schema_registry_(
+          policy::SchemaRegistryServiceFactory::GetForContext(profile)),
       weak_factory_(this) {
   registrar_.Add(this,
                  chrome::NOTIFICATION_EXTENSIONS_READY,
@@ -101,37 +109,64 @@ void ManagedValueStoreCache::ExtensionTracker::Observe(
   if (!ExtensionSystem::Get(profile_)->ready().is_signaled())

[not at google - send to devlin, 2013/11/09 23:52:43]

(how can this be false if we're getting extension load and unload events? bleh)

[Joao da Silva, 2013/11/11 08:46:26]

Left a comment explaining why this is here:

all the extensions of the Profile are loaded on startup and each triggers a
NOTIFICATION_EXTENSION_LOADED before the ExtensionSystem becomes ready (which
triggers NOTIFICATION_EXTENSIONS_READY). This check prevents repeated policy
reloads for each extension loaded on startup, and coalesces those updates into
at most one.

     return;
 
-  scoped_refptr<policy::PolicyDomainDescriptor> descriptor(
-      new policy::PolicyDomainDescriptor(policy::POLICY_DOMAIN_EXTENSIONS));
-  const ExtensionSet* set =
-      ExtensionSystem::Get(profile_)->extension_service()->extensions();
-  scoped_ptr<ExtensionSet> managed_extensions(new ExtensionSet());
-  for (ExtensionSet::const_iterator it = set->begin(); it != set->end(); ++it) {
-    if ((*it)->manifest()->HasPath(manifest_keys::kStorageManagedSchema)) {
-      managed_extensions->Insert(*it);
+  if (type == chrome::NOTIFICATION_EXTENSION_UNLOADED) {

[not at google - send to devlin, 2013/11/09 23:52:43]

use switch. easier to read. I think you could make it look something like this
and it'd be a little less noisy-looking:

scoped_ptr<ExtensionSet> added(new ExtensionSet());
const Extension* removed = NULL;

switch (..) {
  case UNLOADED:
    removed = ..
    break
  case LOADED:
    added.add(...)
    break
   case READ:
    added.addall(...)
}

if (removed && IsEnterpriseExtension(removed)) {
  ...
  return;
}

for (set)
  ...
if (set)
  load

[Joao da Silva, 2013/11/11 08:46:26]

Done.

+    const Extension* extension =
+        content::Details<UnloadedExtensionInfo>(details)->extension;
+    if (IsEnterpriseExtension(extension)) {
+      schema_registry_->UnregisterComponent(policy::PolicyNamespace(
+          policy::POLICY_DOMAIN_EXTENSIONS, extension->id()));
     }
+    return;
+  }
 
-    // TODO(joaodasilva): also load extensions that use the storage API for now,
-    // to support the Legacy Browser Support extension. Remove this.
-    // http://crbug.com/240704
-    if ((*it)->HasAPIPermission(APIPermission::kStorage))
-      managed_extensions->Insert(*it);
+  scoped_ptr<ExtensionSet> set(new ExtensionSet);
+  if (type == chrome::NOTIFICATION_EXTENSION_LOADED) {
+    const Extension* extension =
+        content::Details<const Extension>(details).ptr();
+    if (IsEnterpriseExtension(extension))
+      set->Insert(extension);
+  } else if (type == chrome::NOTIFICATION_EXTENSIONS_READY) {
+    const ExtensionSet* extensions =
+        ExtensionSystem::Get(profile_)->extension_service()->extensions();
+    for (ExtensionSet::const_iterator it = extensions->begin();
+         it != extensions->end(); ++it) {
+      if (IsEnterpriseExtension(*it))
+        set->Insert(*it);
+    }
+  } else {
+    NOTREACHED();
   }
 
+  if (set->is_empty())
+    return;
+
   // Load the schema files in a background thread.
   BrowserThread::PostBlockingPoolSequencedTask(
       kLoadSchemasBackgroundTaskTokenName, FROM_HERE,
       base::Bind(&ExtensionTracker::LoadSchemas,
-                 base::Passed(&managed_extensions),
+                 base::Passed(&set),
                  weak_factory_.GetWeakPtr()));
 }
 
+bool ManagedValueStoreCache::ExtensionTracker::IsEnterpriseExtension(

[not at google - send to devlin, 2013/11/09 23:52:43]

"IsEnterpriseExtension" is a confusing name. It makes me think it's an extension
that was installed due to enterprise policy, which isn't true.

[Joao da Silva, 2013/11/11 08:46:26]

Renamed to UsesManagedStorage.

+    const Extension* extension) const {
+  if (extension->manifest()->HasPath(manifest_keys::kStorageManagedSchema))
+    return true;
+
+  // TODO(joaodasilva): also load extensions that use the storage API for now,
+  // to support the Legacy Browser Support extension. Remove this.
+  // http://crbug.com/240704
+  if (extension->HasAPIPermission(APIPermission::kStorage))
+    return true;
+
+  return false;
+}
+
 // static
 void ManagedValueStoreCache::ExtensionTracker::LoadSchemas(
     scoped_ptr<ExtensionSet> extensions,
     base::WeakPtr<ExtensionTracker> self) {
-  scoped_refptr<policy::PolicyDomainDescriptor> descriptor =
-      new policy::PolicyDomainDescriptor(policy::POLICY_DOMAIN_EXTENSIONS);
+  scoped_ptr<policy::ComponentMap> components(new policy::ComponentMap);
 
   for (ExtensionSet::const_iterator it = extensions->begin();
        it != extensions->end(); ++it) {
@@ -140,7 +175,7 @@ void ManagedValueStoreCache::ExtensionTracker::LoadSchemas(
             manifest_keys::kStorageManagedSchema, &schema_file)) {
       // TODO(joaodasilva): Remove this. http://crbug.com/240704
       if ((*it)->HasAPIPermission(APIPermission::kStorage)) {
-        descriptor->RegisterComponent((*it)->id(), policy::Schema());
+        (*components)[(*it)->id()] = policy::Schema();
       } else {
         NOTREACHED();
       }
@@ -152,19 +187,18 @@ void ManagedValueStoreCache::ExtensionTracker::LoadSchemas(
     policy::Schema schema =
         StorageSchemaManifestHandler::GetSchema(it->get(), &error);
     CHECK(schema.valid()) << error;
-    descriptor->RegisterComponent((*it)->id(), schema);
+    (*components)[(*it)->id()] = schema;
   }
 
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(&ExtensionTracker::RegisterDomain, self, descriptor));
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(&ExtensionTracker::Register, self,
+                                     base::Owned(components.release())));
 }
 
-void ManagedValueStoreCache::ExtensionTracker::RegisterDomain(
-    scoped_refptr<const policy::PolicyDomainDescriptor> descriptor) {
-  policy::ProfilePolicyConnector* connector =
-      policy::ProfilePolicyConnectorFactory::GetForProfile(profile_);
-  connector->policy_service()->RegisterPolicyDomain(descriptor);
+void ManagedValueStoreCache::ExtensionTracker::Register(
+    const policy::ComponentMap* components) {
+  schema_registry_->RegisterComponents(policy::POLICY_DOMAIN_EXTENSIONS,
+                                       *components);
 }
 
 ManagedValueStoreCache::ManagedValueStoreCache(
@@ -186,7 +220,10 @@ ManagedValueStoreCache::ManagedValueStoreCache(
 
   GetPolicyService()->AddObserver(policy::POLICY_DOMAIN_EXTENSIONS, this);
 
-  extension_tracker_.reset(new ExtensionTracker(profile_));
+  // Track the extensions of the original Profile only; the OTR profile has
+  // a subset of those.
+  if (!profile->IsOffTheRecord())
+    extension_tracker_.reset(new ExtensionTracker(profile_));

[not at google - send to devlin, 2013/11/09 23:52:43]

Are we even creating one of these with an OTR profile in the first place? Check
that. I would expect that this object is owned by an ExtensionService which is
shared between main/OTR profiles, so this line of code is moot.

If not, ideally it would be like that.

[Joao da Silva, 2013/11/11 08:46:26]

That's right, removed this test.

 }
 
 ManagedValueStoreCache::~ManagedValueStoreCache() {