Added a PolicyManifestHandler.

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 3dfbf7e0b26bf4f0ac9ae5ce186927a8527a5db8..325012b9681f586e003f0f5a95e433b94c672c7c 100644
--- a/chrome/browser/extensions/api/storage/managed_value_store_cache.cc
+++ b/chrome/browser/extensions/api/storage/managed_value_store_cache.cc
@@ -26,7 +26,9 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/value_store/value_store_change.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_manifest_constants.h"
 #include "chrome/common/extensions/extension_set.h"
+#include "chrome/common/extensions/manifest.h"
 #include "chrome/common/extensions/permissions/api_permission.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
@@ -53,16 +55,25 @@ class ManagedValueStoreCache::ExtensionTracker
                        const content::NotificationDetails& details) OVERRIDE;
 
  private:
+  // Loads the schemas of the |extensions| and passes a PolicyDomainDescriptor
+  // to RegisterDomain().
+  static void LoadSchemas(scoped_ptr<ExtensionSet> extensions,
+                          base::WeakPtr<ExtensionTracker> self);
+  void RegisterDomain(
+      scoped_refptr<const policy::PolicyDomainDescriptor> descriptor);
+
   Profile* profile_;
   bool is_ready_;
   content::NotificationRegistrar registrar_;
+  base::WeakPtrFactory<ExtensionTracker> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionTracker);
 };
 
 ManagedValueStoreCache::ExtensionTracker::ExtensionTracker(Profile* profile)
     : profile_(profile),
-      is_ready_(false) {
+      is_ready_(false),
+      weak_factory_(this) {
   registrar_.Add(this,
                  chrome::NOTIFICATION_EXTENSIONS_READY,
                  content::Source<Profile>(profile_));
@@ -84,20 +95,77 @@ void ManagedValueStoreCache::ExtensionTracker::Observe(
   if (!is_ready_)
     return;
 
-  // TODO(joaodasilva): this currently only registers extensions that use
-  // the storage API, but that still includes a lot of extensions that don't
-  // use the storage.managed namespace. Use the presence of a schema for the
-  // managed namespace in the manifest as a better signal, once available.
   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) {

[Mattias Nissler (ping if slow), 2013/05/15 10:37:39]

It seems this loop is not really needed, you could just pass the entire
extension set to the file thread and do filtering there.

[not at google - send to devlin, 2013/05/16 16:34:15]

One advantage of filtering here is that the method can early-exit if there are
no extensions that have managed storage, and not make extra work for the message
loops. That said - this isn't being done, maybe it should be. And there are less
expensive ways of implementing that than creating a whole extra ExtensionSet.

[Joao da Silva, 2013/05/19 13:21:35]

It's expected that most extensions won't have the schema, so passing only those
that do spares some work.

In any case a copy must be created here, because ExtensionService::extensions()
is UI only.

[Joao da Silva, 2013/05/19 13:21:35]

|managed_extensions| could be created only when the first match is found. But
LoadSchemas() must be invoked even when no match is found, to possibly remove
entries for extensions that have been unloaded. I'd rather have a single path
Observe->LoadSchemas->RegisterDomain than a special-case
Observer->RegisterDomain for the no-matches case.

-    // TODO(joaodasilva): pass the parsed schema here, once available.
+    if ((*it)->manifest()->HasPath(
+            extension_manifest_keys::kStorageManagedSchema)) {
+      managed_extensions->Insert(*it);
+    }
+
+    // TODO(joaodasilva): also load extensions that use the storage API for now,
+    // to support the Legacy Browser Support extension. Remove this for M30.
+    // http://crbug.com/240704
     if ((*it)->HasAPIPermission(APIPermission::kStorage))
-      descriptor->AddComponent((*it)->id(), scoped_ptr<policy::PolicySchema>());
+      managed_extensions->Insert(*it);
   }
 
+  // The 2nd check needs to be performed on the FILE thread.

[Mattias Nissler (ping if slow), 2013/05/15 10:37:39]

What is the 2nd check? Just spell it out :)

[Joao da Silva, 2013/05/19 13:21:35]

Done.

+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ExtensionTracker::LoadSchemas,
+                 base::Passed(&managed_extensions),
+                 weak_factory_.GetWeakPtr()));
+}
+
+// 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);
+
+  for (ExtensionSet::const_iterator it = extensions->begin();
+       it != extensions->end(); ++it) {
+    std::string path;
+    if (!(*it)->manifest()->GetString(
+            extension_manifest_keys::kStorageManagedSchema, &path)) {
+      // TODO(joaodasilva): Remove this for M30. http://crbug.com/240704
+      if ((*it)->HasAPIPermission(APIPermission::kStorage)) {
+        descriptor->AddComponent((*it)->id(),
+                                 scoped_ptr<policy::PolicySchema>());
+      } else {
+        NOTREACHED();
+      }
+      continue;
+    }
+    // The extension should have been validated, so assume the schema exists
+    // and is valid.
+    std::string content;
+    if (!file_util::ReadFileToString((*it)->path().AppendASCII(path),

[Mattias Nissler (ping if slow), 2013/05/15 10:37:39]

I was momentarily confused by the two |path|s here. Maybe rename the local to
schema_file?

[Joao da Silva, 2013/05/19 13:21:35]

Done.

+                                     &content)) {
+      NOTREACHED();
+      continue;
+    }
+    std::string error;
+    scoped_ptr<policy::PolicySchema> schema =
+        policy::PolicySchema::Parse(content, &error);
+    if (schema)
+      descriptor->AddComponent((*it)->id(), schema.Pass());
+    else
+      NOTREACHED();

[Mattias Nissler (ping if slow), 2013/05/15 10:37:39]

Is this really a NOTREACHED()? Won't we hit this if I put an invalid manifest
into an extension?

[not at google - send to devlin, 2013/05/16 16:34:15]

Hopefully we are doing the schema validation at extension install time, but I
haven't reached that part of the review yet.

[Joao da Silva, 2013/05/19 13:21:35]

As Benjamin suspects, this is a NOTREACHED because the extension shouldn't have
validated otherwise.

+  }
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ExtensionTracker::RegisterDomain, self, descriptor));
+}
+
+void ManagedValueStoreCache::ExtensionTracker::RegisterDomain(
+    scoped_refptr<const policy::PolicyDomainDescriptor> descriptor) {
   policy::ProfilePolicyConnector* connector =
       policy::ProfilePolicyConnectorFactory::GetForProfile(profile_);
   connector->policy_service()->RegisterPolicyDomain(descriptor);