Use new callbacks in the IqRequest interface.

Use new callbacks in the IqRequest interface.

BUG=None
TEST=Unittests.

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

[awong, 2011-08-25 21:35:09]

LGTM

Seems pretty mechanical.

On Thu, Aug 25, 2011 at 2:31 PM,  <sergeyu@chromium.org> wrote:
> Reviewers: awong, Wez,
>
> Description:
> Use new callbacks in the IqRequest interface.
>
> BUG=None
> TEST=Unittests.
>
>
> Please review this at http://codereview.chromium.org/7744041/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M remoting/host/heartbeat_sender.cc
>  M remoting/host/register_support_host_request.cc
>  M remoting/host/register_support_host_request_unittest.cc
>  M remoting/jingle_glue/iq_request.h
>  M remoting/jingle_glue/javascript_iq_request.h
>  M remoting/jingle_glue/javascript_iq_request.cc
>  M remoting/jingle_glue/jingle_info_request.cc
>  M remoting/jingle_glue/mock_objects.h
>  M remoting/jingle_glue/xmpp_iq_request.h
>  M remoting/jingle_glue/xmpp_iq_request.cc
>
>
> Index: remoting/host/heartbeat_sender.cc
> diff --git a/remoting/host/heartbeat_sender.cc
> b/remoting/host/heartbeat_sender.cc
> index
>
af739b0930e5be1acd21584d71d6f248db07cf7b..589eb312eb5f98947f3d0331201111d824b74e5c
> 100644
> --- a/remoting/host/heartbeat_sender.cc
> +++ b/remoting/host/heartbeat_sender.cc
> @@ -4,6 +4,7 @@
>
>  #include "remoting/host/heartbeat_sender.h"
>
> +#include "base/bind.h"
>  #include "base/logging.h"
>  #include "base/message_loop_proxy.h"
>  #include "base/string_number_conversions.h"
> @@ -72,7 +73,8 @@ void
> HeartbeatSender::OnSignallingConnected(SignalStrategy* signal_strategy,
>
>   full_jid_ = full_jid;
>   request_.reset(signal_strategy->CreateIqRequest());
> -  request_->set_callback(NewCallback(this,
> &HeartbeatSender::ProcessResponse));
> +  request_->set_callback(base::Bind(&HeartbeatSender::ProcessResponse,
> +                                    base::Unretained(this)));
>
>   DoSendStanza();
>   timer_.Start(base::TimeDelta::FromMilliseconds(interval_ms_), this,
> Index: remoting/host/register_support_host_request.cc
> diff --git a/remoting/host/register_support_host_request.cc
> b/remoting/host/register_support_host_request.cc
> index
>
00045f06f6d2ce978c74eaaf0416c2ec0c5467c9..3c47daba239a0001c85632c04b9597be433fa078
> 100644
> --- a/remoting/host/register_support_host_request.cc
> +++ b/remoting/host/register_support_host_request.cc
> @@ -4,6 +4,7 @@
>
>  #include "remoting/host/register_support_host_request.h"
>
> +#include "base/bind.h"
>  #include "base/logging.h"
>  #include "base/message_loop.h"
>  #include "base/string_number_conversions.h"
> @@ -59,8 +60,8 @@ void RegisterSupportHostRequest::OnSignallingConnected(
>   message_loop_ = MessageLoop::current();
>
>   request_.reset(signal_strategy->CreateIqRequest());
> -  request_->set_callback(NewCallback(
> -      this, &RegisterSupportHostRequest::ProcessResponse));
> +  request_->set_callback(base::Bind(
> +      &RegisterSupportHostRequest::ProcessResponse,
> base::Unretained(this)));
>
>   request_->SendIq(buzz::STR_SET, kChromotingBotJid,
>                    CreateRegistrationRequest(jid));
> Index: remoting/host/register_support_host_request_unittest.cc
> diff --git a/remoting/host/register_support_host_request_unittest.cc
> b/remoting/host/register_support_host_request_unittest.cc
> index
>
e08b99daf413fbc39111073251b266dc1ca9e01e..50c305a63ab8d7a8f6415f8546b3e9d77fe0c492
> 100644
> --- a/remoting/host/register_support_host_request_unittest.cc
> +++ b/remoting/host/register_support_host_request_unittest.cc
> @@ -129,7 +129,7 @@ TEST_F(RegisterSupportHostRequestTest, Send) {
>   support_id_lifetime->AddText(kSupportIdLifetime);
>   result->AddElement(support_id_lifetime);
>
> -  iq_request->callback()->Run(response.get());
> +  iq_request->callback().Run(response.get());
>   message_loop_.RunAllPending();
>  }
>
> Index: remoting/jingle_glue/iq_request.h
> diff --git a/remoting/jingle_glue/iq_request.h
> b/remoting/jingle_glue/iq_request.h
> index
>
9e946ad2f56724e17f1b92e515022176b93c9b50..015d2db9458191bcdb3d6a7dc74665ac6aeeedd0
> 100644
> --- a/remoting/jingle_glue/iq_request.h
> +++ b/remoting/jingle_glue/iq_request.h
> @@ -7,7 +7,7 @@
>
>  #include <string>
>
> -#include "base/callback_old.h"
> +#include "base/callback.h"
>  #include "base/gtest_prod_util.h"
>
>  namespace buzz {
> @@ -23,20 +23,18 @@ namespace remoting {
>  // callback with the response.
>  class IqRequest {
>  public:
> -  typedef Callback1<const buzz::XmlElement*>::Type ReplyCallback;
> +  typedef base::Callback<void(const buzz::XmlElement*)> ReplyCallback;
>
>   IqRequest() {}
>   virtual ~IqRequest() {}
>
>   // Sends stanza of type |type| to |addressee|. |iq_body| contains body of
> -  // the stanza. Ownership of |iq_body| is transfered to IqRequest. Must
> -  // be called on the jingle thread.
> +  // the stanza. Takes pwnership of |iq_body|.
>   virtual void SendIq(const std::string& type, const std::string& addressee,
>                       buzz::XmlElement* iq_body) = 0;
>
> -  // Sets callback that is called when reply stanza is received. Callback
> -  // is called on the jingle thread.
> -  virtual void set_callback(ReplyCallback* callback) = 0;
> +  // Sets callback that is called when reply stanza is received.
> +  virtual void set_callback(const ReplyCallback& callback) = 0;
>
>  protected:
>   static buzz::XmlElement* MakeIqStanza(const std::string& type,
> Index: remoting/jingle_glue/javascript_iq_request.cc
> diff --git a/remoting/jingle_glue/javascript_iq_request.cc
> b/remoting/jingle_glue/javascript_iq_request.cc
> index
>
ec83365e4ec0849148bc2ba630f5c63d079276a9..0b75c71a51244790a45ed78becd5d36efcb72c61
> 100644
> --- a/remoting/jingle_glue/javascript_iq_request.cc
> +++ b/remoting/jingle_glue/javascript_iq_request.cc
> @@ -57,8 +57,9 @@ void JavascriptIqRegistry::OnIncomingStanza(const
> buzz::XmlElement* stanza) {
>     // JavascriptIqRequest::SendIq(), but remove in
>     // JavascriptIqRegistry::OnIq().  We should try to keep the
>     // registration/deregistration in one spot.
> -    if (it->second->callback_.get()) {
> -      it->second->callback_->Run(stanza);
> +    if (it->second->callback_.is_null()) {
> +      it->second->callback_.Run(stanza);
> +      it->second->callback_.Reset();
>     } else {
>       VLOG(1) << "No callback, so dropping: " << stanza->Str();
>     }
> @@ -90,8 +91,8 @@ void JavascriptIqRequest::SendIq(const std::string& type,
>   signal_strategy_->SendStanza(MakeIqStanza(type, addressee, iq_body, id));
>  }
>
> -void JavascriptIqRequest::set_callback(ReplyCallback* callback) {
> -  callback_.reset(callback);
> +void JavascriptIqRequest::set_callback(const ReplyCallback& callback) {
> +  callback_ = callback;
>  }
>
>  }  // namespace remoting
> Index: remoting/jingle_glue/javascript_iq_request.h
> diff --git a/remoting/jingle_glue/javascript_iq_request.h
> b/remoting/jingle_glue/javascript_iq_request.h
> index
>
eedc0d8de8452df4eb7f26d58e60ca453ac0249d..d2ff5d937b2df82ded86de62f1b31e87de6202e0
> 100644
> --- a/remoting/jingle_glue/javascript_iq_request.h
> +++ b/remoting/jingle_glue/javascript_iq_request.h
> @@ -54,15 +54,15 @@ class JavascriptIqRequest : public IqRequest {
>                       JavascriptIqRegistry* registry);
>   virtual ~JavascriptIqRequest();
>
> -  //  IqRequest interface.
> +  // IqRequest interface.
>   virtual void SendIq(const std::string& type, const std::string& addressee,
>                       buzz::XmlElement* iq_body) OVERRIDE;
> -  virtual void set_callback(ReplyCallback* callback) OVERRIDE;
> +  virtual void set_callback(const ReplyCallback& callback) OVERRIDE;
>
>  private:
>   friend class JavascriptIqRegistry;
>
> -  scoped_ptr<ReplyCallback> callback_;
> +  ReplyCallback callback_;
>   SignalStrategy* signal_strategy_;
>   JavascriptIqRegistry* registry_;
>
> Index: remoting/jingle_glue/jingle_info_request.cc
> diff --git a/remoting/jingle_glue/jingle_info_request.cc
> b/remoting/jingle_glue/jingle_info_request.cc
> index
>
2c3a39c0bd005d03fee2548f7aa6647985edc20a..888c7682058ec23791a8913b299ea72a9b18ee4d
> 100644
> --- a/remoting/jingle_glue/jingle_info_request.cc
> +++ b/remoting/jingle_glue/jingle_info_request.cc
> @@ -4,6 +4,7 @@
>
>  #include "remoting/jingle_glue/jingle_info_request.h"
>
> +#include "base/bind.h"
>  #include "base/task.h"
>  #include "base/message_loop.h"
>  #include "base/stl_util.h"
> @@ -22,7 +23,8 @@ JingleInfoRequest::JingleInfoRequest(IqRequest* request,
>                                      HostResolverFactory*
> host_resolver_factory)
>     : host_resolver_factory_(host_resolver_factory),
>       request_(request) {
> -  request_->set_callback(NewCallback(this,
> &JingleInfoRequest::OnResponse));
> +  request_->set_callback(base::Bind(&JingleInfoRequest::OnResponse,
> +                                    base::Unretained(this)));
>  }
>
>  JingleInfoRequest::~JingleInfoRequest() {
> Index: remoting/jingle_glue/mock_objects.h
> diff --git a/remoting/jingle_glue/mock_objects.h
> b/remoting/jingle_glue/mock_objects.h
> index
>
e32e203772c732789efb0168bd0847aee2a57972..6c93b0a6fc352875ca99adb212d3b0e01ec9a9e3
> 100644
> --- a/remoting/jingle_glue/mock_objects.h
> +++ b/remoting/jingle_glue/mock_objects.h
> @@ -30,12 +30,12 @@ class MockIqRequest : public IqRequest {
>   MOCK_METHOD3(SendIq, void(const std::string& type,
>                             const std::string& addressee,
>                             buzz::XmlElement* iq_body));
> -  MOCK_METHOD1(set_callback, void(IqRequest::ReplyCallback*));
> +  MOCK_METHOD1(set_callback, void(const IqRequest::ReplyCallback&));
>
>   // Ensure this takes ownership of the pointer, as the real IqRequest
> object
>   // would, to avoid memory-leak.
> -  void set_callback_hook(IqRequest::ReplyCallback* callback) {
> -    callback_.reset(callback);
> +  void set_callback_hook(const IqRequest::ReplyCallback& callback) {
> +    callback_ = callback;
>   }
>
>   void Init() {
> @@ -44,10 +44,10 @@ class MockIqRequest : public IqRequest {
>             this, &MockIqRequest::set_callback_hook));
>   }
>
> -  IqRequest::ReplyCallback* callback() { return callback_.get(); }
> +  ReplyCallback& callback() { return callback_; }
>
>  private:
> -  scoped_ptr<IqRequest::ReplyCallback> callback_;
> +  ReplyCallback callback_;
>  };
>
>  }  // namespace remoting
> Index: remoting/jingle_glue/xmpp_iq_request.cc
> diff --git a/remoting/jingle_glue/xmpp_iq_request.cc
> b/remoting/jingle_glue/xmpp_iq_request.cc
> index
>
ee55536fec836063b21e2a6fdd18c1119efc3cab..e48120bc5b1cba53d4dc94dc1a7eb571b92a7d80
> 100644
> --- a/remoting/jingle_glue/xmpp_iq_request.cc
> +++ b/remoting/jingle_glue/xmpp_iq_request.cc
> @@ -41,8 +41,8 @@ void XmppIqRequest::SendIq(const std::string& type,
>   xmpp_client_->engine()->SendIq(stanza.get(), this, &cookie_);
>  }
>
> -void XmppIqRequest::set_callback(ReplyCallback* callback) {
> -  callback_.reset(callback);
> +void XmppIqRequest::set_callback(const ReplyCallback& callback) {
> +  callback_ = callback;
>  }
>
>  void XmppIqRequest::Unregister() {
> @@ -57,8 +57,9 @@ void XmppIqRequest::Unregister() {
>
>  void XmppIqRequest::IqResponse(buzz::XmppIqCookie cookie,
>                                const buzz::XmlElement* stanza) {
> -  if (callback_.get() != NULL) {
> -    callback_->Run(stanza);
> +  if (callback_.is_null()) {
> +    callback_.Run(stanza);
> +    callback_.Reset();
>   }
>  }
>
> Index: remoting/jingle_glue/xmpp_iq_request.h
> diff --git a/remoting/jingle_glue/xmpp_iq_request.h
> b/remoting/jingle_glue/xmpp_iq_request.h
> index
>
2e1b8b441829385eb4c6fd0535c0f8d20015ba86..ce8d61bbb9cf3e29b98ce9db3e7dd821eb03b396
> 100644
> --- a/remoting/jingle_glue/xmpp_iq_request.h
> +++ b/remoting/jingle_glue/xmpp_iq_request.h
> @@ -20,15 +20,13 @@ namespace remoting {
>
>  class XmppIqRequest : public IqRequest, public buzz::XmppIqHandler {
>  public:
> -  typedef Callback1<const buzz::XmlElement*>::Type ReplyCallback;
> -
>   XmppIqRequest(MessageLoop* message_loop, buzz::XmppClient* xmpp_client);
>   virtual ~XmppIqRequest();
>
>   // IqRequest interface.
>   virtual void SendIq(const std::string& type, const std::string& addressee,
>                       buzz::XmlElement* iq_body) OVERRIDE;
> -  virtual void set_callback(ReplyCallback* callback) OVERRIDE;
> +  virtual void set_callback(const ReplyCallback& callback) OVERRIDE;
>
>   // buzz::XmppIqHandler interface.
>   virtual void IqResponse(buzz::XmppIqCookie cookie,
> @@ -44,7 +42,7 @@ class XmppIqRequest : public IqRequest, public
> buzz::XmppIqHandler {
>   MessageLoop* message_loop_;
>   buzz::XmppClient* xmpp_client_;
>   buzz::XmppIqCookie cookie_;
> -  scoped_ptr<ReplyCallback> callback_;
> +  ReplyCallback callback_;
>  };
>
>  }  // namespace remoting
>
>
>