Some fixups for app notifications:

chrome/browser/extensions/app_notification_manager.cc

 // Copyright (c) 2011 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/app_notification_manager.h"
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/file_path.h"
 #include "base/location.h"
@@ skipping 92 matching lines @@
 
   SyncAddChange(*linked_item);
 
   if (storage_.get()) {
     BrowserThread::PostTask(
         BrowserThread::FILE,
         FROM_HERE,
         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;
 }
 
 const AppNotificationList* AppNotificationManager::GetAll(
     const std::string& extension_id) const {
   if (!loaded())
     return NULL;
   if (ContainsKey(*notifications_, extension_id))
     return &((*notifications_)[extension_id]);
   return NULL;
@@ skipping 11 matching lines @@
 }
 
 const AppNotification* AppNotificationManager::GetLast(
     const std::string& extension_id) {
   if (!loaded())
     return NULL;
   NotificationMap::iterator found = notifications_->find(extension_id);
   if (found == notifications_->end())
     return NULL;
   const AppNotificationList& list = found->second;
+  if (list.empty())
+    return NULL;
   return list.rbegin()->get();
 }
 
 void AppNotificationManager::ClearAll(const std::string& extension_id) {
   if (!loaded())
     return;
   NotificationMap::iterator found = notifications_->find(extension_id);
   if (found != notifications_->end()) {
     SyncClearAllChange(found->second);
     notifications_->erase(found);
   }
 
   if (storage_.get()) {
     BrowserThread::PostTask(
         BrowserThread::FILE,
         FROM_HERE,
         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(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   CHECK(type == chrome::NOTIFICATION_EXTENSION_UNINSTALLED);
   ClearAll(*content::Details<const std::string>(details).ptr());
 }
 
@@ skipping 21 matching lines @@
       base::Bind(&AppNotificationManager::HandleLoadResults,
           this, result.release()));
 }
 
 void AppNotificationManager::HandleLoadResults(NotificationMap* map) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(map);
   DCHECK(!loaded());
   notifications_.reset(map);
 
-  // Generate STATE_CHAGNED notifications for extensions that have at
-  // least one notification loaded.
-  NotificationMap::const_iterator i;
-  for (i = map->begin(); i != map->end(); ++i) {
-    const std::string& id = i->first;
-    if (i->second.empty())
-      continue;
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED,

[asargent_no_longer_on_chrome, 2011/10/24 19:09:17]

I think removing this code block means that in the case of having the new tab
page open right at chrome start (before
AppNotificationManager::HandleLoadResults gets called), we'll miss showing
notifications.

I think we either need to leave this in, or make the AppLauncherHandler class
listen for NOTIFICATION_APP_NOTIFICATION_MANAGER_LOADED

[Munjal (Google), 2011/10/24 21:44:11]

I thought that since Add now generates STATE_CHANGED notifications, that will
not happen. Is that not right?

[Munjal (Google), 2011/10/24 21:46:25]

Actually, you are right. We do not call Add in this code path. Let me revive
this code.

[Munjal (Google), 2011/10/24 21:55:22]

Done.

-        content::Source<Profile>(profile_),
-        content::Details<const std::string>(&id));
-  }
-
   // Generate MANAGER_LOADED notification.
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_APP_NOTIFICATION_MANAGER_LOADED,
       content::Source<AppNotificationManager>(this),
       content::NotificationService::NoDetails());
 }
 
 void AppNotificationManager::SaveOnFileThread(const std::string& extension_id,
                                               AppNotificationList* list) {
   // Own the |list|.
@@ skipping 205 matching lines @@
   AppNotificationList& list = GetAllInternal(extension_id);
   RemoveByGuid(&list, guid);
 
   if (storage_.get()) {
     BrowserThread::PostTask(
         BrowserThread::FILE,
         FROM_HERE,
         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
 SyncData AppNotificationManager::CreateSyncDataFromNotification(
     const AppNotification& notification) {
   DCHECK(!notification.is_local());
   sync_pb::EntitySpecifics specifics;
   sync_pb::AppNotificationSpecifics* notif_specifics =
       specifics.MutableExtension(sync_pb::app_notification);
   notif_specifics->set_app_id(notification.extension_id());
@@ skipping 21 matching lines @@
 
   AppNotification* notification = new AppNotification(
       false, specifics.guid(), specifics.app_id(),
       specifics.title(), specifics.body_text());
   if (specifics.has_link_text())
     notification->set_link_text(specifics.link_text());
   if (specifics.has_link_url())
     notification->set_link_url(GURL(specifics.link_url()));
   return notification;
 }