RefCounted types should not have public destructors, webkit/

BUG=123295
TEST=none


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

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

7 files changed, 128 insertions, 47 deletions

diff --git a/webkit/quota/mock_quota_manager.cc b/webkit/quota/mock_quota_manager.cc
index bac3fcc..595c43a 100644
--- a/webkit/quota/mock_quota_manager.cc
+++ b/webkit/quota/mock_quota_manager.cc
@@ -27,9 +27,13 @@ class MockQuotaManager::GetModifiedSinceTask : public QuotaThreadTask {
       : QuotaThreadTask(manager, manager->io_thread_),
         origins_(origins),
         type_(type),
-        callback_(callback) {}
+        callback_(callback) {
+  }
 
  protected:
+  virtual ~GetModifiedSinceTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {}
 
   virtual void Completed() OVERRIDE {
@@ -53,9 +57,13 @@ class MockQuotaManager::DeleteOriginDataTask : public QuotaThreadTask {
   DeleteOriginDataTask(MockQuotaManager* manager,
                        const StatusCallback& callback)
       : QuotaThreadTask(manager, manager->io_thread_),
-        callback_(callback) {}
+        callback_(callback) {
+  }
 
  protected:
+  virtual ~DeleteOriginDataTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {}
 
   virtual void Completed() OVERRIDE {
@@ -95,8 +103,6 @@ MockQuotaManager::MockQuotaManager(
         special_storage_policy) {
 }
 
-MockQuotaManager::~MockQuotaManager() {}
-
 bool MockQuotaManager::AddOrigin(
     const GURL& origin,
     StorageType type,
@@ -155,4 +161,6 @@ void MockQuotaManager::DeleteOriginData(
   make_scoped_refptr(new DeleteOriginDataTask(this, callback))->Start();
 }
 
+MockQuotaManager::~MockQuotaManager() {}
+
 }  // namespace quota
diff --git a/webkit/quota/mock_quota_manager.h b/webkit/quota/mock_quota_manager.h
index 317c476..dac1eff 100644
--- a/webkit/quota/mock_quota_manager.h
+++ b/webkit/quota/mock_quota_manager.h
@@ -47,8 +47,6 @@ class MockQuotaManager : public QuotaManager {
                    base::MessageLoopProxy* db_thread,
                    SpecialStoragePolicy* special_storage_policy);
 
-  virtual ~MockQuotaManager();
-
   // Adds an origin to the canned list that will be searched through via
   // GetOriginsModifiedSince. The caller must provide |quota_client_mask|
   // which specifies the types of QuotaClients this canned origin contains
@@ -84,6 +82,9 @@ class MockQuotaManager : public QuotaManager {
                                 int quota_client_mask,
                                 const StatusCallback& callback) OVERRIDE;
 
+ protected:
+  virtual ~MockQuotaManager();
+
  private:
   class GetModifiedSinceTask;
   class DeleteOriginDataTask;
diff --git a/webkit/quota/mock_special_storage_policy.cc b/webkit/quota/mock_special_storage_policy.cc
index c7fdb8f..b8203a3 100644
--- a/webkit/quota/mock_special_storage_policy.cc
+++ b/webkit/quota/mock_special_storage_policy.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.
 
@@ -7,9 +7,8 @@
 namespace quota {
 
 MockSpecialStoragePolicy::MockSpecialStoragePolicy()
-    : all_unlimited_(false) {}
-
-MockSpecialStoragePolicy::~MockSpecialStoragePolicy() {}
+    : all_unlimited_(false) {
+}
 
 bool MockSpecialStoragePolicy::IsStorageProtected(const GURL& origin) {
   return protected_.find(origin) != protected_.end();
@@ -33,4 +32,6 @@ bool MockSpecialStoragePolicy::HasSessionOnlyOrigins() {
   return !session_only_.empty();
 }
 
+MockSpecialStoragePolicy::~MockSpecialStoragePolicy() {}
+
 }  // namespace quota
diff --git a/webkit/quota/mock_special_storage_policy.h b/webkit/quota/mock_special_storage_policy.h
index c3b5d85..1ef6b7fc 100644
--- a/webkit/quota/mock_special_storage_policy.h
+++ b/webkit/quota/mock_special_storage_policy.h
@@ -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.
 
@@ -16,7 +16,6 @@ namespace quota {
 class MockSpecialStoragePolicy : public quota::SpecialStoragePolicy {
  public:
   MockSpecialStoragePolicy();
-  virtual ~MockSpecialStoragePolicy();
 
   virtual bool IsStorageProtected(const GURL& origin) OVERRIDE;
   virtual bool IsStorageUnlimited(const GURL& origin) OVERRIDE;
@@ -56,6 +55,9 @@ class MockSpecialStoragePolicy : public quota::SpecialStoragePolicy {
     SpecialStoragePolicy::NotifyObservers();
   }
 
+ protected:
+  virtual ~MockSpecialStoragePolicy();
+
  private:
   std::set<GURL> protected_;
   std::set<GURL> unlimited_;
diff --git a/webkit/quota/quota_database_unittest.cc b/webkit/quota/quota_database_unittest.cc
index a653e32..bded97f 100644
--- a/webkit/quota/quota_database_unittest.cc
+++ b/webkit/quota/quota_database_unittest.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.
 
@@ -26,11 +26,14 @@ const base::Time kZeroTime;
 
 class TestErrorDelegate : public sql::ErrorDelegate {
  public:
-  virtual ~TestErrorDelegate() { }
-  virtual int OnError(
-      int error, sql::Connection* connection, sql::Statement* stmt) {
+  virtual int OnError(int error,
+                      sql::Connection* connection,
+                      sql::Statement* stmt) OVERRIDE {
     return error;
   }
+
+ protected:
+  virtual ~TestErrorDelegate() {}
 };
 
 }  // namespace
diff --git a/webkit/quota/quota_manager.cc b/webkit/quota/quota_manager.cc
index 640de20..031aa9d 100644
--- a/webkit/quota/quota_manager.cc
+++ b/webkit/quota/quota_manager.cc
@@ -644,8 +644,11 @@ class QuotaManager::DatabaseTaskBase : public QuotaThreadTask {
   }
 
  protected:
+  virtual ~DatabaseTaskBase() {}
+
   virtual void DatabaseTaskCompleted() = 0;
 
+  // QuotaThreadTask:
   virtual void Completed() OVERRIDE {
     manager_->db_disabled_ = db_disabled_;
     DatabaseTaskCompleted();
@@ -674,6 +677,9 @@ class QuotaManager::InitializeTask : public QuotaManager::DatabaseTaskBase {
   }
 
  protected:
+  virtual ~InitializeTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     // See if we have overriding temporary quota configuration.
     database()->GetQuotaConfigValue(QuotaDatabase::kTemporaryQuotaOverrideKey,
@@ -682,6 +688,7 @@ class QuotaManager::InitializeTask : public QuotaManager::DatabaseTaskBase {
                                     &desired_available_space_);
   }
 
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {
     manager()->temporary_quota_override_ = temporary_quota_override_;
     manager()->desired_available_space_ = desired_available_space_;
@@ -706,6 +713,9 @@ class QuotaManager::UpdateTemporaryQuotaOverrideTask
         callback_(callback) {}
 
  protected:
+  virtual ~UpdateTemporaryQuotaOverrideTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->SetQuotaConfigValue(
             QuotaDatabase::kTemporaryQuotaOverrideKey, new_quota_)) {
@@ -715,6 +725,7 @@ class QuotaManager::UpdateTemporaryQuotaOverrideTask
     }
   }
 
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {
     if (!db_disabled()) {
       manager()->temporary_quota_override_ = new_quota_;
@@ -748,15 +759,22 @@ class QuotaManager::GetPersistentHostQuotaTask
         quota_(-1),
         callback_(callback) {
   }
+
  protected:
+  virtual ~GetPersistentHostQuotaTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->GetHostQuota(host_, kStorageTypePersistent, &quota_))
       quota_ = 0;
   }
+
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {
     callback_.Run(kQuotaStatusOk,
                   host_, kStorageTypePersistent, quota_);
   }
+
  private:
   std::string host_;
   int64 quota_;
@@ -777,7 +795,11 @@ class QuotaManager::UpdatePersistentHostQuotaTask
         callback_(callback) {
     DCHECK_GE(new_quota_, 0);
   }
+
  protected:
+  virtual ~UpdatePersistentHostQuotaTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->SetHostQuota(host_, kStorageTypePersistent, new_quota_)) {
       set_db_disabled(true);
@@ -785,15 +807,16 @@ class QuotaManager::UpdatePersistentHostQuotaTask
     }
   }
 
+  virtual void Aborted() OVERRIDE {
+    callback_.Reset();
+  }
+
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {
     callback_.Run(db_disabled() ? kQuotaErrorInvalidAccess : kQuotaStatusOk,
                   host_, kStorageTypePersistent, new_quota_);
   }
 
-  virtual void Aborted() OVERRIDE {
-    callback_.Reset();
-  }
-
  private:
   std::string host_;
   int64 new_quota_;
@@ -828,19 +851,23 @@ class QuotaManager::GetLRUOriginTask
   }
 
  protected:
+  virtual ~GetLRUOriginTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     database()->GetLRUOrigin(
         type_, exceptions_, special_storage_policy_, &url_);
   }
 
-  virtual void DatabaseTaskCompleted() OVERRIDE {
-    callback_.Run(url_);
-  }
-
   virtual void Aborted() OVERRIDE {
     callback_.Reset();
   }
 
+  // DatabaseTaskBase:
+  virtual void DatabaseTaskCompleted() OVERRIDE {
+    callback_.Run(url_);
+  }
+
  private:
   StorageType type_;
   std::set<GURL> exceptions_;
@@ -861,11 +888,16 @@ class QuotaManager::DeleteOriginInfo
         type_(type) {}
 
  protected:
+  virtual ~DeleteOriginInfo() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->DeleteOriginInfo(origin_, type_)) {
       set_db_disabled(true);
     }
   }
+
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {}
 
  private:
@@ -886,6 +918,9 @@ class QuotaManager::InitializeTemporaryOriginsInfoTask
   }
 
  protected:
+  virtual ~InitializeTemporaryOriginsInfoTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->IsOriginDatabaseBootstrapped()) {
       // Register existing origins with 0 last time access.
@@ -898,6 +933,8 @@ class QuotaManager::InitializeTemporaryOriginsInfoTask
       }
     }
   }
+
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {
     if (has_registered_origins_)
       manager()->StartEviction();
@@ -918,9 +955,11 @@ class QuotaManager::AvailableSpaceQueryTask : public QuotaThreadTask {
         space_(-1),
         callback_(callback) {
   }
-  virtual ~AvailableSpaceQueryTask() {}
 
  protected:
+  virtual ~AvailableSpaceQueryTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     space_ = base::SysInfo::AmountOfFreeDiskSpace(profile_path_);
   }
@@ -953,11 +992,16 @@ class QuotaManager::UpdateAccessTimeTask
         accessed_time_(accessed_time) {}
 
  protected:
+  virtual ~UpdateAccessTimeTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->SetOriginLastAccessTime(origin_, type_, accessed_time_)) {
       set_db_disabled(true);
     }
   }
+
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {}
 
  private:
@@ -980,12 +1024,17 @@ class QuotaManager::UpdateModifiedTimeTask
         modified_time_(modified_time) {}
 
  protected:
+  virtual ~UpdateModifiedTimeTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->SetOriginLastModifiedTime(
             origin_, type_, modified_time_)) {
       set_db_disabled(true);
     }
   }
+
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {}
 
  private:
@@ -1008,6 +1057,9 @@ class QuotaManager::GetModifiedSinceTask
         callback_(callback) {}
 
  protected:
+  virtual ~GetModifiedSinceTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->GetOriginsModifiedSince(
             type_, &origins_, modified_since_)) {
@@ -1015,14 +1067,15 @@ class QuotaManager::GetModifiedSinceTask
     }
   }
 
-  virtual void DatabaseTaskCompleted() OVERRIDE {
-    callback_.Run(origins_, type_);
-  }
-
   virtual void Aborted() OVERRIDE {
     callback_.Run(std::set<GURL>(), type_);
   }
 
+  // DatabaseTaskBase:
+  virtual void DatabaseTaskCompleted() OVERRIDE {
+    callback_.Run(origins_, type_);
+  }
+
  private:
   StorageType type_;
   base::Time modified_since_;
@@ -1046,7 +1099,11 @@ class QuotaManager::DumpQuotaTableTask
       : DatabaseTaskBase(manager),
         callback_(callback) {
   }
+
  protected:
+  virtual ~DumpQuotaTableTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->DumpQuotaTable(
             new TableCallback(
@@ -1058,6 +1115,7 @@ class QuotaManager::DumpQuotaTableTask
     callback_.Run(TableEntries());
   }
 
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {
     callback_.Run(entries_);
   }
@@ -1088,7 +1146,11 @@ class QuotaManager::DumpOriginInfoTableTask
       : DatabaseTaskBase(manager),
         callback_(callback) {
   }
+
  protected:
+  virtual ~DumpOriginInfoTableTask() {}
+
+  // QuotaThreadTask:
   virtual void RunOnTargetThread() OVERRIDE {
     if (!database()->DumpOriginInfoTable(
             new TableCallback(
@@ -1100,6 +1162,7 @@ class QuotaManager::DumpOriginInfoTableTask
     callback_.Run(TableEntries());
   }
 
+  // DatabaseTaskBase:
   virtual void DatabaseTaskCompleted() OVERRIDE {
     callback_.Run(entries_);
   }
@@ -1136,14 +1199,6 @@ QuotaManager::QuotaManager(bool is_incognito,
     weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
-QuotaManager::~QuotaManager() {
-  proxy_->manager_ = NULL;
-  std::for_each(clients_.begin(), clients_.end(),
-                std::mem_fun(&QuotaClient::OnQuotaManagerDestroyed));
-  if (database_.get())
-    db_thread_->DeleteSoon(FROM_HERE, database_.release());
-}
-
 void QuotaManager::GetUsageInfo(const GetUsageInfoCallback& callback) {
   LazyInitialize();
   GetUsageInfoTask* get_usage_info = new GetUsageInfoTask(this, callback);
@@ -1274,6 +1329,14 @@ void QuotaManager::GetOriginsModifiedSince(StorageType type,
       this, type, modified_since, callback))->Start();
 }
 
+QuotaManager::~QuotaManager() {
+  proxy_->manager_ = NULL;
+  std::for_each(clients_.begin(), clients_.end(),
+                std::mem_fun(&QuotaClient::OnQuotaManagerDestroyed));
+  if (database_.get())
+    db_thread_->DeleteSoon(FROM_HERE, database_.release());
+}
+
 void QuotaManager::LazyInitialize() {
   DCHECK(io_thread_->BelongsToCurrentThread());
   if (database_.get()) {
diff --git a/webkit/quota/quota_manager.h b/webkit/quota/quota_manager.h
index 6536152..b86cbfd 100644
--- a/webkit/quota/quota_manager.h
+++ b/webkit/quota/quota_manager.h
@@ -21,6 +21,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/sequenced_task_runner_helpers.h"
 #include "webkit/quota/quota_database.h"
 #include "webkit/quota/quota_client.h"
 #include "webkit/quota/quota_task.h"
@@ -109,8 +110,6 @@ class QuotaManager : public QuotaTaskObserver,
                base::MessageLoopProxy* db_thread,
                SpecialStoragePolicy* special_storage_policy);
 
-  virtual ~QuotaManager();
-
   // Returns a proxy object that can be used on any thread.
   QuotaManagerProxy* proxy() { return proxy_.get(); }
 
@@ -204,7 +203,19 @@ class QuotaManager : public QuotaTaskObserver,
 
   static const int kEvictionIntervalInMilliSeconds;
 
+ protected:
+  virtual ~QuotaManager();
+
  private:
+  friend class base::DeleteHelper<QuotaManager>;
+  friend class MockQuotaManager;
+  friend class MockStorageClient;
+  friend class quota_internals::QuotaInternalsProxy;
+  friend class QuotaManagerProxy;
+  friend class QuotaManagerTest;
+  friend class QuotaTemporaryStorageEvictor;
+  friend struct QuotaManagerDeleter;
+
   class DatabaseTaskBase;
   class InitializeTask;
   class UpdateTemporaryQuotaOverrideTask;
@@ -256,14 +267,6 @@ class QuotaManager : public QuotaTaskObserver,
   typedef QuotaEvictionHandler::GetUsageAndQuotaForEvictionCallback
       UsageAndQuotaDispatcherCallback;
 
-  friend class quota_internals::QuotaInternalsProxy;
-  friend struct QuotaManagerDeleter;
-  friend class MockStorageClient;
-  friend class QuotaManagerProxy;
-  friend class QuotaManagerTest;
-  friend class QuotaTemporaryStorageEvictor;
-  friend class MockQuotaManager;
-
   // This initialization method is lazily called on the IO thread
   // when the first quota manager API is called.
   // Initialize must be called after all quota clients are added to the