Adding chrome::NOTIFICATION_BROWSING_DATA_REMOVED.

After BrowsingDataRemover removes browsing data, it now triggers a
chrome::NOTIFICATION_BROWSING_DATA_REMOVED, which uses the profile
in which data was removed as it's |source|, and a struct containing the
beginning of the removal's timeframe, and the removal mask
(BrowsingDataRemover::NotificationDetail) as it's |details|.

BUG=107202
TEST=BrowsingDataRemover unit tests.


Review URL: http://codereview.chromium.org/8907015

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

chrome/browser/browsing_data_remover.cc

@@ -45,7 +45,7 @@
 #include "content/browser/in_process_webkit/webkit_context.h"
 #include "content/browser/user_metrics.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_source.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/browser/plugin_data_remover.h"
 #include "net/base/cookie_monster.h"
 #include "net/base/net_errors.h"
@@ -65,6 +65,26 @@ DISABLE_RUNNABLE_METHOD_REFCOUNT(BrowsingDataRemover);
 
 bool BrowsingDataRemover::removing_ = false;
 
+BrowsingDataRemover::NotificationDetails::NotificationDetails()
+    : removal_begin(base::Time()),
+      removal_mask(-1) {
+}
+
+BrowsingDataRemover::NotificationDetails::NotificationDetails(
+    const BrowsingDataRemover::NotificationDetails& details)
+    : removal_begin(details.removal_begin),
+      removal_mask(details.removal_mask) {
+}
+
+BrowsingDataRemover::NotificationDetails::NotificationDetails(
+    base::Time removal_begin,
+    int removal_mask)
+    : removal_begin(removal_begin),
+      removal_mask(removal_mask) {
+}
+
+BrowsingDataRemover::NotificationDetails::~NotificationDetails() {}
+
 BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
                                          base::Time delete_begin,
                                          base::Time delete_end)
@@ -84,7 +104,8 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
       waiting_for_clear_networking_history_(false),
       waiting_for_clear_cookies_(false),
       waiting_for_clear_cache_(false),
-      waiting_for_clear_lso_data_(false) {
+      waiting_for_clear_lso_data_(false),
+      remove_mask_(0) {
   DCHECK(profile);
 }
 
@@ -107,7 +128,8 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
       waiting_for_clear_networking_history_(false),
       waiting_for_clear_cookies_(false),
       waiting_for_clear_cache_(false),
-      waiting_for_clear_lso_data_(false) {
+      waiting_for_clear_lso_data_(false),
+      remove_mask_(0) {
   DCHECK(profile);
 }
 
@@ -124,6 +146,7 @@ void BrowsingDataRemover::set_removing(bool removing) {
 void BrowsingDataRemover::Remove(int remove_mask) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   set_removing(true);
+  remove_mask_ = remove_mask;
 
   if (remove_mask & REMOVE_HISTORY) {
     HistoryService* history_service =
@@ -350,7 +373,7 @@ void BrowsingDataRemover::Observe(int type,
   // them to complete before continuing.
   DCHECK(type == chrome::NOTIFICATION_TEMPLATE_URL_SERVICE_LOADED);
   TemplateURLService* model = content::Source<TemplateURLService>(source).ptr();
-  if (model->profile() == profile_->GetOriginalProfile()) {
+  if (model->profile() == profile_) {
     registrar_.RemoveAll();
     model->RemoveAutoGeneratedBetween(delete_begin_, delete_end_);
     NotifyAndDeleteIfDone();
@@ -370,6 +393,14 @@ void BrowsingDataRemover::NotifyAndDeleteIfDone() {
     g_browser_process->net_log()->ClearAllPassivelyCapturedEvents();
 
   set_removing(false);
+
+  // Send global notification, then notify any explicit observers.
+  BrowsingDataRemover::NotificationDetails details(delete_begin_, remove_mask_);
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_BROWSING_DATA_REMOVED,
+      content::Source<Profile>(profile_),
+      content::Details<BrowsingDataRemover::NotificationDetails>(&details));
+
   FOR_EACH_OBSERVER(Observer, observer_list_, OnBrowsingDataRemoverDone());
 
   // History requests aren't happy if you delete yourself from the callback.

chrome/browser/browsing_data_remover.h

@@ -77,6 +77,23 @@ class BrowsingDataRemover : public content::NotificationObserver,
                        REMOVE_LSO_DATA | REMOVE_WEBSQL
   };
 
+  // When BrowsingDataRemover successfully removes data, a notification of type
+  // NOTIFICATION_BROWSING_DATA_REMOVED is triggered with a Details object of
+  // this type.
+  struct NotificationDetails {
+    NotificationDetails();
+    NotificationDetails(const NotificationDetails& details);
+    NotificationDetails(base::Time removal_begin,
+                       int removal_mask);
+    ~NotificationDetails();
+
+    // The beginning of the removal time range.
+    base::Time removal_begin;
+
+    // The removal mask (see the RemoveDataMask enum for details)
+    int removal_mask;
+  };
+
   // Observer is notified when the removal is done. Done means keywords have
   // been deleted, cache cleared and all other tasks scheduled.
   class Observer {
@@ -255,6 +272,9 @@ class BrowsingDataRemover : public content::NotificationObserver,
   int quota_managed_origins_to_delete_count_;
   int quota_managed_storage_types_to_delete_count_;
 
+  // The removal mask for the current removal operation.
+  int remove_mask_;
+
   ObserverList<Observer> observer_list_;
 
   // Used if we need to clear history.

chrome/browser/browsing_data_remover_unittest.cc

@@ -12,9 +12,11 @@
 #include "base/platform_file.h"
 #include "chrome/browser/extensions/mock_extension_special_storage_policy.h"
 #include "chrome/browser/history/history.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service.h"
 #include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/notification_service.h"
 #include "content/test/test_browser_thread.h"
 #include "net/base/cookie_monster.h"
 #include "net/url_request/url_request_context.h"
@@ -120,7 +122,7 @@ class RemoveCookieTester : public BrowsingDataRemoverTester {
     if (cookies == "A=1") {
       get_cookie_success_ = true;
     } else {
-      EXPECT_EQ(cookies, "");
+      EXPECT_EQ("", cookies);
       get_cookie_success_ = false;
     }
     Notify();
@@ -234,7 +236,8 @@ class RemoveQuotaManagedDataTester : public BrowsingDataRemoverTester {
 
 // Test Class ----------------------------------------------------------------
 
-class BrowsingDataRemoverTest : public testing::Test {
+class BrowsingDataRemoverTest : public testing::Test,
+                                public content::NotificationObserver {
  public:
   BrowsingDataRemoverTest()
       : ui_thread_(BrowserThread::UI, &message_loop_),
@@ -243,6 +246,8 @@ class BrowsingDataRemoverTest : public testing::Test {
         file_thread_(BrowserThread::FILE, &message_loop_),
         io_thread_(BrowserThread::IO, &message_loop_),
         profile_(new TestingProfile()) {
+    registrar_.Add(this, chrome::NOTIFICATION_BROWSING_DATA_REMOVED,
+                   content::Source<Profile>(profile_.get()));
   }
 
   virtual ~BrowsingDataRemoverTest() {
@@ -266,6 +271,8 @@ class BrowsingDataRemoverTest : public testing::Test {
         base::Time::Now() + base::TimeDelta::FromMilliseconds(10));
     remover->AddObserver(tester);
 
+    called_with_details_.reset(new BrowsingDataRemover::NotificationDetails());
+
     // BrowsingDataRemover deletes itself when it completes.
     remover->Remove(remove_mask);
     tester->BlockUntilNotified();
@@ -275,6 +282,14 @@ class BrowsingDataRemoverTest : public testing::Test {
     return profile_.get();
   }
 
+  base::Time GetBeginTime() {
+    return called_with_details_->removal_begin;
+  }
+
+  int GetRemovalMask() {
+    return called_with_details_->removal_mask;
+  }
+
   quota::MockQuotaManager* GetMockManager() {
     if (profile_->GetQuotaManager() == NULL) {
       profile_->SetQuotaManager(new quota::MockQuotaManager(
@@ -287,7 +302,25 @@ class BrowsingDataRemoverTest : public testing::Test {
     return (quota::MockQuotaManager*) profile_->GetQuotaManager();
   }
 
+  // content::NotificationObserver implementation.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE {
+    DCHECK_EQ(type, chrome::NOTIFICATION_BROWSING_DATA_REMOVED);
+
+    // 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()));
+
+    registrar_.RemoveAll();
+  }
+
  private:
+  scoped_ptr<BrowsingDataRemover::NotificationDetails> called_with_details_;
+  content::NotificationRegistrar registrar_;
+
   // message_loop_, as well as all the threads associated with it must be
   // defined before profile_ to prevent explosions. Oh how I love C++.
   MessageLoopForUI message_loop_;
@@ -313,6 +346,7 @@ TEST_F(BrowsingDataRemoverTest, RemoveCookieForever) {
   BlockUntilBrowsingDataRemoved(BrowsingDataRemover::EVERYTHING,
       BrowsingDataRemover::REMOVE_COOKIES, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_COOKIES, GetRemovalMask());
   EXPECT_FALSE(tester->ContainsCookie());
 }
 
@@ -326,6 +360,7 @@ TEST_F(BrowsingDataRemoverTest, RemoveHistoryForever) {
   BlockUntilBrowsingDataRemoved(BrowsingDataRemover::EVERYTHING,
       BrowsingDataRemover::REMOVE_HISTORY, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_HISTORY, GetRemovalMask());
   EXPECT_FALSE(tester->HistoryContainsURL(kOrigin1));
 }
 
@@ -343,6 +378,7 @@ TEST_F(BrowsingDataRemoverTest, RemoveHistoryForLastHour) {
   BlockUntilBrowsingDataRemoved(BrowsingDataRemover::LAST_HOUR,
       BrowsingDataRemover::REMOVE_HISTORY, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_HISTORY, GetRemovalMask());
   EXPECT_FALSE(tester->HistoryContainsURL(kOrigin1));
   EXPECT_TRUE(tester->HistoryContainsURL(kOrigin2));
 }
@@ -356,6 +392,8 @@ TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedDataForeverBoth) {
       BrowsingDataRemover::REMOVE_SITE_DATA &
       ~BrowsingDataRemover::REMOVE_LSO_DATA, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_SITE_DATA &
+      ~BrowsingDataRemover::REMOVE_LSO_DATA, GetRemovalMask());
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin1,
       quota::kStorageTypeTemporary));
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin2,
@@ -379,6 +417,8 @@ TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedDataForeverOnlyTemporary) {
       BrowsingDataRemover::REMOVE_SITE_DATA &
       ~BrowsingDataRemover::REMOVE_LSO_DATA, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_SITE_DATA &
+      ~BrowsingDataRemover::REMOVE_LSO_DATA, GetRemovalMask());
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin1,
       quota::kStorageTypeTemporary));
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin2,
@@ -402,6 +442,8 @@ TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedDataForeverOnlyPersistent) {
       BrowsingDataRemover::REMOVE_SITE_DATA &
       ~BrowsingDataRemover::REMOVE_LSO_DATA, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_SITE_DATA &
+      ~BrowsingDataRemover::REMOVE_LSO_DATA, GetRemovalMask());
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin1,
       quota::kStorageTypeTemporary));
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin2,
@@ -425,6 +467,8 @@ TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedDataForeverNeither) {
       BrowsingDataRemover::REMOVE_SITE_DATA &
       ~BrowsingDataRemover::REMOVE_LSO_DATA, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_SITE_DATA &
+      ~BrowsingDataRemover::REMOVE_LSO_DATA, GetRemovalMask());
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin1,
       quota::kStorageTypeTemporary));
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin2,
@@ -448,6 +492,8 @@ TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedDataForLastHour) {
       BrowsingDataRemover::REMOVE_SITE_DATA &
       ~BrowsingDataRemover::REMOVE_LSO_DATA, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_SITE_DATA &
+      ~BrowsingDataRemover::REMOVE_LSO_DATA, GetRemovalMask());
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin1,
       quota::kStorageTypeTemporary));
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin2,
@@ -471,6 +517,8 @@ TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedDataForLastWeek) {
       BrowsingDataRemover::REMOVE_SITE_DATA &
       ~BrowsingDataRemover::REMOVE_LSO_DATA, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_SITE_DATA &
+      ~BrowsingDataRemover::REMOVE_LSO_DATA, GetRemovalMask());
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin1,
       quota::kStorageTypeTemporary));
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin2,
@@ -500,6 +548,8 @@ TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedUnprotectedOrigins) {
       BrowsingDataRemover::REMOVE_SITE_DATA &
       ~BrowsingDataRemover::REMOVE_LSO_DATA, tester.get());
 
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_SITE_DATA &
+      ~BrowsingDataRemover::REMOVE_LSO_DATA, GetRemovalMask());
   EXPECT_TRUE(GetMockManager()->OriginHasData(kOrigin1,
       quota::kStorageTypeTemporary));
   EXPECT_FALSE(GetMockManager()->OriginHasData(kOrigin2,

chrome/common/chrome_notification_types.h

@@ -984,6 +984,15 @@ 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,
+
   // Note:-
   // Currently only Content and Chrome define and use notifications.
   // Custom notifications not belonging to Content and Chrome should start