Added a PolicyManifestHandler.

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 <set>
 
 #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/extensions/api/storage/policy_value_store.h"
 #include "chrome/browser/extensions/api/storage/settings_storage_factory.h"
 #include "chrome/browser/extensions/event_names.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/policy_schema.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #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"
 #include "content/public/browser/notification_registrar.h"
 
 using content::BrowserThread;
 
 namespace extensions {
 
 // This helper observes initialization of all the installed extensions and
 // subsequent loads and unloads, and keeps the PolicyService of the Profile
 // in sync with the current list of extensions. This allows the PolicyService
 // to fetch cloud policy for those extensions, and allows its providers to
 // selectively load only extension policy that has users.
 class ManagedValueStoreCache::ExtensionTracker
     : public content::NotificationObserver {
  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().
+  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_));
   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 (type == chrome::NOTIFICATION_EXTENSIONS_READY)
     is_ready_ = true;
 
   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);
 }
 
 ManagedValueStoreCache::ManagedValueStoreCache(
     Profile* profile,
     const scoped_refptr<SettingsStorageFactory>& factory,
     const scoped_refptr<SettingsObserverList>& observers)
     : weak_factory_(this),
@@ skipping 228 matching lines @@
     continuation.Run();
 }
 
 policy::PolicyService* ManagedValueStoreCache::GetPolicyService() {
   policy::ProfilePolicyConnector* connector =
       policy::ProfilePolicyConnectorFactory::GetForProfile(profile_);
   return connector->policy_service();
 }
 
 }  // namespace extensions