Objects that derive from RefCounted should not have public dtors

Objects that derive from RefCounted should not have public dtors

BUG=none
TEST=none


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

[Ryan Sleevi, 2012-04-05 06:47:42]

jamie: remoting/
sky: content/browser

jamie: One thing I noticed is that talk_base::MessageHandler (which
JingleMessagePump is-a) has a public virtual dtor. This has me a little nervous,
as it would mean anything that had MessageHandler* handler =
(JingleMessagePump*)something; could then delete handler.

Unless you can think of a reason, seems reasonable that we should try to get
talk_base::MessageHandler to use a protected virtual dtor. However, if it is
intentional, then I think there's the opportunity for a real bug to creep in (if
it hasn't yet), so it would be good to refactor the interfaces so that the
MessageHandler* is separate from the RCTS*.

WDYT?

[Jamie, 2012-04-05 06:51:59]

I think I'll defer to Sergey on this one. He knows that code better than I do.

[sky, 2012-04-05 16:25:39]

LGTM, but don't inline the desturctors.

[Ryan Sleevi, 2012-04-05 16:30:19]

On 2012/04/05 16:25:39, sky wrote:
> LGTM, but don't inline the desturctors.

Could you clarify what you mean here?

These are in implementation (.cc) files. If it's a style nit about whether there
should a class declaration before the definition, I don't think we really follow
that in the .cc files in general, but certainly not these two files.

[sky, 2012-04-05 16:32:49]

I'm stupid. I thought these were .h files.

[Sergey Ulanov, 2012-04-05 17:46:47]

remoting/ - lgtm.

I don't think there is a good reason for MessageHandler interface to have public
destructor. I'll try to fix that in libjingle.

On 2012/04/05 06:47:42, Ryan Sleevi wrote:
> jamie: remoting/
> sky: content/browser
> 
> jamie: One thing I noticed is that talk_base::MessageHandler (which
> JingleMessagePump is-a) has a public virtual dtor. This has me a little
nervous,
> as it would mean anything that had MessageHandler* handler =
> (JingleMessagePump*)something; could then delete handler.
> 
> Unless you can think of a reason, seems reasonable that we should try to get
> talk_base::MessageHandler to use a protected virtual dtor. However, if it is
> intentional, then I think there's the opportunity for a real bug to creep in
(if
> it hasn't yet), so it would be good to refactor the interfaces so that the
> MessageHandler* is separate from the RCTS*.
> 
> WDYT?

[commit-bot: I haz the power, 2012-04-05 18:39:40]

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

[commit-bot: I haz the power, 2012-04-05 22:22:08]

Change committed as 131026