Some fixups for app notifications:

- Generate STATE_CHANGED notification in the Add method instead of
  in other places which call Add.
- Generate STATE_CHANGED notification in ClearAll and Remove methods.
- Hook up the bubble close button action with code that calls ClearAll.
Review URL: http://codereview.chromium.org/8382010

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

chrome/browser/extensions/app_notification_manager.cc

@@ -110,6 +110,12 @@ bool AppNotificationManager::Add(AppNotification* item) {
         base::Bind(&AppNotificationManager::SaveOnFileThread,
             this, extension_id, CopyAppNotificationList(list)));
   }
+
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED,
+      content::Source<Profile>(profile_),
+      content::Details<const std::string>(&extension_id));
+
   return true;
 }
 
@@ -141,6 +147,8 @@ const AppNotification* AppNotificationManager::GetLast(
   if (found == notifications_->end())
     return NULL;
   const AppNotificationList& list = found->second;
+  if (list.empty())
+    return NULL;
   return list.rbegin()->get();
 }
 
@@ -160,6 +168,11 @@ void AppNotificationManager::ClearAll(const std::string& extension_id) {
         base::Bind(&AppNotificationManager::DeleteOnFileThread,
             this, extension_id));
   }
+
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED,
+      content::Source<Profile>(profile_),
+      content::Details<const std::string>(&extension_id));
 }
 
 void AppNotificationManager::Observe(
@@ -439,6 +452,11 @@ void AppNotificationManager::Remove(const std::string& extension_id,
         base::Bind(&AppNotificationManager::SaveOnFileThread,
             this, extension_id, CopyAppNotificationList(list)));
   }
+
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED,
+      content::Source<Profile>(profile_),
+      content::Details<const std::string>(&extension_id));
 }
 
 // static

chrome/browser/extensions/app_notification_manager_sync_unittest.cc

@@ -146,12 +146,6 @@ class AppNotificationManagerSyncTest : public testing::Test {
     return notif;
   }
 
-  static AppNotification* CreateNotification(const AppNotification& notif) {
-    return CreateNotification(
-        notif.is_local(), notif.guid(), notif.extension_id(), notif.title(),
-        notif.body(), notif.link_url().spec(), notif.link_text());
-  }
-
   static SyncData CreateSyncData(int suffix) {
     scoped_ptr<AppNotification> notif(CreateNotification(suffix));
     return AppNotificationManager::CreateSyncDataFromNotification(*notif);
@@ -491,8 +485,7 @@ TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesNonEmptyModel) {
   SyncChangeList changes;
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(3)));
-  changes.push_back(CreateSyncChange(
-      SyncChange::ACTION_DELETE, CreateNotification(*n1)));
+  changes.push_back(CreateSyncChange(SyncChange::ACTION_DELETE, n1->Copy()));
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(4)));
 

chrome/browser/extensions/extension_app_api.cc

@@ -72,11 +72,6 @@ bool AppNotifyFunction::RunImpl() {
 
   manager->Add(item.release());
 
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED,
-      content::Source<Profile>(profile_),
-      content::Details<const std::string>(&id));
-
   return true;
 }
 
@@ -94,9 +89,5 @@ bool AppClearAllNotificationsFunction::RunImpl() {
   AppNotificationManager* manager =
       profile()->GetExtensionService()->app_notification_manager();
   manager->ClearAll(id);
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED,
-      content::Source<Profile>(profile_),
-      content::Details<const std::string>(&id));
   return true;
 }

chrome/browser/resources/ntp4/apps_page.js

@@ -301,7 +301,7 @@ cr.define('ntp4', function() {
     // text.
     setupNotification_: function(notification) {
       // Remove the old notification from this node (if any).
-      if (this.appNotification_)
+      if (this.appNotification_ && this.appNotification_.parentNode)
         this.appNotification_.parentNode.removeChild(this.appNotification_);
 
       if (notification) {
@@ -455,8 +455,13 @@ cr.define('ntp4', function() {
       }
 
       var infoBubble = new cr.ui.Bubble;
+      infoBubble.appId = this.appData_.id;
       infoBubble.anchorNode = e.target;
       infoBubble.content = container;
+      infoBubble.handleCloseEvent = function() {
+        chrome.send('closeNotification', [this.appId]);
+        this.hide();
+      };
       infoBubble.show();
     },
 

chrome/browser/ui/webui/ntp/app_launcher_handler.cc

@@ -277,6 +277,9 @@ void AppLauncherHandler::RegisterMessages() {
   web_ui_->RegisterMessageCallback("recordAppLaunchByURL",
       base::Bind(&AppLauncherHandler::HandleRecordAppLaunchByURL,
                  base::Unretained(this)));
+  web_ui_->RegisterMessageCallback("closeNotification",
+      base::Bind(&AppLauncherHandler::HandleNotificationClose,
+                 base::Unretained(this)));
 }
 
 void AppLauncherHandler::Observe(int type,
@@ -837,6 +840,20 @@ void AppLauncherHandler::HandleRecordAppLaunchByURL(
   RecordAppLaunchByURL(Profile::FromWebUI(web_ui_), url, bucket);
 }
 
+void AppLauncherHandler::HandleNotificationClose(const ListValue* args) {
+  std::string extension_id;
+  CHECK(args->GetString(0, &extension_id));
+
+  const Extension* extension = extension_service_->GetExtensionById(
+      extension_id, true);
+  if (!extension)
+    return;
+
+  AppNotificationManager* notification_manager =
+      extension_service_->app_notification_manager();
+  notification_manager->ClearAll(extension_id);
+}
+
 void AppLauncherHandler::OnFaviconForApp(FaviconService::Handle handle,
                                          history::FaviconData data) {
   scoped_ptr<AppInstallInfo> install_info(

chrome/browser/ui/webui/ntp/app_launcher_handler.h

@@ -111,6 +111,9 @@ class AppLauncherHandler : public WebUIMessageHandler,
   // action for UMA.
   void HandleRecordAppLaunchByURL(const base::ListValue* args);
 
+  // Callback for "closeNotification" message.
+  void HandleNotificationClose(const base::ListValue* args);
+
   // Register app launcher preferences.
   static void RegisterUserPrefs(PrefService* pref_service);