QuotaManager::DeleteOriginData now allows deletion of specific QuotaClients

webkit/quota/quota_manager.cc

Index: webkit/quota/quota_manager.cc
diff --git a/webkit/quota/quota_manager.cc b/webkit/quota/quota_manager.cc
index dc2b27b3933d7315f5b7d1bd72d7fae979e31d1a..dfdee201fea399f7e483b667f0531e6138beb225 100644
--- a/webkit/quota/quota_manager.cc
+++ b/webkit/quota/quota_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -461,12 +461,15 @@ class QuotaManager::OriginDataDeleter : public QuotaTask {
   OriginDataDeleter(QuotaManager* manager,
                     const GURL& origin,
                     StorageType type,
+                    int quota_client_mask,
                     const StatusCallback& callback)
       : QuotaTask(manager),
         origin_(origin),
         type_(type),
+        quota_client_mask_(quota_client_mask),
         error_count_(0),
         remaining_clients_(-1),
+        skipped_clients_(0),
         callback_(callback),
         weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {}
 
@@ -476,16 +479,25 @@ class QuotaManager::OriginDataDeleter : public QuotaTask {
     remaining_clients_ = manager()->clients_.size();
     for (QuotaClientList::iterator iter = manager()->clients_.begin();
          iter != manager()->clients_.end(); ++iter) {
-      (*iter)->DeleteOriginData(
-          origin_, type_,
-          base::Bind(&OriginDataDeleter::DidDeleteOriginData,
-                     weak_factory_.GetWeakPtr()));
+      if (quota_client_mask_ & (*iter)->id() ||
+          quota_client_mask_ == QuotaClient::kAllClientsMask) {

[michaeln, 2012/01/18 23:07:48]

is the second condition needed or will the first eval to true in all cases where
the second would?

[Mike West, 2012/01/19 13:09:11]

In an earlier review, Kinuko suggested using -1 for the `kAllClientsMask` to
reduce the potential that the "all clients" mask falls out of sync with the
enum. Doing that requires the second check.

[kinuko, 2012/01/19 13:53:11]

If quota_client_mask_ is kAllClientsMask (-1) the first check always pass,
doesn't it?

[Mike West, 2012/01/19 16:51:00]

... I need to go back to bitwise-school. You're right. I'm dropping the second
test. :)

+        (*iter)->DeleteOriginData(
+            origin_, type_,
+            base::Bind(&OriginDataDeleter::DidDeleteOriginData,
+                       weak_factory_.GetWeakPtr()));
+      } else {
+        ++skipped_clients_;
+        if (--remaining_clients_ == 0)
+          CallCompleted();
+      }
     }
   }
 
   virtual void Completed() OVERRIDE {
     if (error_count_ == 0) {
-      manager()->DeleteOriginFromDatabase(origin_, type_);
+      // Only remove the entire origin if we didn't skip any client types.
+      if (skipped_clients_ == 0)
+        manager()->DeleteOriginFromDatabase(origin_, type_);
       callback_.Run(kQuotaStatusOk);
     } else {
       callback_.Run(kQuotaErrorInvalidModification);
@@ -514,8 +526,10 @@ class QuotaManager::OriginDataDeleter : public QuotaTask {
 
   GURL origin_;
   StorageType type_;
+  int quota_client_mask_;
   int error_count_;
   int remaining_clients_;
+  int skipped_clients_;
   StatusCallback callback_;
 
   base::WeakPtrFactory<OriginDataDeleter> weak_factory_;
@@ -527,10 +541,12 @@ class QuotaManager::HostDataDeleter : public QuotaTask {
   HostDataDeleter(QuotaManager* manager,
                   const std::string& host,
                   StorageType type,
+                  int quota_client_mask,
                   const StatusCallback& callback)
       : QuotaTask(manager),
         host_(host),
         type_(type),
+        quota_client_mask_(quota_client_mask),
         error_count_(0),
         remaining_clients_(-1),
         remaining_deleters_(-1),
@@ -584,7 +600,7 @@ class QuotaManager::HostDataDeleter : public QuotaTask {
          ++p) {
       OriginDataDeleter* deleter =
           new OriginDataDeleter(
-              manager(), *p, type_,
+              manager(), *p, type_, quota_client_mask_,
               base::Bind(&HostDataDeleter::DidDeleteOriginData,
                          weak_factory_.GetWeakPtr()));
       deleter->Start();
@@ -607,6 +623,7 @@ class QuotaManager::HostDataDeleter : public QuotaTask {
 
   std::string host_;
   StorageType type_;
+  int quota_client_mask_;
   std::set<GURL> origins_;
   int error_count_;
   int remaining_clients_;
@@ -1313,21 +1330,24 @@ void QuotaManager::NotifyOriginNoLongerInUse(const GURL& origin) {
 }
 
 void QuotaManager::DeleteOriginData(
-    const GURL& origin, StorageType type, const StatusCallback& callback) {
+    const GURL& origin, StorageType type, int quota_client_mask,
+    const StatusCallback& callback) {
   LazyInitialize();
 
-  if (origin.is_empty() || clients_.empty()) {
+  if (origin.is_empty() || clients_.empty() ||
+      quota_client_mask == QuotaClient::kUnknown) {
     callback.Run(kQuotaStatusOk);
     return;
   }
 
   OriginDataDeleter* deleter =
-      new OriginDataDeleter(this, origin, type, callback);
+      new OriginDataDeleter(this, origin, type, quota_client_mask, callback);
   deleter->Start();
 }
 
 void QuotaManager::DeleteHostData(const std::string& host,
                                   StorageType type,
+                                  int quota_client_mask,
                                   const StatusCallback& callback) {
 
   LazyInitialize();
@@ -1338,7 +1358,7 @@ void QuotaManager::DeleteHostData(const std::string& host,
   }
 
   HostDataDeleter* deleter =
-      new HostDataDeleter(this, host, type, callback);
+      new HostDataDeleter(this, host, type, quota_client_mask, callback);
   deleter->Start();
 }
 
@@ -1527,7 +1547,8 @@ void QuotaManager::EvictOriginData(
   eviction_context_.evicted_type = type;
   eviction_context_.evict_origin_data_callback = callback;
 
-  DeleteOriginData(origin, type,
+
+  DeleteOriginData(origin, type, QuotaClient::kAllClientsMask,
       base::Bind(&QuotaManager::DidOriginDataEvicted,
                  weak_factory_.GetWeakPtr()));
 }