Remove NOTIFICATION_PROFILE_DESTROYED from NotificationUiManager

Bug: 268984
Change-Id: I703e5a294a7c51d02a19cfd504cb8e0f2dfbd97a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875270Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708807}

chrome/browser/notifications/notification_system_observer.cc

@@ -21,8 +21,6 @@ NotificationSystemObserver::NotificationSystemObserver(
                  content::NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
                  content::NotificationService::AllSources());
-  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
-                 content::NotificationService::AllSources());
   for (auto* profile :
        g_browser_process->profile_manager()->GetLoadedProfiles()) {
     extension_registry_observer_.Add(
@@ -52,12 +50,6 @@ void NotificationSystemObserver::Observe(
         extension_registry_observer_.Add(registry);
       break;
     }
-    case chrome::NOTIFICATION_PROFILE_DESTROYED:
-      // We only want to remove the incognito notifications.
-      if (content::Source<Profile>(source)->IsOffTheRecord())
-        ui_manager_->CancelAllByProfile(NotificationUIManager::GetProfileID(
-            content::Source<Profile>(source).ptr()));
-      break;
     default:
       NOTREACHED();
   }

chrome/browser/notifications/notification_test_util.cc

@@ -126,11 +126,6 @@ bool StubNotificationUIManager::CancelAllBySourceOrigin(
   return true;
 }
 
-bool StubNotificationUIManager::CancelAllByProfile(ProfileID profile_id) {
-  NOTIMPLEMENTED();
-  return false;
-}
-
 void StubNotificationUIManager::CancelAll() {
   for (const auto& pair : notifications_)
     pair.first.delegate()->Close(false /* by_user */);

chrome/browser/notifications/notification_test_util.h

@@ -53,7 +53,6 @@ class StubNotificationUIManager : public NotificationUIManager {
                   ProfileID profile_id) override;
   std::set<std::string> GetAllIdsByProfile(ProfileID profile_id) override;
   bool CancelAllBySourceOrigin(const GURL& source_origin) override;
-  bool CancelAllByProfile(ProfileID profile_id) override;
   void CancelAll() override;
   void StartShutdown() override;
 

chrome/browser/notifications/notification_ui_manager.h

@@ -81,10 +81,6 @@ class NotificationUIManager {
   // extension ID). Returns true if anything was removed.
   virtual bool CancelAllBySourceOrigin(const GURL& source_origin) = 0;
 
-  // Removes notifications matching |profile_id|. Returns true if any were
-  // removed.
-  virtual bool CancelAllByProfile(ProfileID profile_id) = 0;
-
   // Cancels all pending notifications and closes anything currently showing.
   // Used when the app is terminating.
   virtual void CancelAll() = 0;

chrome/browser/notifications/notification_ui_manager_impl.cc

@@ -85,6 +85,9 @@ void NotificationUIManagerImpl::Add(
   MessageCenter::Get()->AddNotification(
       std::make_unique<message_center::Notification>(
           profile_notification->notification()));
+
+  if (profile->IsOffTheRecord())
+    observed_otr_profiles_.Add(profile);
 }
 
 bool NotificationUIManagerImpl::Update(
@@ -190,23 +193,6 @@ bool NotificationUIManagerImpl::CancelAllBySourceOrigin(const GURL& source) {
   return removed;
 }
 
-bool NotificationUIManagerImpl::CancelAllByProfile(ProfileID profile_id) {
-  // Same pattern as CancelAllBySourceOrigin.
-  bool removed = false;
-
-  for (auto loopiter = profile_notifications_.begin();
-       loopiter != profile_notifications_.end();) {
-    auto curiter = loopiter++;
-    if (profile_id == (*curiter).second->profile_id()) {
-      const std::string id = curiter->first;
-      RemoveProfileNotification(id);
-      MessageCenter::Get()->RemoveNotification(id, /* by_user */ false);
-      removed = true;
-    }
-  }
-  return removed;
-}
-
 void NotificationUIManagerImpl::CancelAll() {
   MessageCenter::Get()->RemoveAllNotifications(
       false /* by_user */, message_center::MessageCenter::RemoveType::ALL);
@@ -226,6 +212,24 @@ void NotificationUIManagerImpl::OnNotificationRemoved(const std::string& id,
   RemoveProfileNotification(id);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// ProfileObserver
+
+void NotificationUIManagerImpl::OnProfileWillBeDestroyed(Profile* profile) {
+  observed_otr_profiles_.Remove(profile);
+
+  // Same pattern as CancelAllBySourceOrigin.
+  for (auto loopiter = profile_notifications_.begin();
+       loopiter != profile_notifications_.end();) {
+    auto curiter = loopiter++;
+    if (GetProfileID(profile) == (*curiter).second->profile_id()) {
+      const std::string id = curiter->first;
+      RemoveProfileNotification(id);
+      MessageCenter::Get()->RemoveNotification(id, /* by_user */ false);
+    }
+  }
+}
+
 void NotificationUIManagerImpl::ResetUiControllerForTest() {
   popups_only_ui_controller_.reset();
 }

chrome/browser/notifications/notification_ui_manager_impl.h

@@ -13,10 +13,13 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/notifications/notification_system_observer.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_observer.h"
 #include "ui/message_center/message_center_observer.h"
 #include "ui/message_center/message_center_types.h"
 #include "ui/message_center/public/cpp/notification.h"
@@ -35,7 +38,8 @@ FORWARD_DECLARE_TEST(NotificationTrayTest, ManuallyCloseMessageCenter);
 // of notifications to MessageCenter, doing necessary conversions. This is only
 // used on platforms that support non-native notifications.
 class NotificationUIManagerImpl : public NotificationUIManager,
-                                  public message_center::MessageCenterObserver {
+                                  public message_center::MessageCenterObserver,
+                                  public ProfileObserver {
  public:
   NotificationUIManagerImpl();
   ~NotificationUIManagerImpl() override;
@@ -52,7 +56,6 @@ class NotificationUIManagerImpl : public NotificationUIManager,
                   ProfileID profile_id) override;
   std::set<std::string> GetAllIdsByProfile(ProfileID profile_id) override;
   bool CancelAllBySourceOrigin(const GURL& source_origin) override;
-  bool CancelAllByProfile(ProfileID profile_id) override;
   void CancelAll() override;
   void StartShutdown() override;
 
@@ -60,6 +63,9 @@ class NotificationUIManagerImpl : public NotificationUIManager,
   void OnNotificationRemoved(const std::string& notification_id,
                              bool by_user) override;
 
+  // ProfileObserver:
+  void OnProfileWillBeDestroyed(Profile* profile) override;
+
   // Resets the ui controller.
   void ResetUiControllerForTest();
 
@@ -95,6 +101,8 @@ class NotificationUIManagerImpl : public NotificationUIManager,
 
   NotificationSystemObserver system_observer_;
 
+  ScopedObserver<Profile, ProfileObserver> observed_otr_profiles_{this};
+
   // Delegate of this class.
   std::unique_ptr<PopupsOnlyUiController> popups_only_ui_controller_;