RefCounted types should not have public destructors, ipc/ edition

BUG=123295
TEST=none


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

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

5 files changed, 49 insertions, 59 deletions

diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index 7eed1bd..f0e9715 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -14,33 +14,10 @@
 
 namespace IPC {
 
-// This helper ensures the message is deleted if the task is deleted without
-// having been run.
-class SendCallbackHelper
-    : public base::RefCountedThreadSafe<SendCallbackHelper> {
- public:
-  SendCallbackHelper(ChannelProxy::Context* context, Message* message)
-      : context_(context),
-        message_(message) {
-  }
-
-  void Send() {
-    context_->OnSendMessage(message_.release());
-  }
-
- private:
-  scoped_refptr<ChannelProxy::Context> context_;
-  scoped_ptr<Message> message_;
-
-  DISALLOW_COPY_AND_ASSIGN(SendCallbackHelper);
-};
-
 //------------------------------------------------------------------------------
 
 ChannelProxy::MessageFilter::MessageFilter() {}
 
-ChannelProxy::MessageFilter::~MessageFilter() {}
-
 void ChannelProxy::MessageFilter::OnFilterAdded(Channel* channel) {}
 
 void ChannelProxy::MessageFilter::OnFilterRemoved() {}
@@ -59,6 +36,8 @@ void ChannelProxy::MessageFilter::OnDestruct() const {
   delete this;
 }
 
+ChannelProxy::MessageFilter::~MessageFilter() {}
+
 //------------------------------------------------------------------------------
 
 ChannelProxy::Context::Context(Channel::Listener* listener,
@@ -186,13 +165,12 @@ void ChannelProxy::Context::OnChannelClosed() {
 }
 
 // Called on the IPC::Channel thread
-void ChannelProxy::Context::OnSendMessage(Message* message) {
+void ChannelProxy::Context::OnSendMessage(scoped_ptr<Message> message) {
   if (!channel_.get()) {
-    delete message;
     OnChannelClosed();
     return;
   }
-  if (!channel_->Send(message))
+  if (!channel_->Send(message.release()))
     OnChannelError();
 }
 
@@ -368,8 +346,8 @@ bool ChannelProxy::Send(Message* message) {
 
   context_->ipc_message_loop()->PostTask(
       FROM_HERE,
-      base::Bind(&SendCallbackHelper::Send,
-                 new SendCallbackHelper(context_.get(), message)));
+      base::Bind(&ChannelProxy::Context::OnSendMessage,
+                 context_, base::Passed(scoped_ptr<Message>(message))));
   return true;
 }
 
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index 5cbe664..8c0031a 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -58,7 +58,6 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
       : public base::RefCountedThreadSafe<MessageFilter, MessageFilterTraits> {
    public:
     MessageFilter();
-    virtual ~MessageFilter();
 
     // Called on the background thread to provide the filter with access to the
     // channel.  Called when the IPC channel is initialized or when AddFilter
@@ -90,6 +89,13 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
     // derived classes the option of controlling which thread they're deleted
     // on etc.
     virtual void OnDestruct() const;
+
+   protected:
+    virtual ~MessageFilter();
+
+   private:
+    friend class base::RefCountedThreadSafe<MessageFilter,
+                                            MessageFilterTraits>;
   };
 
   struct MessageFilterTraits {
@@ -98,6 +104,7 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
     }
   };
 
+
   // Interface for a filter to be imposed on outgoing messages which can
   // re-write the message.  Used mainly for testing.
   class OutgoingMessageFilter {
@@ -233,7 +240,7 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
                        const Channel::Mode& mode);
 
     // Methods called on the IO thread.
-    void OnSendMessage(Message* message_ptr);
+    void OnSendMessage(scoped_ptr<Message> message_ptr);
     void OnAddFilter();
     void OnRemoveFilter(MessageFilter* filter);
 
diff --git a/ipc/ipc_sync_channel_unittest.cc b/ipc/ipc_sync_channel_unittest.cc
index 0424b2f..7f0a7ab 100644
--- a/ipc/ipc_sync_channel_unittest.cc
+++ b/ipc/ipc_sync_channel_unittest.cc
@@ -1110,6 +1110,9 @@ class TestSyncMessageFilter : public SyncMessageFilter {
     worker_->Done();
   }
 
+ private:
+  virtual ~TestSyncMessageFilter() {}
+
   Worker* worker_;
   base::Thread thread_;
 };
diff --git a/ipc/ipc_sync_message_filter.cc b/ipc/ipc_sync_message_filter.cc
index e49ebd6..347a68c 100644
--- a/ipc/ipc_sync_message_filter.cc
+++ b/ipc/ipc_sync_message_filter.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.
 
@@ -21,9 +21,6 @@ SyncMessageFilter::SyncMessageFilter(base::WaitableEvent* shutdown_event)
       shutdown_event_(shutdown_event) {
 }
 
-SyncMessageFilter::~SyncMessageFilter() {
-}
-
 bool SyncMessageFilter::Send(Message* message) {
   {
     base::AutoLock auto_lock(lock_);
@@ -69,29 +66,6 @@ bool SyncMessageFilter::Send(Message* message) {
   return pending_message.send_result;
 }
 
-void SyncMessageFilter::SendOnIOThread(Message* message) {
-  if (channel_) {
-    channel_->Send(message);
-    return;
-  }
-
-  if (message->is_sync()) {
-    // We don't know which thread sent it, but it doesn't matter, just signal
-    // them all.
-    SignalAllEvents();
-  }
-
-  delete message;
-}
-
-void SyncMessageFilter::SignalAllEvents() {
-  base::AutoLock auto_lock(lock_);
-  for (PendingSyncMessages::iterator iter = pending_sync_messages_.begin();
-       iter != pending_sync_messages_.end(); ++iter) {
-    (*iter)->done_event->Signal();
-  }
-}
-
 void SyncMessageFilter::OnFilterAdded(Channel* channel) {
   channel_ = channel;
   base::AutoLock auto_lock(lock_);
@@ -125,4 +99,30 @@ bool SyncMessageFilter::OnMessageReceived(const Message& message) {
   return false;
 }
 
+SyncMessageFilter::~SyncMessageFilter() {
+}
+
+void SyncMessageFilter::SendOnIOThread(Message* message) {
+  if (channel_) {
+    channel_->Send(message);
+    return;
+  }
+
+  if (message->is_sync()) {
+    // We don't know which thread sent it, but it doesn't matter, just signal
+    // them all.
+    SignalAllEvents();
+  }
+
+  delete message;
+}
+
+void SyncMessageFilter::SignalAllEvents() {
+  base::AutoLock auto_lock(lock_);
+  for (PendingSyncMessages::iterator iter = pending_sync_messages_.begin();
+       iter != pending_sync_messages_.end(); ++iter) {
+    (*iter)->done_event->Signal();
+  }
+}
+
 }  // namespace IPC
diff --git a/ipc/ipc_sync_message_filter.h b/ipc/ipc_sync_message_filter.h
index 1f0f46d..a4c27a2 100644
--- a/ipc/ipc_sync_message_filter.h
+++ b/ipc/ipc_sync_message_filter.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.
 
@@ -30,7 +30,6 @@ class IPC_EXPORT SyncMessageFilter : public ChannelProxy::MessageFilter,
                                      public Message::Sender {
  public:
   explicit SyncMessageFilter(base::WaitableEvent* shutdown_event);
-  virtual ~SyncMessageFilter();
 
   // Message::Sender implementation.
   virtual bool Send(Message* message) OVERRIDE;
@@ -41,6 +40,9 @@ class IPC_EXPORT SyncMessageFilter : public ChannelProxy::MessageFilter,
   virtual void OnChannelClosing() OVERRIDE;
   virtual bool OnMessageReceived(const Message& message) OVERRIDE;
 
+ protected:
+  virtual ~SyncMessageFilter();
+
  private:
   void SendOnIOThread(Message* message);
   // Signal all the pending sends as done, used in an error condition.