Policy providers all get a SchemaRegistry to work with.

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/storage/managed_value_store_cache.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/storage/policy_value_store.h"
 #include "chrome/browser/extensions/api/storage/settings_storage_factory.h"
 #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"
 #include "extensions/common/constants.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/permissions/api_permission.h"
 
 using content::BrowserThread;
 
@@ skipping 18 matching lines @@
  public:
   explicit ExtensionTracker(Profile* profile);
   virtual ~ExtensionTracker() {}
 
   // NotificationObserver implementation:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        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);
 };
 
 ManagedValueStoreCache::ExtensionTracker::ExtensionTracker(Profile* profile)
     : profile_(profile),
+      schema_registry_(
+          policy::SchemaRegistryServiceFactory::GetForContext(profile)),
       weak_factory_(this) {
   registrar_.Add(this,
                  chrome::NOTIFICATION_EXTENSIONS_READY,
                  content::Source<Profile>(profile_));
   registrar_.Add(this,
                  chrome::NOTIFICATION_EXTENSION_LOADED,
                  content::Source<Profile>(profile_));
   registrar_.Add(this,
                  chrome::NOTIFICATION_EXTENSION_UNLOADED,
                  content::Source<Profile>(profile_));
 }
 
 void ManagedValueStoreCache::ExtensionTracker::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   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) {
     std::string schema_file;
     if (!(*it)->manifest()->GetString(
             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();
       }
       continue;
     }
     // The extension should have been validated, so assume the schema exists
     // and is valid.
     std::string error;
     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(
     Profile* profile,
     const scoped_refptr<SettingsStorageFactory>& factory,
     const scoped_refptr<SettingsObserverList>& observers)
     : weak_factory_(this),
       weak_this_on_ui_(weak_factory_.GetWeakPtr()),
       profile_(profile),
       event_router_(ExtensionSystem::Get(profile)->event_router()),
       storage_factory_(factory),
       observers_(observers),
       base_path_(profile->GetPath().AppendASCII(
           extensions::kManagedSettingsDirectoryName)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // |event_router_| can be NULL on unit_tests.
   if (event_router_)
     event_router_->RegisterObserver(this, storage::OnChanged::kEventName);
 
   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() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DCHECK(!event_router_);
   // Delete the PolicyValueStores on FILE.
   store_map_.clear();
 }
 
 void ManagedValueStoreCache::ShutdownOnUI() {
@@ skipping 203 matching lines @@
     continuation.Run();
 }
 
 policy::PolicyService* ManagedValueStoreCache::GetPolicyService() {
   policy::ProfilePolicyConnector* connector =
       policy::ProfilePolicyConnectorFactory::GetForProfile(profile_);
   return connector->policy_service();
 }
 
 }  // namespace extensions