Remove NOTIFICATION_BROWSING_DATA_REMOVED

Replace the notification system with a static callback list in
BrowsingDataRemover.

BUG=chromium:268984
TEST=BrowsingDataRemoverTest and ExtensionBrowsingDataTest pass.
Signed-off-by: Simon Que <sque@chromium.org>

Review URL: https://codereview.chromium.org/874613004

Cr-Commit-Position: refs/heads/master@{#313137}

chrome/browser/browsing_data/browsing_data_remover.cc

@@ -101,6 +101,24 @@ using content::BrowserContext;
 using content::BrowserThread;
 using content::DOMStorageContext;
 
+namespace {
+
+using CallbackList =
+    base::CallbackList<void(const BrowsingDataRemover::NotificationDetails&)>;
+
+// Contains all registered callbacks for browsing data removed notifications.
+CallbackList* g_on_browsing_data_removed_callbacks = nullptr;
+
+// Accessor for |*g_on_browsing_data_removed_callbacks|. Creates a new object
+// the first time so that it always returns a valid object.
+CallbackList* GetOnBrowsingDataRemovedCallbacks() {
+  if (!g_on_browsing_data_removed_callbacks)
+    g_on_browsing_data_removed_callbacks = new CallbackList();
+  return g_on_browsing_data_removed_callbacks;
+}
+
+}  // namespace
+
 bool BrowsingDataRemover::is_removing_ = false;
 
 BrowsingDataRemover::CompletionInhibitor*
@@ -808,13 +826,11 @@ void BrowsingDataRemover::OnKeywordsLoaded() {
 void BrowsingDataRemover::NotifyAndDelete() {
   set_removing(false);
 
-  // Send global notification, then notify any explicit observers.
+  // Notify observers.
   BrowsingDataRemover::NotificationDetails details(delete_begin_, remove_mask_,
       origin_set_mask_);
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_BROWSING_DATA_REMOVED,
-      content::Source<Profile>(profile_),
-      content::Details<BrowsingDataRemover::NotificationDetails>(&details));
+
+  GetOnBrowsingDataRemovedCallbacks()->Notify(details);
 
   FOR_EACH_OBSERVER(Observer, observer_list_, OnBrowsingDataRemoverDone());
 
@@ -1199,3 +1215,10 @@ void BrowsingDataRemover::OnClearedDomainReliabilityMonitor() {
   waiting_for_clear_domain_reliability_monitor_ = false;
   NotifyAndDeleteIfDone();
 }
+
+// static
+BrowsingDataRemover::CallbackSubscription
+    BrowsingDataRemover::RegisterOnBrowsingDataRemovedCallback(
+        const BrowsingDataRemover::Callback& callback) {
+  return GetOnBrowsingDataRemovedCallbacks()->Add(callback);
+}

chrome/browser/browsing_data/browsing_data_remover.h

@@ -152,6 +152,10 @@ class BrowsingDataRemover
     virtual ~Observer() {}
   };
 
+  using Callback = base::Callback<void(const NotificationDetails&)>;
+  using CallbackSubscription = scoped_ptr<
+      base::CallbackList<void(const NotificationDetails&)>::Subscription>;
+
   // The completion inhibitor can artificially delay completion of the browsing
   // data removal process. It is used during testing to simulate scenarios in
   // which the deletion stalls or takes a very long time.
@@ -200,6 +204,12 @@ class BrowsingDataRemover
     completion_inhibitor_ = inhibitor;
   }
 
+  // Add a callback to the list of callbacks to be called during a browsing data
+  // removal event. Returns a subscription object that can be used to
+  // un-register the callback.
+  static CallbackSubscription RegisterOnBrowsingDataRemovedCallback(
+      const Callback& callback);
+
   // Removes the specified items related to browsing for all origins that match
   // the provided |origin_set_mask| (see BrowsingDataHelper::OriginSetMask).
   void Remove(int remove_mask, int origin_set_mask);

chrome/browser/browsing_data/browsing_data_remover_unittest.cc

@@ -20,7 +20,6 @@
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_test_util.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/domain_reliability/service_factory.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
@@ -39,7 +38,6 @@
 #include "content/public/browser/cookie_store_factory.h"
 #include "content/public/browser/dom_storage_context.h"
 #include "content/public/browser/local_storage_usage_info.h"
-#include "content/public/browser/notification_service.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/test/test_browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
@@ -703,13 +701,13 @@ class ClearDomainReliabilityTester {
 
 // Test Class ----------------------------------------------------------------
 
-class BrowsingDataRemoverTest : public testing::Test,
-                                public content::NotificationObserver {
+class BrowsingDataRemoverTest : public testing::Test {
  public:
-  BrowsingDataRemoverTest()
-      : profile_(new TestingProfile()) {
-    registrar_.Add(this, chrome::NOTIFICATION_BROWSING_DATA_REMOVED,
-                   content::Source<Profile>(profile_.get()));
+  BrowsingDataRemoverTest() : profile_(new TestingProfile()) {
+    callback_subscription_ =
+        BrowsingDataRemover::RegisterOnBrowsingDataRemovedCallback(
+            base::Bind(&BrowsingDataRemoverTest::NotifyWithDetails,
+                       base::Unretained(this)));
   }
 
   ~BrowsingDataRemoverTest() override {}
@@ -794,19 +792,15 @@ class BrowsingDataRemoverTest : public testing::Test,
     return storage_partition_removal_data_;
   }
 
-  // content::NotificationObserver implementation.
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override {
-    DCHECK_EQ(type, chrome::NOTIFICATION_BROWSING_DATA_REMOVED);
-
+  // Callback for browsing data removal events.
+  void NotifyWithDetails(
+      const BrowsingDataRemover::NotificationDetails& details) {
     // We're not taking ownership of the details object, but storing a copy of
     // it locally.
-    called_with_details_.reset(new BrowsingDataRemover::NotificationDetails(
-        *content::Details<BrowsingDataRemover::NotificationDetails>(
-            details).ptr()));
+    called_with_details_.reset(
+        new BrowsingDataRemover::NotificationDetails(details));
 
-    registrar_.RemoveAll();
+    callback_subscription_.reset();
   }
 
   MockExtensionSpecialStoragePolicy* CreateMockPolicy() {
@@ -841,8 +835,6 @@ class BrowsingDataRemoverTest : public testing::Test,
   scoped_ptr<BrowsingDataRemover::NotificationDetails> called_with_details_;
 
  private:
-  content::NotificationRegistrar registrar_;
-
   content::TestBrowserThreadBundle thread_bundle_;
   scoped_ptr<TestingProfile> profile_;
 
@@ -852,6 +844,8 @@ class BrowsingDataRemoverTest : public testing::Test,
   scoped_refptr<MockExtensionSpecialStoragePolicy> mock_policy_;
 #endif
 
+  BrowsingDataRemover::CallbackSubscription callback_subscription_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowsingDataRemoverTest);
 };
 

chrome/browser/chrome_notification_types.h

@@ -621,15 +621,6 @@ enum NotificationType {
   // all error UIs should update.
   NOTIFICATION_GLOBAL_ERRORS_CHANGED,
 
-  // BrowsingDataRemover ----------------------------------------------------
-  // Sent on the UI thread after BrowsingDataRemover has removed browsing data
-  // but before it has notified its explicit observers. The source is a
-  // Source<Profile> containing the profile in which browsing data was removed,
-  // and the detail is a BrowsingDataRemover::NotificationDetail containing the
-  // removal mask and the start of the removal timeframe with which
-  // BrowsingDataRemove::Remove was called.
-  NOTIFICATION_BROWSING_DATA_REMOVED,
-
   // The user accepted or dismissed a SSL client authentication request.
   // The source is a Source<net::HttpNetworkSession>.  Details is a
   // (std::pair<net::SSLCertRequestInfo*, net::X509Certificate*>).

chrome/browser/extensions/api/browsing_data/browsing_data_test.cc

@@ -13,14 +13,12 @@
 #include "base/values.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/browsing_data/browsing_data_api.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
-#include "content/public/browser/notification_service.h"
 
 using extension_function_test_utils::RunFunctionAndReturnError;
 using extension_function_test_utils::RunFunctionAndReturnSingleResult;
@@ -42,8 +40,7 @@ const char kRemoveEverythingArguments[] = "[{\"since\": 1000}, {"
     "}]";
 
 
-class ExtensionBrowsingDataTest : public InProcessBrowserTest,
-                                  public content::NotificationObserver {
+class ExtensionBrowsingDataTest : public InProcessBrowserTest {
  public:
   base::Time GetBeginTime() {
     return called_with_details_->removal_begin;
@@ -60,21 +57,19 @@ class ExtensionBrowsingDataTest : public InProcessBrowserTest,
  protected:
   void SetUpOnMainThread() override {
     called_with_details_.reset(new BrowsingDataRemover::NotificationDetails());
-    registrar_.Add(this, chrome::NOTIFICATION_BROWSING_DATA_REMOVED,
-                   content::Source<Profile>(browser()->profile()));
+    callback_subscription_ =
+        BrowsingDataRemover::RegisterOnBrowsingDataRemovedCallback(
+            base::Bind(&ExtensionBrowsingDataTest::NotifyWithDetails,
+                       base::Unretained(this)));
   }
 
-  // content::NotificationObserver implementation.
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override {
-    DCHECK_EQ(type, chrome::NOTIFICATION_BROWSING_DATA_REMOVED);
-
+  // Callback for browsing data removal events.
+  void NotifyWithDetails(
+      const BrowsingDataRemover::NotificationDetails& details) {
     // We're not taking ownership of the details object, but storing a copy of
     // it locally.
-    called_with_details_.reset(new BrowsingDataRemover::NotificationDetails(
-        *content::Details<BrowsingDataRemover::NotificationDetails>(
-            details).ptr()));
+    called_with_details_.reset(
+        new BrowsingDataRemover::NotificationDetails(details));
   }
 
   int GetAsMask(const base::DictionaryValue* dict, std::string path,
@@ -257,7 +252,9 @@ class ExtensionBrowsingDataTest : public InProcessBrowserTest,
 
  private:
   scoped_ptr<BrowsingDataRemover::NotificationDetails> called_with_details_;
-  content::NotificationRegistrar registrar_;
+
+  BrowsingDataRemover::CallbackSubscription callback_subscription_;
+
 };
 
 }  // namespace