Fix a race in BrowsingDataRemover.

We were accesing a flag from multiple threads, which cloud lead to BrowsingDataRemover::NotifyAndDeleteIfDone() to be called multiple times, which in turn would cause a DCHECK().

BUG=None
TEST=No DCHECK on cancelling enterprise enrollment on CrOS.


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

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

2 files changed, 19 insertions, 13 deletions

diff --git a/chrome/browser/browsing_data_remover.cc b/chrome/browser/browsing_data_remover.cc
index 7c8cfd3..76a2305 100644
--- a/chrome/browser/browsing_data_remover.cc
+++ b/chrome/browser/browsing_data_remover.cc
@@ -459,7 +459,6 @@ void BrowsingDataRemover::DoClearCache(int rv) {
 
 void BrowsingDataRemover::ClearQuotaManagedDataOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK(waiting_for_clear_quota_managed_data_);
 
   // Ask the QuotaManager for all origins with temporary quota modified within
   // the user-specified timeframe, and deal with the resulting set in
@@ -503,9 +502,7 @@ void BrowsingDataRemover::OnGotQuotaManagedOrigins(
   }
 
   --quota_managed_storage_types_to_delete_count_;
-  if (quota_managed_storage_types_to_delete_count_ == 0 &&
-      quota_managed_origins_to_delete_count_ == 0)
-    CheckQuotaManagedDataDeletionStatus();
+  CheckQuotaManagedDataDeletionStatus();
 }
 
 void BrowsingDataRemover::OnQuotaManagedOriginDeletion(
@@ -518,22 +515,26 @@ void BrowsingDataRemover::OnQuotaManagedOriginDeletion(
   }
 
   --quota_managed_origins_to_delete_count_;
-  if (quota_managed_storage_types_to_delete_count_ == 0 &&
-      quota_managed_origins_to_delete_count_ == 0)
-    CheckQuotaManagedDataDeletionStatus();
+  CheckQuotaManagedDataDeletionStatus();
 }
 
 void BrowsingDataRemover::CheckQuotaManagedDataDeletionStatus() {
-  DCHECK_EQ(quota_managed_origins_to_delete_count_, 0);
-  DCHECK_EQ(quota_managed_storage_types_to_delete_count_, 0);
-  DCHECK(waiting_for_clear_quota_managed_data_);
+  if (quota_managed_storage_types_to_delete_count_ != 0 ||
+      quota_managed_origins_to_delete_count_ != 0) {
+    return;
+  }
 
-  waiting_for_clear_quota_managed_data_ = false;
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(
           this,
-          &BrowsingDataRemover::NotifyAndDeleteIfDone));
+          &BrowsingDataRemover::OnQuotaManagedDataDeleted));
+}
+
+void BrowsingDataRemover::OnQuotaManagedDataDeleted() {
+  DCHECK(waiting_for_clear_quota_managed_data_);
+  waiting_for_clear_quota_managed_data_ = false;
+  NotifyAndDeleteIfDone();
 }
 
 void BrowsingDataRemover::OnWaitableEventSignaled(
diff --git a/chrome/browser/browsing_data_remover.h b/chrome/browser/browsing_data_remover.h
index 115c0d5..48c99da 100644
--- a/chrome/browser/browsing_data_remover.h
+++ b/chrome/browser/browsing_data_remover.h
@@ -164,9 +164,13 @@ class BrowsingDataRemover : public NotificationObserver,
 
   // Called to check whether all temporary and persistent origin data that
   // should be deleted has been deleted. If everything's good to go, invokes
-  // NotifyAndDeleteIfDone on the UI thread.
+  // OnQuotaManagedDataDeleted on the UI thread.
   void CheckQuotaManagedDataDeletionStatus();
 
+  // Completion handler that runs on the UI thread once persistent data has been
+  // deleted. Updates the waiting flag and invokes NotifyAndDeleteIfDone.
+  void OnQuotaManagedDataDeleted();
+
   // Callback when Cookies has been deleted. Invokes NotifyAndDeleteIfDone.
   void OnClearedCookies(int num_deleted);
 
@@ -225,6 +229,7 @@ class BrowsingDataRemover : public NotificationObserver,
   base::WaitableEventWatcher watcher_;
 
   // True if we're waiting for various data to be deleted.
+  // These may only be accessed from UI thread in order to avoid races!
   bool waiting_for_clear_history_;
   bool waiting_for_clear_quota_managed_data_;
   bool waiting_for_clear_networking_history_;