Add notifications to allow desktop notification permissions to be synced.

Fix a bug where sites can be added to allow and deny lists multiple times.

BUG=none
TEST=Open two browsers with separate profiles.  Request permission for notifications on a site on one.  Check that the permissions are the same on the other browser.

Review URL: http://codereview.chromium.org/1578023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44234 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 166 insertions, 29 deletions

diff --git a/base/values.cc b/base/values.cc
index f214ead..feff299 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -832,6 +832,16 @@ void ListValue::Append(Value* in_value) {
   list_.push_back(in_value);
 }
 
+bool ListValue::AppendIfNotPresent(Value* in_value) {
+  DCHECK(in_value);
+  for (ValueVector::iterator i(list_.begin()); i != list_.end(); ++i) {
+    if ((*i)->Equals(in_value))
+      return false;
+  }
+  list_.push_back(in_value);
+  return true;
+}
+
 bool ListValue::Insert(size_t index, Value* in_value) {
   DCHECK(in_value);
   if (index > list_.size())
diff --git a/base/values.h b/base/values.h
index 6c66345..248fc1d 100644
--- a/base/values.h
+++ b/base/values.h
@@ -401,6 +401,10 @@ class ListValue : public Value {
   // Appends a Value to the end of the list.
   void Append(Value* in_value);
 
+  // Appends a Value if it's not already present.
+  // Returns true if successful, or false if the value was already present.
+  bool AppendIfNotPresent(Value* in_value);
+
   // Insert a Value at index.
   // Returns true if successful, or false if the index was out of range.
   bool Insert(size_t index, Value* in_value);
diff --git a/chrome/browser/notifications/desktop_notification_service.cc b/chrome/browser/notifications/desktop_notification_service.cc
index aa769d3..ef92a76 100644
--- a/chrome/browser/notifications/desktop_notification_service.cc
+++ b/chrome/browser/notifications/desktop_notification_service.cc
@@ -21,9 +21,11 @@
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/renderer_host/site_instance.h"
+#include "chrome/browser/scoped_pref_update.h"
 #include "chrome/browser/tab_contents/infobar_delegate.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/worker_host/worker_process_host.h"
+#include "chrome/common/notification_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
@@ -194,32 +196,60 @@ DesktopNotificationService::DesktopNotificationService(Profile* profile,
     : profile_(profile),
       ui_manager_(ui_manager) {
   InitPrefs();
+  StartObserving();
 }
 
 DesktopNotificationService::~DesktopNotificationService() {
+  StopObserving();
 }
 
 // Initialize the cache with the allowed and denied origins, or
 // create the preferences if they don't exist yet.
 void DesktopNotificationService::InitPrefs() {
   PrefService* prefs = profile_->GetPrefs();
-  const ListValue* allowed_sites = NULL;
-  const ListValue* denied_sites = NULL;
+  std::vector<GURL> allowed_origins;
+  std::vector<GURL> denied_origins;
 
   if (!profile_->IsOffTheRecord()) {
     if (!prefs->FindPreference(prefs::kDesktopNotificationAllowedOrigins))
       prefs->RegisterListPref(prefs::kDesktopNotificationAllowedOrigins);
-    allowed_sites = prefs->GetList(prefs::kDesktopNotificationAllowedOrigins);
+    const ListValue* allowed_sites =
+        prefs->GetList(prefs::kDesktopNotificationAllowedOrigins);
+    if (allowed_sites)
+      NotificationsPrefsCache::ListValueToGurlVector(*allowed_sites,
+                                                     &allowed_origins);
 
     if (!prefs->FindPreference(prefs::kDesktopNotificationDeniedOrigins))
       prefs->RegisterListPref(prefs::kDesktopNotificationDeniedOrigins);
-    denied_sites = prefs->GetList(prefs::kDesktopNotificationDeniedOrigins);
+    const ListValue* denied_sites =
+        prefs->GetList(prefs::kDesktopNotificationDeniedOrigins);
+    if (denied_sites)
+      NotificationsPrefsCache::ListValueToGurlVector(*denied_sites,
+                                                     &denied_origins);
   }
 
-  prefs_cache_ = new NotificationsPrefsCache(allowed_sites, denied_sites);
+  prefs_cache_ = new NotificationsPrefsCache();
+  prefs_cache_->SetCacheAllowedOrigins(allowed_origins);
+  prefs_cache_->SetCacheDeniedOrigins(denied_origins);
   prefs_cache_->set_is_initialized(true);
 }
 
+void DesktopNotificationService::StartObserving() {
+  if (!profile_->IsOffTheRecord()) {
+    PrefService* prefs = profile_->GetPrefs();
+    prefs->AddPrefObserver(prefs::kDesktopNotificationAllowedOrigins, this);
+    prefs->AddPrefObserver(prefs::kDesktopNotificationDeniedOrigins, this);
+  }
+}
+
+void DesktopNotificationService::StopObserving() {
+  if (!profile_->IsOffTheRecord()) {
+    PrefService* prefs = profile_->GetPrefs();
+    prefs->RemovePrefObserver(prefs::kDesktopNotificationAllowedOrigins, this);
+    prefs->RemovePrefObserver(prefs::kDesktopNotificationDeniedOrigins, this);
+  }
+}
+
 void DesktopNotificationService::GrantPermission(const GURL& origin) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   PersistPermissionChange(origin, true);
@@ -244,6 +274,42 @@ void DesktopNotificationService::DenyPermission(const GURL& origin) {
           origin));
 }
 
+void DesktopNotificationService::Observe(NotificationType type,
+                                         const NotificationSource& source,
+                                         const NotificationDetails& details) {
+  DCHECK(NotificationType::PREF_CHANGED == type);
+  PrefService* prefs = profile_->GetPrefs();
+  std::wstring* name = Details<std::wstring>(details).ptr();
+
+  if (0 == name->compare(prefs::kDesktopNotificationAllowedOrigins)) {
+    const ListValue* allowed_sites =
+        prefs->GetList(prefs::kDesktopNotificationAllowedOrigins);
+    std::vector<GURL> allowed_origins;
+    NotificationsPrefsCache::ListValueToGurlVector(*allowed_sites,
+                                                   &allowed_origins);
+    // Schedule a cache update on the IO thread.
+    ChromeThread::PostTask(
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableMethod(
+            prefs_cache_.get(),
+            &NotificationsPrefsCache::SetCacheAllowedOrigins,
+            allowed_origins));
+  } else if (0 == name->compare(prefs::kDesktopNotificationDeniedOrigins)) {
+    const ListValue* denied_sites =
+        prefs->GetList(prefs::kDesktopNotificationDeniedOrigins);
+    std::vector<GURL> denied_origins;
+    NotificationsPrefsCache::ListValueToGurlVector(*denied_sites,
+                                                   &denied_origins);
+    // Schedule a cache update on the IO thread.
+    ChromeThread::PostTask(
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableMethod(
+            prefs_cache_.get(),
+            &NotificationsPrefsCache::SetCacheDeniedOrigins,
+            denied_origins));
+  }
+}
+
 void DesktopNotificationService::PersistPermissionChange(
     const GURL& origin, bool is_allowed) {
   // Don't persist changes when off the record.
@@ -251,20 +317,51 @@ void DesktopNotificationService::PersistPermissionChange(
     return;
 
   PrefService* prefs = profile_->GetPrefs();
+
+  StopObserving();
+
+  bool allowed_changed = false;
+  bool denied_changed = false;
+
   ListValue* allowed_sites =
       prefs->GetMutableList(prefs::kDesktopNotificationAllowedOrigins);
   ListValue* denied_sites =
       prefs->GetMutableList(prefs::kDesktopNotificationDeniedOrigins);
   StringValue* value = new StringValue(origin.spec());
+
   // Remove from one list and add to the other.
   if (is_allowed) {
-    allowed_sites->Append(value);
-    denied_sites->Remove(*value);
+    // Add to the allowed list.
+    if (allowed_sites->AppendIfNotPresent(value))
+      allowed_changed = true;
+
+    // Remove from the denied list.
+    if (denied_sites->Remove(*value) != -1)
+      denied_changed = true;
   } else {
-    allowed_sites->Remove(*value);
-    denied_sites->Append(value);
+    // Remove from the allowed list.
+    if (allowed_sites->Remove(*value) != -1)
+      allowed_changed = true;
+
+    // Remove from the denied list.
+    if (denied_sites->AppendIfNotPresent(value))
+      denied_changed = true;
+  }
+
+  // Persist the pref if anthing changed, but only send updates for the
+  // list that changed.
+  if (allowed_changed || denied_changed) {
+    if (allowed_changed) {
+      ScopedPrefUpdate updateAllowed(
+          prefs, prefs::kDesktopNotificationAllowedOrigins);
+    }
+    if (denied_changed) {
+      ScopedPrefUpdate updateDenied(
+          prefs, prefs::kDesktopNotificationDeniedOrigins);
+    }
+    prefs->ScheduleSavePersistentPrefs();
   }
-  prefs->ScheduleSavePersistentPrefs();
+  StartObserving();
 }
 
 void DesktopNotificationService::RequestPermission(
diff --git a/chrome/browser/notifications/desktop_notification_service.h b/chrome/browser/notifications/desktop_notification_service.h
index 8be6f85..c3a41f9 100644
--- a/chrome/browser/notifications/desktop_notification_service.h
+++ b/chrome/browser/notifications/desktop_notification_service.h
@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "chrome/browser/notifications/notification.h"
+#include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_service.h"
 #include "googleurl/src/gurl.h"
@@ -21,7 +22,7 @@ class Task;
 
 // The DesktopNotificationService is an object, owned by the Profile,
 // which provides the creation of desktop "toasts" to web pages and workers.
-class DesktopNotificationService {
+class DesktopNotificationService : public NotificationObserver {
  public:
   enum DesktopNotificationSource {
     PageNotification,
@@ -69,6 +70,11 @@ class DesktopNotificationService {
   void GrantPermission(const GURL& origin);
   void DenyPermission(const GURL& origin);
 
+  // NotificationObserver implementation.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
   NotificationsPrefsCache* prefs_cache() { return prefs_cache_; }
 
   // Creates a data:xxxx URL which contains the full HTML for a notification
@@ -78,6 +84,8 @@ class DesktopNotificationService {
                                 const string16& body);
  private:
   void InitPrefs();
+  void StartObserving();
+  void StopObserving();
 
   // Save a permission change to the profile.
   void PersistPermissionChange(const GURL& origin, bool is_allowed);
diff --git a/chrome/browser/notifications/desktop_notifications_unittest.cc b/chrome/browser/notifications/desktop_notifications_unittest.cc
index 637d9a1..be7a6f6 100644
--- a/chrome/browser/notifications/desktop_notifications_unittest.cc
+++ b/chrome/browser/notifications/desktop_notifications_unittest.cc
@@ -82,9 +82,9 @@ void DesktopNotificationsTest::SetUp() {
 }
 
 void DesktopNotificationsTest::TearDown() {
+  service_.reset(NULL);
   profile_.reset(NULL);
   ui_manager_.reset(NULL);
-  service_.reset(NULL);
 }
 
 TEST_F(DesktopNotificationsTest, TestShow) {
diff --git a/chrome/browser/notifications/notifications_prefs_cache.cc b/chrome/browser/notifications/notifications_prefs_cache.cc
index b127282..6a5172b 100644
--- a/chrome/browser/notifications/notifications_prefs_cache.cc
+++ b/chrome/browser/notifications/notifications_prefs_cache.cc
@@ -10,23 +10,8 @@
 #include "chrome/browser/pref_service.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebNotificationPresenter.h"
 
-NotificationsPrefsCache::NotificationsPrefsCache(
-    const ListValue* allowed, const ListValue* denied)
+NotificationsPrefsCache::NotificationsPrefsCache()
         : is_initialized_(false) {
-  ListValue::const_iterator i;
-  std::wstring origin;
-  if (allowed) {
-    for (i = allowed->begin(); i != allowed->end(); ++i) {
-      (*i)->GetAsString(&origin);
-      allowed_origins_.insert(GURL(WideToUTF8(origin)));
-    }
-  }
-  if (denied) {
-    for (i = denied->begin(); i != denied->end(); ++i) {
-      (*i)->GetAsString(&origin);
-      denied_origins_.insert(GURL(WideToUTF8(origin)));
-    }
-  }
 }
 
 void NotificationsPrefsCache::CacheAllowedOrigin(
@@ -47,6 +32,30 @@ void NotificationsPrefsCache::CacheDeniedOrigin(
     allowed_origins_.erase(iter);
 }
 
+void NotificationsPrefsCache::SetCacheAllowedOrigins(
+    const std::vector<GURL>& allowed) {
+  allowed_origins_.clear();
+  allowed_origins_.insert(allowed.begin(), allowed.end());
+}
+
+void NotificationsPrefsCache::SetCacheDeniedOrigins(
+    const std::vector<GURL>& denied) {
+  denied_origins_.clear();
+  denied_origins_.insert(denied.begin(), denied.end());
+}
+
+// static
+void NotificationsPrefsCache::ListValueToGurlVector(
+    const ListValue& origin_list,
+    std::vector<GURL>* origin_vector) {
+  ListValue::const_iterator i;
+  std::wstring origin;
+  for (i = origin_list.begin(); i != origin_list.end(); ++i) {
+    (*i)->GetAsString(&origin);
+    origin_vector->push_back(GURL(WideToUTF8(origin)));
+  }
+}
+
 int NotificationsPrefsCache::HasPermission(const GURL& origin) {
   if (IsOriginAllowed(origin))
     return WebKit::WebNotificationPresenter::PermissionAllowed;
diff --git a/chrome/browser/notifications/notifications_prefs_cache.h b/chrome/browser/notifications/notifications_prefs_cache.h
index de00774..1b87a29 100644
--- a/chrome/browser/notifications/notifications_prefs_cache.h
+++ b/chrome/browser/notifications/notifications_prefs_cache.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATIONS_PREFS_CACHE_H_
 
 #include <set>
+#include <vector>
 
 #include "base/ref_counted.h"
 #include "googleurl/src/gurl.h"
@@ -19,7 +20,7 @@ class ListValue;
 class NotificationsPrefsCache
     : public base::RefCountedThreadSafe<NotificationsPrefsCache> {
  public:
-  NotificationsPrefsCache(const ListValue* allowed, const ListValue* denied);
+  NotificationsPrefsCache();
 
   void set_is_initialized(bool val) { is_initialized_ = val; }
   bool is_initialized() { return is_initialized_; }
@@ -33,6 +34,14 @@ class NotificationsPrefsCache
   void CacheAllowedOrigin(const GURL& origin);
   void CacheDeniedOrigin(const GURL& origin);
 
+  // Set the cache to the supplied values.  This clears the current
+  // contents of the cache.
+  void SetCacheAllowedOrigins(const std::vector<GURL>& allowed);
+  void SetCacheDeniedOrigins(const std::vector<GURL>& denied);
+
+  static void ListValueToGurlVector(const ListValue& origin_list,
+                                    std::vector<GURL>* origin_vector);
+
  private:
   friend class base::RefCountedThreadSafe<NotificationsPrefsCache>;