Trigger chrome.storage.onChanged events for policy updates on the 'ma…

…naged' namespace. BUG=108992 TEST=ExtensionSettingsApiTest.ManagedStorageEvents Review URL: https://chromiumcodereview.appspot.com/10807086 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148350 0039d316-1c4b-4281-b951-d872f2087c98
qweri0p · Jul 25, 2012 · 9a30138 · 9a30138
1 parent 71d504f
commit 9a30138
Show file tree

Hide file tree

Showing 13 changed files with 176 additions and 37 deletions.
diff --git a/chrome/browser/extensions/settings/managed_value_store_cache.cc b/chrome/browser/extensions/settings/managed_value_store_cache.cc
@@ -4,11 +4,13 @@
 
 #include "chrome/browser/extensions/settings/managed_value_store_cache.h"
 
+#include <set>
+
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "base/sequenced_task_runner.h"
-#include "chrome/browser/policy/policy_service.h"
 #include "chrome/browser/value_store/policy_value_store.h"
+#include "chrome/browser/value_store/value_store_change.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -17,13 +19,22 @@ using content::BrowserThread;
 namespace extensions {
 
 ManagedValueStoreCache::ManagedValueStoreCache(
-    policy::PolicyService* policy_service)
-    : policy_service_(policy_service) {}
+    policy::PolicyService* policy_service,
+    scoped_refptr<SettingsObserverList> observers)
+    : policy_service_(policy_service),
+      observers_(observers) {
+  policy_service_->AddObserver(policy::POLICY_DOMAIN_EXTENSIONS, this);
+}
 
 ManagedValueStoreCache::~ManagedValueStoreCache() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
+void ManagedValueStoreCache::ShutdownOnUI() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  policy_service_->RemoveObserver(policy::POLICY_DOMAIN_EXTENSIONS, this);
+}
+
 scoped_refptr<base::MessageLoopProxy>
     ManagedValueStoreCache::GetMessageLoop() const {
   return BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI);
@@ -45,4 +56,37 @@ void ManagedValueStoreCache::DeleteStorageSoon(
   // There's no real storage backing this cache, so this is a nop.
 }
 
+void ManagedValueStoreCache::OnPolicyUpdated(policy::PolicyDomain domain,
+                                             const std::string& component_id,
+                                             const policy::PolicyMap& previous,
+                                             const policy::PolicyMap& current) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  ValueStoreChangeList changes;
+  std::set<std::string> changed_keys;
+  previous.GetDifferingKeys(current, &changed_keys);
+  for (std::set<std::string>::iterator it = changed_keys.begin();
+       it != changed_keys.end(); ++it) {
+    // A policy might've changed only its scope or level, while the value
+    // stayed the same. Events should be triggered only for mandatory values
+    // that have changed.
+    const policy::PolicyMap::Entry* old_entry = previous.Get(*it);
+    const policy::PolicyMap::Entry* new_entry = current.Get(*it);
+    scoped_ptr<base::Value> old_value;
+    scoped_ptr<base::Value> new_value;
+    if (old_entry && old_entry->level == policy::POLICY_LEVEL_MANDATORY)
+      old_value.reset(old_entry->value->DeepCopy());
+    if (new_entry && new_entry->level == policy::POLICY_LEVEL_MANDATORY)
+      new_value.reset(new_entry->value->DeepCopy());
+    if (!base::Value::Equals(old_value.get(), new_value.get())) {
+      changes.push_back(
+          ValueStoreChange(*it, old_value.release(), new_value.release()));
+    }
+  }
+  observers_->Notify(
+      &SettingsObserver::OnSettingsChanged,
+      component_id,
+      settings_namespace::MANAGED,
+      ValueStoreChange::ToJson(changes));
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/settings/managed_value_store_cache.h b/chrome/browser/extensions/settings/managed_value_store_cache.h
@@ -7,23 +7,26 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/extensions/settings/settings_observer.h"
 #include "chrome/browser/extensions/settings/value_store_cache.h"
-
-namespace policy {
-class PolicyService;
-}  // namespace policy
+#include "chrome/browser/policy/policy_service.h"
 
 namespace extensions {
 
 // Runs the StorageCallback with a read-only ValueStore that pulls values from
 // the PolicyService for the given extension.
-class ManagedValueStoreCache : public ValueStoreCache {
+class ManagedValueStoreCache : public ValueStoreCache,
+                               public policy::PolicyService::Observer {
  public:
-  explicit ManagedValueStoreCache(policy::PolicyService* policy_service);
+  ManagedValueStoreCache(policy::PolicyService* policy_service,
+                         scoped_refptr<SettingsObserverList> observers);
   virtual ~ManagedValueStoreCache();
 
   // ValueStoreCache implementation:
 
+  virtual void ShutdownOnUI() OVERRIDE;
+
   virtual scoped_refptr<base::MessageLoopProxy> GetMessageLoop() const OVERRIDE;
 
   virtual void RunWithValueStoreForExtension(
@@ -32,9 +35,18 @@ class ManagedValueStoreCache : public ValueStoreCache {
 
   virtual void DeleteStorageSoon(const std::string& extension_id) OVERRIDE;
 
+  // PolicyService::Observer implementation:
+
+  virtual void OnPolicyUpdated(policy::PolicyDomain domain,
+                               const std::string& component_id,
+                               const policy::PolicyMap& previous,
+                               const policy::PolicyMap& current) OVERRIDE;
+
  private:
   policy::PolicyService* policy_service_;
 
+  scoped_refptr<SettingsObserverList> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(ManagedValueStoreCache);
 };
 

diff --git a/chrome/browser/extensions/settings/settings_apitest.cc b/chrome/browser/extensions/settings/settings_apitest.cc
@@ -139,6 +139,17 @@ class ExtensionSettingsApiTest : public ExtensionApiTest {
               GetBackendForSync(kModelType));
   }
 
+#if defined(ENABLE_CONFIGURATION_POLICY)
+  void SetPolicies(const base::DictionaryValue& policies) {
+    scoped_ptr<policy::PolicyBundle> bundle(new policy::PolicyBundle());
+    policy::PolicyMap& policy_map = bundle->Get(
+        policy::POLICY_DOMAIN_EXTENSIONS, kManagedStorageExtensionId);
+    policy_map.LoadFrom(
+        &policies, policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER);
+    policy_provider_.UpdatePolicy(bundle.Pass());
+  }
+#endif
+
  private:
   const Extension* MaybeLoadAndReplyWhenSatisfied(
       Namespace settings_namespace,
@@ -429,18 +440,40 @@ IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, ManagedStorage) {
               .Append(extensions::DictionaryBuilder()
                   .Set("three", 3))))
       .Build();
-
-  scoped_ptr<policy::PolicyBundle> bundle(new policy::PolicyBundle());
-  policy::PolicyMap& policy_map =
-      bundle->Get(policy::POLICY_DOMAIN_EXTENSIONS, kManagedStorageExtensionId);
-  policy_map.LoadFrom(
-      policy.get(), policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER);
-  policy_provider_.UpdatePolicy(bundle.Pass());
-
+  SetPolicies(*policy);
   // Now run the extension.
   ASSERT_TRUE(RunExtensionTest("settings/managed_storage")) << message_;
 }
-#endif
+
+IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, ManagedStorageEvents) {
+  ResultCatcher catcher;
+
+  // Set policies for the test extension.
+  scoped_ptr<base::DictionaryValue> policy = extensions::DictionaryBuilder()
+      .Set("constant-policy", "aaa")
+      .Set("changes-policy", "bbb")
+      .Set("deleted-policy", "ccc")
+      .Build();
+  SetPolicies(*policy);
+
+  ExtensionTestMessageListener ready_listener("ready", false);
+  // Load the extension to install the event listener.
+  const Extension* extension = LoadExtension(
+      test_data_dir_.AppendASCII("settings/managed_storage_events"));
+  ASSERT_TRUE(extension);
+  // Wait until the extension sends the "ready" message.
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+
+  // Now change the policies and wait until the extension is done.
+  policy = extensions::DictionaryBuilder()
+      .Set("constant-policy", "aaa")
+      .Set("changes-policy", "ddd")
+      .Set("new-policy", "eee")
+      .Build();
+  SetPolicies(*policy);
+  EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
+}
+#endif  // defined(ENABLE_CONFIGURATION_POLICY)
 
 IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, ManagedStorageDisabled) {
   // Disable the 'managed' namespace. This is redundant when
@@ -449,16 +482,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, ManagedStorageDisabled) {
       browser()->profile()->GetExtensionService()->settings_frontend();
   frontend->DisableStorageForTesting(MANAGED);
   EXPECT_FALSE(frontend->IsStorageEnabled(MANAGED));
-
-  // Set a policy for the extension.
-  scoped_ptr<policy::PolicyBundle> bundle(new policy::PolicyBundle());
-  policy::PolicyMap& policy_map =
-      bundle->Get(policy::POLICY_DOMAIN_EXTENSIONS, kManagedStorageExtensionId);
-  policy_map.Set(
-      "policy", policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
-      base::Value::CreateStringValue("policy_value"));
-  policy_provider_.UpdatePolicy(bundle.Pass());
-
   // Now run the extension.
   ASSERT_TRUE(RunExtensionTest("settings/managed_storage_disabled"))
       << message_;

diff --git a/chrome/browser/extensions/settings/settings_frontend.cc b/chrome/browser/extensions/settings/settings_frontend.cc
@@ -117,7 +117,7 @@ SettingsFrontend::SettingsFrontend(
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
   caches_[settings_namespace::MANAGED] =
-      new ManagedValueStoreCache(profile->GetPolicyService());
+      new ManagedValueStoreCache(profile->GetPolicyService(), observers_);
 #endif
 }
 
@@ -128,6 +128,7 @@ SettingsFrontend::~SettingsFrontend() {
   // after any other task that might've been posted before.
   for (CacheMap::iterator it = caches_.begin(); it != caches_.end(); ++it) {
     ValueStoreCache* cache = it->second;
+    cache->ShutdownOnUI();
     cache->GetMessageLoop()->DeleteSoon(FROM_HERE, cache);
   }
 }
@@ -201,6 +202,7 @@ void SettingsFrontend::DisableStorageForTesting(
   CacheMap::iterator it = caches_.find(settings_namespace);
   if (it != caches_.end()) {
     ValueStoreCache* cache = it->second;
+    cache->ShutdownOnUI();
     cache->GetMessageLoop()->DeleteSoon(FROM_HERE, cache);
     caches_.erase(it);
   }

diff --git a/chrome/browser/extensions/settings/value_store_cache.cc b/chrome/browser/extensions/settings/value_store_cache.cc
@@ -8,4 +8,6 @@ namespace extensions {
 
 ValueStoreCache::~ValueStoreCache() {}
 
+void ValueStoreCache::ShutdownOnUI() {}
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/settings/value_store_cache.h b/chrome/browser/extensions/settings/value_store_cache.h
@@ -19,14 +19,20 @@ class Extension;
 
 // Each namespace of the storage API implements this interface.
 // Instances are created on the UI thread, but from then on live on the loop
-// returned by GetMessageLoop() and every methods (except GetMessageLoop())
-// are always called in that loop, including the destructor.
+// returned by GetMessageLoop() and every method (except GetMessageLoop()
+// and ShutdownOnUI()) is always called in that loop, including the destructor.
 class ValueStoreCache {
  public:
   typedef base::Callback<void(ValueStore*)> StorageCallback;
 
   virtual ~ValueStoreCache();
 
+  // This is invoked from the UI thread during destruction of the Profile that
+  // ultimately owns this object. Any Profile-related cleanups should be
+  // performed in this method, since the destructor will execute later, after
+  // the Profile is already gone.
+  virtual void ShutdownOnUI();
+
   // Returns the loop that the methods of this class should be invoked on.
   virtual scoped_refptr<base::MessageLoopProxy> GetMessageLoop() const = 0;
 

diff --git a/chrome/common/extensions/api/storage.json b/chrome/common/extensions/api/storage.json
@@ -163,7 +163,7 @@
           {
             "name": "areaName",
             "type": "string",
-            "description": "The name of the storage area (\"sync\" or \"local\") the changes are for."
+            "description": "The name of the storage area (\"sync\", \"local\" or \"managed\") the changes are for."
           }
         ]
       }

diff --git a/chrome/common/extensions/docs/apps/storage.html b/chrome/common/extensions/docs/apps/storage.html
@@ -666,7 +666,7 @@ <h4>Listener parameters</h4>
                 </div>
               </em>
           </dt>
-          <dd>The name of the storage area ("sync" or "local") the changes are for.</dd>
+          <dd>The name of the storage area ("sync", "local" or "managed") the changes are for.</dd>
           <!-- OBJECT PROPERTIES -->
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->

diff --git a/chrome/common/extensions/docs/extensions/storage.html b/chrome/common/extensions/docs/extensions/storage.html
@@ -695,7 +695,7 @@ <h4>Listener parameters</h4>
                 </div>
               </em>
           </dt>
-          <dd>The name of the storage area ("sync" or "local") the changes are for.</dd>
+          <dd>The name of the storage area ("sync", "local" or "managed") the changes are for.</dd>
           <!-- OBJECT PROPERTIES -->
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->

diff --git a/chrome/test/data/extensions/api_test/settings/managed_storage/background.js b/chrome/test/data/extensions/api_test/settings/managed_storage/background.js
@@ -26,7 +26,6 @@ chrome.test.runTests([
 
   function getAllPolicies() {
     chrome.storage.managed.get(
-        null,
         chrome.test.callbackPass(function(results) {
           chrome.test.assertEq({
             'string-policy': 'value',
@@ -43,7 +42,6 @@ chrome.test.runTests([
 
   function getBytesInUse() {
     chrome.storage.managed.getBytesInUse(
-        null,
         chrome.test.callbackPass(function(bytes) {
           chrome.test.assertEq(0, bytes);
         }));

diff --git a/chrome/test/data/extensions/api_test/settings/managed_storage_disabled/background.js b/chrome/test/data/extensions/api_test/settings/managed_storage_disabled/background.js
@@ -5,7 +5,6 @@
 chrome.test.runTests([
   function storageIsDisabled() {
     chrome.storage.managed.get(
-        null,
         chrome.test.callbackFail(
             '"managed" is not available in this instance of Chrome'));
   }

diff --git a/chrome/test/data/extensions/api_test/settings/managed_storage_events/background.js b/chrome/test/data/extensions/api_test/settings/managed_storage_events/background.js
@@ -0,0 +1,42 @@
+// 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.
+
+chrome.test.runTests([
+  function managedChangeEvents() {
+    // Verify initial values.
+    chrome.storage.managed.get(
+        chrome.test.callbackPass(function(results) {
+          chrome.test.assertEq({
+            'constant-policy': 'aaa',
+            'changes-policy': 'bbb',
+            'deleted-policy': 'ccc'
+          }, results);
+
+          // Now start listening for changes.
+          // chrome.test.listenOnce() adds and removes the listener to the
+          // given event, and only lets the test complete after receiving the
+          // event.
+          chrome.test.listenOnce(
+              chrome.storage.onChanged,
+              function(changes, namespace) {
+                chrome.test.assertEq('managed', namespace);
+                chrome.test.assertEq({
+                    'changes-policy': {
+                      'oldValue': 'bbb',
+                      'newValue': 'ddd'
+                    },
+                    'deleted-policy': {
+                      'oldValue': 'ccc'
+                    },
+                    'new-policy': {
+                      'newValue': 'eee'
+                    }
+                }, changes);
+              });
+
+          // Signal to the browser that we're listening.
+          chrome.test.sendMessage('ready');
+        }));
+  }
+]);
diff --git a/chrome/test/data/extensions/api_test/settings/managed_storage_events/manifest.json b/chrome/test/data/extensions/api_test/settings/managed_storage_events/manifest.json
@@ -0,0 +1,11 @@
+{
+  "name": "managed storage event tests",
+   "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDkprt3BRSqoikAhSygI6VUzDLt18cKODYmkaa/dwPp4dboyz93RSB+v76grbqsNWrJjkrEwRD3QFeBYBq7h27XAMV4X5XvWjmWQBkRTBQyQI8cZd7M9dgfKrI3EqX9OJvd/wTJkC0dgF47nwWRa/Tvwl7Y66GwEEUjpn2MTv4klwIDAQAB",
+  "version": "1.0",
+  "manifest_version": 2,
+  "description": "Tests events on the 'managed' namespace of the Storage API.",
+  "permissions": ["storage"],
+  "background": {
+    "scripts": ["background.js"]
+  }
+}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -8,4 +8,6 @@ namespace extensions {

		ValueStoreCache::~ValueStoreCache() {}

		void ValueStoreCache::ShutdownOnUI() {}

		} // namespace extensions