RefCounted types should not have public destructors, webkit/ edition

RefCounted types should not have public destructors, webkit/ edition

BUG=123295
TEST=it compiles


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=140265

[Ryan Sleevi, 2012-05-21 08:14:37]

jamesr: The low-hanging fruit from webkit/. Mostly, a bunch of interfaces that
may be implemented by RefCounted types, so ensure the destructors are protected.

[jamesr, 2012-05-21 08:35:38]

Lgtm
On May 21, 2012 1:14 AM, <rsleevi@chromium.org> wrote:

> Reviewers: jamesr,
>
> Message:
> jamesr: The low-hanging fruit from webkit/. Mostly, a bunch of interfaces
> that
> may be implemented by RefCounted types, so ensure the destructors are
> protected.
>
> Description:
> RefCounted types should not have public destructors, webkit/ edition
>
> BUG=123295
> TEST=it compiles
>
>
> Please review this at
http://codereview.chromium.**org/10416004/<http://codereview.chromium.org/104...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M webkit/appcache/appcache_**interfaces.h
>  M webkit/appcache/appcache_**storage.h
>  M webkit/database/database_**tracker.h
>  M webkit/dom_storage/dom_**storage_context.h
>  M webkit/fileapi/isolated_mount_**point_provider.h
>  M webkit/fileapi/test_mount_**point_provider.h
>  M webkit/glue/resource_loader_**bridge.h
>  M webkit/glue/**websocketstreamhandle_**delegate.h
>  M webkit/media/webmediaplayer_**delegate.h
>  M webkit/plugins/npapi/**webplugin.h
>  M webkit/plugins/npapi/**webplugin_page_delegate.h
>  M webkit/plugins/ppapi/**fullscreen_container.h
>  M webkit/plugins/ppapi/plugin_**delegate.h
>  M webkit/quota/quota_manager.h
>  M webkit/quota/quota_task.h
>  M webkit/quota/quota_temporary_**storage_evictor_unittest.cc
>
>
> Index: webkit/appcache/appcache_**interfaces.h
> diff --git a/webkit/appcache/appcache_**interfaces.h
> b/webkit/appcache/appcache_**interfaces.h
> index 113fe47bbf1adfda87a1cb4cc7766b**69c8e7541f..**
> b1ddbf6daa0428b78ea1df51dbde73**a51302a4c2 100644
> --- a/webkit/appcache/appcache_**interfaces.h
> +++ b/webkit/appcache/appcache_**interfaces.h
> @@ -152,6 +152,7 @@ class APPCACHE_EXPORT AppCacheBackend {
>   virtual void GetResourceList(
>       int host_id, std::vector<**AppCacheResourceInfo>* resource_infos) =
> 0;
>
> + protected:
>   virtual ~AppCacheBackend() {}
>  };
>
> Index: webkit/appcache/appcache_**storage.h
> diff --git a/webkit/appcache/appcache_**storage.h
> b/webkit/appcache/appcache_**storage.h
> index 9f74a5919c37ba764a83adf60c0f02**7cd6099f1a..**
> e30bd1cd487c8e1f311097d71f0e27**58a842fcb8 100644
> --- a/webkit/appcache/appcache_**storage.h
> +++ b/webkit/appcache/appcache_**storage.h
> @@ -36,8 +36,6 @@ class APPCACHE_EXPORT AppCacheStorage {
>
>   class APPCACHE_EXPORT Delegate {
>    public:
> -    virtual ~Delegate() {}
> -
>     // If retrieval fails, 'collection' will be NULL.
>     virtual void OnAllInfo(**AppCacheInfoCollection* collection) {}
>
> @@ -70,6 +68,9 @@ class APPCACHE_EXPORT AppCacheStorage {
>         const GURL& url, const AppCacheEntry& entry,
>         const GURL& namespace_entry_url, const AppCacheEntry&
> fallback_entry,
>         int64 cache_id, int64 group_id, const GURL& mainfest_url) {}
> +
> +   protected:
> +    virtual ~Delegate() {}
>   };
>
>   explicit AppCacheStorage(**AppCacheService* service);
> Index: webkit/database/database_**tracker.h
> diff --git a/webkit/database/database_**tracker.h
> b/webkit/database/database_**tracker.h
> index 709e66215cd7c63293af51110f9608**dae1078fd6..**
> 05f125f0f712d2aafa3907e6e7cf58**94733af00a 100644
> --- a/webkit/database/database_**tracker.h
> +++ b/webkit/database/database_**tracker.h
> @@ -86,6 +86,8 @@ class DatabaseTracker
>     virtual void OnDatabaseScheduledForDeletion**(
>         const string16& origin_identifier,
>         const string16& database_name) = 0;
> +
> +   protected:
>     virtual ~Observer() {}
>   };
>
> Index: webkit/dom_storage/dom_**storage_context.h
> diff --git a/webkit/dom_storage/dom_**storage_context.h
> b/webkit/dom_storage/dom_**storage_context.h
> index ca2b570f618d74cb9cc0f66e3717fa**f6fde6fb4a..**
> 630b408a05f5474900b11d0d783b5d**7668bd602e 100644
> --- a/webkit/dom_storage/dom_**storage_context.h
> +++ b/webkit/dom_storage/dom_**storage_context.h
> @@ -85,6 +85,8 @@ class DomStorageContext
>     virtual void OnDomStorageAreaCleared(
>         const DomStorageArea* area,
>         const GURL& page_url) = 0;
> +
> +   protected:
>     virtual ~EventObserver() {}
>   };
>
> Index: webkit/fileapi/isolated_mount_**point_provider.h
> diff --git a/webkit/fileapi/isolated_**mount_point_provider.h
> b/webkit/fileapi/isolated_**mount_point_provider.h
> index 85559d0a96100078e6eac4f4e1b4ce**d171c20340..**
> b41293544905ad62aeac2fd95be192**68aa577181 100644
> --- a/webkit/fileapi/isolated_**mount_point_provider.h
> +++ b/webkit/fileapi/isolated_**mount_point_provider.h
> @@ -13,6 +13,7 @@
>  namespace fileapi {
>
>  class IsolatedContext;
> +class IsolatedFileUtil;
>
>  class IsolatedMountPointProvider : public FileSystemMountPointProvider {
>  public:
> @@ -60,7 +61,7 @@ class IsolatedMountPointProvider : public
> FileSystemMountPointProvider {
>   IsolatedContext* isolated_context() const;
>   FilePath GetPathFromURL(const GURL& url) const;
>
> -  scoped_ptr<FileSystemFileUtil> isolated_file_util_;
> +  scoped_ptr<IsolatedFileUtil> isolated_file_util_;
>  };
>
>  }  // namespace fileapi
> Index: webkit/fileapi/test_mount_**point_provider.h
> diff --git a/webkit/fileapi/test_mount_**point_provider.h
> b/webkit/fileapi/test_mount_**point_provider.h
> index aa739bebe325f57c803a07a44e0f7c**87af20cc66..**
> 8cdfade3f923214c0ef5621489f49c**d3a24baea4 100644
> --- a/webkit/fileapi/test_mount_**point_provider.h
> +++ b/webkit/fileapi/test_mount_**point_provider.h
> @@ -17,6 +17,7 @@ class SequencedTaskRunner;
>
>  namespace fileapi {
>
> +class LocalFileUtil;
>  class FileSystemQuotaUtil;
>
>  // This should be only used for testing.
> @@ -68,7 +69,7 @@ class TestMountPointProvider : public
> FileSystemMountPointProvider {
>
>  private:
>   FilePath base_path_;
> -  scoped_ptr<FileSystemFileUtil> local_file_util_;
> +  scoped_ptr<LocalFileUtil> local_file_util_;
>   scoped_ptr<**FileSystemQuotaUtil> quota_util_;
>  };
>
> Index: webkit/glue/resource_loader_**bridge.h
> diff --git a/webkit/glue/resource_loader_**bridge.h
> b/webkit/glue/resource_loader_**bridge.h
> index a537b1fdd058329a47354b77193192**156e8be795..**
> 7724cd506c0b504824cf0be0f5af74**cd354b0bb2 100644
> --- a/webkit/glue/resource_loader_**bridge.h
> +++ b/webkit/glue/resource_loader_**bridge.h
> @@ -295,8 +295,6 @@ class ResourceLoaderBridge {
>   // for more information.
>   class Peer {
>    public:
> -    virtual ~Peer() {}
> -
>     // Called as upload progress is made.
>     // note: only for requests with LOAD_ENABLE_UPLOAD_PROGRESS set
>     virtual void OnUploadProgress(uint64 position, uint64 size) = 0;
> @@ -342,6 +340,9 @@ class ResourceLoaderBridge {
>         const net::URLRequestStatus& status,
>         const std::string& security_info,
>         const base::TimeTicks& completion_time) = 0;
> +
> +   protected:
> +    virtual ~Peer() {}
>   };
>
>   // use WebKitPlatformSupportImpl::**CreateResourceLoader() for
> construction, but
> Index: webkit/glue/**websocketstreamhandle_**delegate.h
> diff --git a/webkit/glue/**websocketstreamhandle_**delegate.h
> b/webkit/glue/**websocketstreamhandle_**delegate.h
> index 21486996400ccf993b36d7e718f37c**bb594d2f54..**
> 717c83396f04954b69b67ba20f9b2b**fddfc44a5b 100644
> --- a/webkit/glue/**websocketstreamhandle_**delegate.h
> +++ b/webkit/glue/**websocketstreamhandle_**delegate.h
> @@ -16,7 +16,6 @@ namespace webkit_glue {
>  class WebSocketStreamHandleDelegate {
>  public:
>   WebSocketStreamHandleDelegate(**) {}
> -  virtual ~**WebSocketStreamHandleDelegate(**) {}
>
>   virtual void WillOpenStream(WebKit::**WebSocketStreamHandle* handle,
>                               const GURL& url) {}
> @@ -30,6 +29,9 @@ class WebSocketStreamHandleDelegate {
>   virtual void DidReceiveData(WebKit::**WebSocketStreamHandle* handle,
>                               const char* data, int len) {}
>   virtual void DidClose(WebKit::**WebSocketStreamHandle*) {}
> +
> + protected:
> +  virtual ~**WebSocketStreamHandleDelegate(**) {}
>  };
>
>  }  // namespace webkit_glue
> Index: webkit/media/webmediaplayer_**delegate.h
> diff --git a/webkit/media/webmediaplayer_**delegate.h
> b/webkit/media/webmediaplayer_**delegate.h
> index 14edcaa01c9370ded3b736a88b8839**4729c30624..**
> 7d66250bc2ab6bbe1f89e8c9fac49e**94cfadaa81 100644
> --- a/webkit/media/webmediaplayer_**delegate.h
> +++ b/webkit/media/webmediaplayer_**delegate.h
> @@ -14,7 +14,6 @@ class WebMediaPlayerImpl;
>  class WebMediaPlayerDelegate {
>  public:
>   WebMediaPlayerDelegate() {}
> -  virtual ~WebMediaPlayerDelegate() {}
>
>   // The specified player started playing media.
>   virtual void DidPlay(WebMediaPlayerImpl* player) {}
> @@ -24,6 +23,9 @@ class WebMediaPlayerDelegate {
>
>   // The specified player was destroyed. Do not call any methods on it.
>   virtual void PlayerGone(WebMediaPlayerImpl* player) {}
> +
> + protected:
> +  virtual ~WebMediaPlayerDelegate() {}
>  };
>
>  }  // namespace webkit_media
> Index: webkit/plugins/npapi/**webplugin.h
> diff --git a/webkit/plugins/npapi/**webplugin.h b/webkit/plugins/npapi/**
> webplugin.h
> index 4c5d41cf6e7c7a3a2fa4332108e906**350ab2e5d0..**
> a7d86cf518ad1537f538a73b9c51ad**3747c9188a 100644
> --- a/webkit/plugins/npapi/**webplugin.h
> +++ b/webkit/plugins/npapi/**webplugin.h
> @@ -176,6 +176,7 @@ class WebPlugin {
>  class WebPluginResourceClient {
>  public:
>   virtual ~WebPluginResourceClient() {}
> +
>   virtual void WillSendRequest(const GURL& url, int http_status_code) = 0;
>   // The request_is_seekable parameter indicates whether byte range
> requests
>   // can be issued for the underlying stream.
> Index: webkit/plugins/npapi/**webplugin_page_delegate.h
> diff --git a/webkit/plugins/npapi/**webplugin_page_delegate.h
> b/webkit/plugins/npapi/**webplugin_page_delegate.h
> index 496bd6f1739c466ca0589c81a89686**95e4fefb37..**
> ec4b34a26cd71bafaf56ea6ae9451a**9ef1f11588 100644
> --- a/webkit/plugins/npapi/**webplugin_page_delegate.h
> +++ b/webkit/plugins/npapi/**webplugin_page_delegate.h
> @@ -60,6 +60,9 @@ class WebPluginPageDelegate {
>
>   // The WebCookieJar to use for this plugin.
>   virtual WebKit::WebCookieJar* GetCookieJar() = 0;
> +
> + protected:
> +  virtual ~WebPluginPageDelegate() {}
>  };
>
>  }  // namespace npapi
> Index: webkit/plugins/ppapi/**fullscreen_container.h
> diff --git a/webkit/plugins/ppapi/**fullscreen_container.h
> b/webkit/plugins/ppapi/**fullscreen_container.h
> index dafaea6f4159a81f7c0db93220d9e2**ce7f3d850c..**
> 58af40619977944ec160e01af4878a**b97c656ad9 100644
> --- a/webkit/plugins/ppapi/**fullscreen_container.h
> +++ b/webkit/plugins/ppapi/**fullscreen_container.h
> @@ -19,8 +19,6 @@ namespace ppapi {
>  // plugins, that only handles painting.
>  class FullscreenContainer {
>  public:
> -  virtual ~FullscreenContainer() {}
> -
>   // Invalidates the full plugin region.
>   virtual void Invalidate() = 0;
>
> @@ -38,6 +36,9 @@ class FullscreenContainer {
>   virtual void DidChangeCursor(const WebKit::WebCursorInfo& cursor) = 0;
>
>   virtual PluginDelegate::**PlatformContext3D* CreateContext3D() = 0;
> +
> + protected:
> +  virtual ~FullscreenContainer() {}
>  };
>
>  }  // namespace ppapi
> Index: webkit/plugins/ppapi/plugin_**delegate.h
> diff --git a/webkit/plugins/ppapi/plugin_**delegate.h
> b/webkit/plugins/ppapi/plugin_**delegate.h
> index 304cb0905246032531a9cd8c19e479**b5a87e12eb..**
> 379978639a3b4ab69e8887a73d9e4c**10a61c53f2 100644
> --- a/webkit/plugins/ppapi/plugin_**delegate.h
> +++ b/webkit/plugins/ppapi/plugin_**delegate.h
> @@ -280,10 +280,14 @@ class PluginDelegate {
>   class PlatformVideoCapture : public media::VideoCapture,
>                                public
base::RefCounted<**PlatformVideoCapture>
> {
>    public:
> -    virtual ~PlatformVideoCapture() {}
> -
>     // Detaches the event handler and stops sending notifications to it.
>     virtual void DetachEventHandler() = 0;
> +
> +   protected:
> +    virtual ~PlatformVideoCapture() {}
> +
> +   private:
> +    friend class base::RefCounted<**PlatformVideoCapture>;
>   };
>
>   // Provides access to the ppapi broker.
> Index: webkit/quota/quota_manager.h
> diff --git a/webkit/quota/quota_manager.h b/webkit/quota/quota_manager.h
> index eaa47576a2ee8f70a28bb08c2a7a1f**f6b5cf2a85..**
> bcf55c29c9ebb67cceb3797a388795**530c436fc9 100644
> --- a/webkit/quota/quota_manager.h
> +++ b/webkit/quota/quota_manager.h
> @@ -64,8 +64,6 @@ class QuotaEvictionHandler {
>                               const QuotaAndUsage& quota_and_usage)>
>       GetUsageAndQuotaForEvictionCal**lback;
>
> -  virtual ~QuotaEvictionHandler() {}
> -
>   // Returns the least recently used origin.  It might return empty
>   // GURL when there are no evictable origins.
>   virtual void GetLRUOrigin(
> @@ -79,6 +77,9 @@ class QuotaEvictionHandler {
>
>   virtual void GetUsageAndQuotaForEviction(
>       const GetUsageAndQuotaForEvictionCal**lback& callback) = 0;
> +
> + protected:
> +  virtual ~QuotaEvictionHandler() {}
>  };
>
>  struct UsageInfo {
> Index: webkit/quota/quota_task.h
> diff --git a/webkit/quota/quota_task.h b/webkit/quota/quota_task.h
> index ae9b2f1a76d30d0ca641c30326a3df**89d9432d19..**
> 6ade8306ef6e1b918bd3d838fccc8a**4e45fad314 100644
> --- a/webkit/quota/quota_task.h
> +++ b/webkit/quota/quota_task.h
> @@ -11,6 +11,7 @@
>  #include "base/basictypes.h"
>  #include "base/compiler_specific.h"
>  #include "base/memory/ref_counted.h"
> +#include "base/sequenced_task_runner_**helpers.h"
>
>  namespace base {
>  class SingleThreadTaskRunner;
> @@ -26,11 +27,11 @@ class QuotaThreadTask;
>  // TODO(kinuko): Revise this using base::Callback.
>  class QuotaTask {
>  public:
> -  virtual ~QuotaTask();
>   void Start();
>
>  protected:
>   explicit QuotaTask(QuotaTaskObserver* observer);
> +  virtual ~QuotaTask();
>
>   // The task body.
>   virtual void Run() = 0;
> @@ -52,8 +53,10 @@ class QuotaTask {
>   }
>
>  private:
> +  friend class base::DeleteHelper<QuotaTask>;
>   friend class QuotaTaskObserver;
>   friend class QuotaThreadTask;
> +
>   void Abort();
>   QuotaTaskObserver* observer_;
>   scoped_refptr<base::**SingleThreadTaskRunner> original_task_runner_;
> @@ -96,14 +99,13 @@ class QuotaThreadTask : public QuotaTask,
>  };
>
>  class QuotaTaskObserver {
> - public:
> -  virtual ~QuotaTaskObserver();
> -
>  protected:
>   friend class QuotaTask;
>   friend class QuotaThreadTask;
>
>   QuotaTaskObserver();
> +  virtual ~QuotaTaskObserver();
> +
>   void RegisterTask(QuotaTask* task);
>   void UnregisterTask(QuotaTask* task);
>
> Index: webkit/quota/quota_temporary_**storage_evictor_unittest.cc
> diff --git a/webkit/quota/quota_**temporary_storage_evictor_**unittest.cc
> b/webkit/quota/quota_**temporary_storage_evictor_**unittest.cc
> index dc87a99df667cb4622579962a21d98**a79694a104..**
> 012fd010cdce58a2a46799b6365b83**26a9550590 100644
> --- a/webkit/quota/quota_**temporary_storage_evictor_**unittest.cc
> +++ b/webkit/quota/quota_**temporary_storage_evictor_**unittest.cc
> @@ -222,7 +222,7 @@ class QuotaTemporaryStorageEvictorTe**st : public
> testing::Test {
>         reset_min_available_disk_**space_to_start_eviction();
>   }
>
> -  scoped_ptr<**QuotaEvictionHandler> quota_eviction_handler_;
> +  scoped_ptr<**MockQuotaEvictionHandler> quota_eviction_handler_;
>   scoped_ptr<**QuotaTemporaryStorageEvictor> temporary_storage_evictor_;
>
>   int num_get_usage_and_quota_for_**eviction_;
>
>
>

[commit-bot: I haz the power, 2012-05-21 08:49:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/10416004/3001

[commit-bot: I haz the power, 2012-05-21 12:37:25]

Try job failure for 10416004-3001 (retry) on win_rel for steps "base_unittests,
sync_unit_tests".
It's a second try, previously, steps "base_unittests, sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-21 22:18:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/10416004/3001

[commit-bot: I haz the power, 2012-05-22 01:31:11]

Try job failure for 10416004-3001 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-06-04 07:59:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/10416004/13001

[commit-bot: I haz the power, 2012-06-04 08:54:16]

Change committed as 140265