Remove NOTIFICATION_PROFILE_DESTROYED from DownloadReporter

Bug: 268984
Change-Id: I3e31bd137be5df84c1fcad094813965d23b2cc44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848502Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705180}

chrome/browser/download/download_offline_content_provider.cc

@@ -422,7 +422,8 @@ void DownloadOfflineContentProvider::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
-void DownloadOfflineContentProvider::OnManagerGoingDown() {
+void DownloadOfflineContentProvider::OnManagerGoingDown(
+    SimpleDownloadManagerCoordinator* manager) {
   std::vector<DownloadItem*> all_items;
   GetAllDownloads(&all_items);
 

chrome/browser/download/download_offline_content_provider.h

@@ -108,7 +108,7 @@ class DownloadOfflineContentProvider
 
   // SimpleDownloadManagerCoordinator::Observer overrides
   void OnDownloadsInitialized(bool active_downloads_only) override;
-  void OnManagerGoingDown() override;
+  void OnManagerGoingDown(SimpleDownloadManagerCoordinator* manager) override;
 
   void GetAllDownloads(std::vector<DownloadItem*>* all_items);
   DownloadItem* GetDownload(const std::string& download_guid);

chrome/browser/safe_browsing/download_protection/download_reporter.cc

@@ -90,62 +90,34 @@ void ReportDangerousDownloadWarningBypassed(
 DownloadReporter::DownloadReporter() {
   profiles_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CREATED,
                           content::NotificationService::AllSources());
-  profiles_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
-                          content::NotificationService::AllSources());
 }
 
-DownloadReporter::~DownloadReporter() {
-  profiles_registrar_.RemoveAll();
-
-  for (download::SimpleDownloadManagerCoordinator* coordinator :
-       observed_coordinators_) {
-    coordinator->RemoveObserver(this);
-  }
-
-  for (download::DownloadItem* download_item : observed_downloads_) {
-    download_item->RemoveObserver(this);
-  }
-}
+DownloadReporter::~DownloadReporter() = default;
 
 void DownloadReporter::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
-  switch (type) {
-    case chrome::NOTIFICATION_PROFILE_CREATED: {
-      DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-      Profile* profile = content::Source<Profile>(source).ptr();
-      download::SimpleDownloadManagerCoordinator* coordinator =
-          SimpleDownloadManagerCoordinatorFactory::GetForKey(
-              profile->GetProfileKey());
-      coordinator->AddObserver(this);
-      observed_coordinators_.insert(coordinator);
-      break;
-    }
-    case chrome::NOTIFICATION_PROFILE_DESTROYED: {
-      DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-      Profile* profile = content::Source<Profile>(source).ptr();
-      download::SimpleDownloadManagerCoordinator* coordinator =
-          SimpleDownloadManagerCoordinatorFactory::GetForKey(
-              profile->GetProfileKey());
-      coordinator->RemoveObserver(this);
-      observed_coordinators_.erase(coordinator);
-      break;
-    }
-    default:
-      NOTREACHED();
-  }
+  DCHECK_EQ(type, chrome::NOTIFICATION_PROFILE_CREATED);
+  Profile* profile = content::Source<Profile>(source).ptr();
+  download::SimpleDownloadManagerCoordinator* coordinator =
+      SimpleDownloadManagerCoordinatorFactory::GetForKey(
+          profile->GetProfileKey());
+  observed_coordinators_.Add(coordinator);
+}
+
+void DownloadReporter::OnManagerGoingDown(
+    download::SimpleDownloadManagerCoordinator* coordinator) {
+  observed_coordinators_.Remove(coordinator);
 }
 
 void DownloadReporter::OnDownloadCreated(download::DownloadItem* download) {
   danger_types_[download] = download->GetDangerType();
-  download->AddObserver(this);
-  observed_downloads_.insert(download);
+  observed_downloads_.Add(download);
 }
 
 void DownloadReporter::OnDownloadDestroyed(download::DownloadItem* download) {
-  download->RemoveObserver(this);
+  observed_downloads_.Remove(download);
   danger_types_.erase(download);
-  observed_downloads_.erase(download);
 }
 
 void DownloadReporter::OnDownloadUpdated(download::DownloadItem* download) {

chrome/browser/safe_browsing/download_protection/download_reporter.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_PROTECTION_DOWNLOAD_REPORTER_H_
 #define CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_PROTECTION_DOWNLOAD_REPORTER_H_
 
+#include "base/macros.h"
+#include "base/scoped_observer.h"
 #include "components/download/public/common/download_item.h"
 #include "components/download/public/common/simple_download_manager_coordinator.h"
 #include "content/public/browser/notification_observer.h"
@@ -28,6 +30,8 @@ class DownloadReporter
                const content::NotificationDetails& details) override;
 
   // SimpleDownloadManagerCoordinator::Observer implementation:
+  void OnManagerGoingDown(
+      download::SimpleDownloadManagerCoordinator* coordinator) override;
   void OnDownloadCreated(download::DownloadItem* download) override;
 
   // DownloadItem::Observer implementation:
@@ -38,8 +42,13 @@ class DownloadReporter
   content::NotificationRegistrar profiles_registrar_;
   base::flat_map<download::DownloadItem*, download::DownloadDangerType>
       danger_types_;
-  std::set<download::SimpleDownloadManagerCoordinator*> observed_coordinators_;
-  std::set<download::DownloadItem*> observed_downloads_;
+  ScopedObserver<download::SimpleDownloadManagerCoordinator,
+                 download::SimpleDownloadManagerCoordinator::Observer>
+      observed_coordinators_{this};
+  ScopedObserver<download::DownloadItem, download::DownloadItem::Observer>
+      observed_downloads_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadReporter);
 };
 
 }  // namespace safe_browsing

components/download/internal/common/all_download_event_notifier.cc

@@ -49,7 +49,9 @@ void AllDownloadEventNotifier::OnDownloadsInitialized(
                                     active_downloads_only);
 }
 
-void AllDownloadEventNotifier::OnManagerGoingDown() {
+void AllDownloadEventNotifier::OnManagerGoingDown(
+    SimpleDownloadManagerCoordinator* manager) {
+  DCHECK_EQ(manager, simple_download_manager_coordinator_);
   for (auto& observer : observers_)
     observer.OnManagerGoingDown(simple_download_manager_coordinator_);
   simple_download_manager_coordinator_->RemoveObserver(this);

components/download/internal/common/simple_download_manager_coordinator.cc

@@ -31,7 +31,7 @@ SimpleDownloadManagerCoordinator::~SimpleDownloadManagerCoordinator() {
   if (simple_download_manager_)
     simple_download_manager_->RemoveObserver(this);
   for (auto& observer : observers_)
-    observer.OnManagerGoingDown();
+    observer.OnManagerGoingDown(this);
 }
 
 void SimpleDownloadManagerCoordinator::SetSimpleDownloadManager(

components/download/public/common/all_download_event_notifier.h

@@ -56,7 +56,7 @@ class COMPONENTS_DOWNLOAD_EXPORT AllDownloadEventNotifier
  private:
   // SimpleDownloadManagerCoordinator::Observer
   void OnDownloadsInitialized(bool active_downloads_only) override;
-  void OnManagerGoingDown() override;
+  void OnManagerGoingDown(SimpleDownloadManagerCoordinator* manager) override;
   void OnDownloadCreated(DownloadItem* item) override;
 
   // DownloadItem::Observer

components/download/public/common/simple_download_manager_coordinator.h

@@ -36,7 +36,8 @@ class COMPONENTS_DOWNLOAD_EXPORT SimpleDownloadManagerCoordinator
     Observer() = default;
     virtual ~Observer() = default;
 
-    virtual void OnManagerGoingDown() {}
+    virtual void OnManagerGoingDown(
+        SimpleDownloadManagerCoordinator* coordinator) {}
     virtual void OnDownloadsInitialized(bool active_downloads_only) {}
     virtual void OnDownloadCreated(DownloadItem* item) {}