Teaching BrowsingDataRemover how to delete application data.

chrome/browser/browsing_data_remover.cc

Index: chrome/browser/browsing_data_remover.cc
diff --git a/chrome/browser/browsing_data_remover.cc b/chrome/browser/browsing_data_remover.cc
index fb1736f08eb71bfa443d056428da3935ffbdd7fb..438de1e87bd07695a048a857abd5a6f2bb753979 100644
--- a/chrome/browser/browsing_data_remover.cc
+++ b/chrome/browser/browsing_data_remover.cc
@@ -61,6 +61,7 @@
 #include "net/url_request/url_request_context_getter.h"
 #include "webkit/quota/quota_manager.h"
 #include "webkit/quota/quota_types.h"
+#include "webkit/quota/special_storage_policy.h"
 
 using content::BrowserContext;
 using content::BrowserThread;
@@ -90,11 +91,13 @@ BrowsingDataRemover::NotificationDetails::NotificationDetails(
 
 BrowsingDataRemover::NotificationDetails::~NotificationDetails() {}
 
+// TODO(mkwst): We should have one constructor, not two. http://crbug.com/130732
 BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
                                          base::Time delete_begin,
                                          base::Time delete_end)
     : profile_(profile),
       quota_manager_(NULL),
+      dom_storage_context_(NULL),
       special_storage_policy_(profile->GetExtensionSpecialStoragePolicy()),
       delete_begin_(delete_begin),
       delete_end_(delete_end),
@@ -106,6 +109,7 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
       waiting_for_clear_cache_(false),
       waiting_for_clear_cookies_count_(0),
       waiting_for_clear_history_(false),
+      waiting_for_clear_local_storage_(false),
       waiting_for_clear_networking_history_(false),
       waiting_for_clear_server_bound_certs_(false),
       waiting_for_clear_plugin_data_(false),
@@ -113,7 +117,7 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
       waiting_for_clear_content_licenses_(false),
       remove_mask_(0),
       remove_origin_(GURL()),
-      remove_protected_(false) {
+      origin_set_mask_(0) {
   DCHECK(profile);
 }
 
@@ -122,6 +126,7 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
                                          base::Time delete_end)
     : profile_(profile),
       quota_manager_(NULL),
+      dom_storage_context_(NULL),
       special_storage_policy_(profile->GetExtensionSpecialStoragePolicy()),
       delete_begin_(CalculateBeginDeleteTime(time_period)),
       delete_end_(delete_end),
@@ -133,6 +138,7 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
       waiting_for_clear_cache_(false),
       waiting_for_clear_cookies_count_(0),
       waiting_for_clear_history_(false),
+      waiting_for_clear_local_storage_(false),
       waiting_for_clear_networking_history_(false),
       waiting_for_clear_server_bound_certs_(false),
       waiting_for_clear_plugin_data_(false),
@@ -140,7 +146,7 @@ BrowsingDataRemover::BrowsingDataRemover(Profile* profile,
       waiting_for_clear_content_licenses_(false),
       remove_mask_(0),
       remove_origin_(GURL()),
-      remove_protected_(false) {
+      origin_set_mask_(0) {
   DCHECK(profile);
 }
 
@@ -169,18 +175,18 @@ int BrowsingDataRemover::GenerateQuotaClientMask(int remove_mask) {
   return quota_client_mask;
 }
 
-void BrowsingDataRemover::Remove(int remove_mask) {
-  RemoveImpl(remove_mask, GURL(), false);
+void BrowsingDataRemover::Remove(int remove_mask, int origin_set_mask) {
+  RemoveImpl(remove_mask, GURL(), origin_set_mask);
 }
 
 void BrowsingDataRemover::RemoveImpl(int remove_mask,
                                      const GURL& origin,
-                                     bool remove_protected_origins) {
+                                     int origin_set_mask) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   set_removing(true);
   remove_mask_ = remove_mask;
   remove_origin_ = origin;
-  remove_protected_ = remove_protected_origins;
+  origin_set_mask_ = origin_set_mask;
 
   if (remove_mask & REMOVE_HISTORY) {
     HistoryService* history_service =
@@ -311,8 +317,13 @@ void BrowsingDataRemover::RemoveImpl(int remove_mask,
   }
 
   if (remove_mask & REMOVE_LOCAL_STORAGE) {
-    BrowserContext::GetDOMStorageContext(profile_)->DeleteDataModifiedSince(
-        delete_begin_);
+    waiting_for_clear_local_storage_ = true;
+    if (!dom_storage_context_)
+      dom_storage_context_ = BrowserContext::GetDOMStorageContext(profile_);
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&BrowsingDataRemover::ClearLocalStorageOnIOThread,
+                   base::Unretained(this)));
   }
 
   if (remove_mask & REMOVE_INDEXEDDB || remove_mask & REMOVE_WEBSQL ||
@@ -596,6 +607,43 @@ void BrowsingDataRemover::DoClearCache(int rv) {
   }
 }
 
+void BrowsingDataRemover::ClearLocalStorageOnIOThread() {

[michaeln, 2012/06/01 20:59:36]

it'd be fine to do this directly on the UI thread too w/o the extra thread hops

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(waiting_for_clear_local_storage_);
+
+  dom_storage_context_->GetUsageInfo(
+      base::Bind(&BrowsingDataRemover::OnGotLocalStorageUsageInfo,
+                 base::Unretained(this)));
+}
+
+void BrowsingDataRemover::OnGotLocalStorageUsageInfo(
+    const std::vector<content::DOMStorageContext::UsageInfo>& infos) {

[michaeln, 2012/06/01 20:59:36]

fyi: this type is changing in the other interface CL to...
   dom_storage::DomStorageContext::UsageInfo
see https://chromiumcodereview.appspot.com/10449103/

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(waiting_for_clear_local_storage_);
+
+  for (size_t i = 0; i < infos.size(); ++i) {
+    if (!BrowsingDataHelper::DoesOriginMatchMask(infos[i].origin,
+                                                 origin_set_mask_,
+                                                 special_storage_policy_))
+      continue;
+
+    if (infos[i].last_modified >= delete_begin_ &&
+        infos[i].last_modified <= delete_end_)
+      dom_storage_context_->DeleteOrigin(infos[i].origin);
+  }
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&BrowsingDataRemover::OnLocalStorageCleared,
+                 base::Unretained(this)));
+}
+
+void BrowsingDataRemover::OnLocalStorageCleared() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(waiting_for_clear_local_storage_);
+  waiting_for_clear_local_storage_ = false;
+  NotifyAndDeleteIfDone();
+}
+
 void BrowsingDataRemover::ClearQuotaManagedDataOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
@@ -603,24 +651,24 @@ void BrowsingDataRemover::ClearQuotaManagedDataOnIOThread() {
   // the user-specified timeframe, and deal with the resulting set in
   // OnGotQuotaManagedOrigins().
   quota_managed_origins_to_delete_count_ = 0;
-  quota_managed_storage_types_to_delete_count_ = 2;
-
-  if (delete_begin_ == base::Time()) {
-    // If we're deleting since the beginning of time, ask the QuotaManager for
-    // all origins with persistent quota modified within the user-specified
-    // timeframe, and deal with the resulting set in
-    // OnGotPersistentQuotaManagedOrigins.
+  quota_managed_storage_types_to_delete_count_ = 0;
+
+  if (delete_begin_ == base::Time() ||
+      origin_set_mask_ &
+          (BrowsingDataHelper::PROTECTED_WEB | BrowsingDataHelper::EXTENSION)) {
+    // If we're deleting since the beginning of time, or we're removing
+    // protected origins, then ask the QuotaManager for all origins with
+    // persistent quota modified within the user-specified timeframe, and deal
+    // with the resulting set in OnGotQuotaManagedOrigins.
+    ++quota_managed_storage_types_to_delete_count_;
     quota_manager_->GetOriginsModifiedSince(
         quota::kStorageTypePersistent, delete_begin_,
         base::Bind(&BrowsingDataRemover::OnGotQuotaManagedOrigins,
                    base::Unretained(this)));
-  } else {
-    // Otherwise, we don't need to deal with persistent storage.
-    --quota_managed_storage_types_to_delete_count_;
   }
 
-  // Do the same for temporary quota, regardless, passing the resulting set into
-  // OnGotTemporaryQuotaManagedOrigins.
+  // Do the same for temporary quota.
+  ++quota_managed_storage_types_to_delete_count_;
   quota_manager_->GetOriginsModifiedSince(
       quota::kStorageTypeTemporary, delete_begin_,
       base::Bind(&BrowsingDataRemover::OnGotQuotaManagedOrigins,
@@ -631,15 +679,18 @@ void BrowsingDataRemover::OnGotQuotaManagedOrigins(
     const std::set<GURL>& origins, quota::StorageType type) {
   DCHECK_GT(quota_managed_storage_types_to_delete_count_, 0);
   // Walk through the origins passed in, delete quota of |type| from each that
-  // isn't protected.
+  // matches the |origin_set_mask_|.
   std::set<GURL>::const_iterator origin;
   for (origin = origins.begin(); origin != origins.end(); ++origin) {
-    if (!BrowsingDataHelper::IsWebScheme(origin->scheme()))
-      continue;
-    if (special_storage_policy_->IsStorageProtected(origin->GetOrigin()))
-      continue;
+    // TODO(mkwst): Clean this up, it's slow. http://crbug.com/130746

[michaeln, 2012/06/01 20:59:36]

the bug refers to memory alignment?

[Mike West, 2012/06/04 06:42:08]

No, simpler: If the caller wants to delete all data from a single origin, I'm
currently grabbing all the origins, and walking through them to find the one I'm
looking for. This can be vastly improved by just deleting that origin's data
without grabbing data for all other origins.

     if (!remove_origin_.is_empty() && remove_origin_ != origin->GetOrigin())
       continue;
+
+    if (!BrowsingDataHelper::DoesOriginMatchMask(origin->GetOrigin(),
+                                                 origin_set_mask_,
+                                                 special_storage_policy_))
+      continue;
+
     ++quota_managed_origins_to_delete_count_;
     quota_manager_->DeleteOriginData(
         origin->GetOrigin(), type,