RefCounted types should not have public destructors, net/

net/base/address_list_net_log_param.h

 // Copyright (c) 2011 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.
 
 #ifndef NET_BASE_ADDRESS_LIST_NET_LOG_PARAM_H_
 #define NET_BASE_ADDRESS_LIST_NET_LOG_PARAM_H_
 #pragma once
 
 #include "net/base/address_list.h"
 #include "net/base/net_log.h"
 
 namespace net {
 
 // NetLog parameter to describe an address list.
 // Note that AddressList uses ref-counted data, so this doesn't introduce
 // much of a memory overhead.
 class AddressListNetLogParam : public NetLog::EventParameters {
  public:
   explicit AddressListNetLogParam(const AddressList& address_list);
 
   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_;
 };
 
 }  // namespace net
 
 #endif  // NET_BASE_ADDRESS_LIST_NET_LOG_PARAM_H_