RefCounted types should not have public destructors, net/

net/base/address_list_net_log_param.h

Index: net/base/address_list_net_log_param.h
diff --git a/net/base/address_list_net_log_param.h b/net/base/address_list_net_log_param.h
index b33fee17f55eeec8f3045876347f4d72deb9ef14..f8b8f883e2087d50524b9b20b111d518ab160a8b 100644
--- a/net/base/address_list_net_log_param.h
+++ b/net/base/address_list_net_log_param.h
@@ -21,6 +21,8 @@ class AddressListNetLogParam : public NetLog::EventParameters {
   virtual base::Value* ToValue() const OVERRIDE;
 
  private:
+  virtual ~AddressListNetLogParam() {}

[wtc, 2012/04/17 23:16:18]

It is not obvious why this class needs to have a private
destructor because the fact that this class is ref-counted
is not shown in this header.

If one tries to delete an instance of this class directly,
won't the compiler bark about that?  If the compiler detects
this, then I prefer that we don't make the destructor of
such a derived class private.

[Ryan Sleevi, 2012/04/17 23:42:42]

No. It will not. And several recent bugs show that this does happen.

The destructor must be /protected/ or /private/ for the compiler to know. It's
perfectly valid for a derived class to give greater visibility than its base
class affords.

Here, the default destructor is being implicitly created by the compiler, which
defaults to public. This then permits the following construct:

AddressListNetLogParam* param = new AddressListNetLogParam(...);
delete reinterpret_cast<NetLog::EventParameters*>(param);  // compiler error
delete param;  // valid, as AddressListNetLogParam is public

More commonly (and where the bugs crept in) was people sub-classing some
interface, not declaring a destructor, then sticking the subclassed type in a
scoped_ptr<>. Had they done scoped_ptr<Base> it would have failed to compile,
but because they did scoped_ptr<Derived>, it succeeds.

The compiler warning for Clang that I added to our plugin is able to walk the
object hierarchy and detect these transitive-ref-counted types and warn
appropriately, which is how this CL was generated (
http://codereview.chromium.org/10005022/ ). Once the existing instances are
fixed, this will be enabled, which will cover Mac, Linux, and ChromeOS, and
provide significant coverage for shared code.

+
   AddressList address_list_;
 };