RpcHandlerTest cleanup

components/copresence/rpc/rpc_handler.cc

 // Copyright 2014 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.
 
 #include "components/copresence/rpc/rpc_handler.h"
 
 #include <map>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 135 matching lines @@
   signals->set_medium(token.audible ? AUDIO_AUDIBLE_DTMF
                                     : AUDIO_ULTRASOUND_PASSBAND);
   signals->set_observed_time_millis(base::Time::Now().ToJsTime());
 }
 
 }  // namespace
 
 // Public methods
 
 RpcHandler::RpcHandler(CopresenceDelegate* delegate,
-                       DirectiveHandler* directive_handler)
+                       DirectiveHandler* directive_handler,
+                       const PostCallback& server_post_callback)
     : delegate_(delegate),
       directive_handler_(directive_handler),
       invalid_audio_token_cache_(
           base::TimeDelta::FromMilliseconds(kInvalidTokenExpiryTimeMs),
           kMaxInvalidTokens),
-      server_post_callback_(base::Bind(&RpcHandler::SendHttpPost,
-                                       base::Unretained(this))) {
+      server_post_callback_(server_post_callback) {
     DCHECK(delegate_);
     DCHECK(directive_handler_);
+
+    if (server_post_callback_.is_null()) {
+      server_post_callback_ =
+          base::Bind(&RpcHandler::SendHttpPost, base::Unretained(this));
+    }
   }
 
 RpcHandler::~RpcHandler() {
   for (HttpPost* post : pending_posts_) {
     delete post;
   }
 
-  if (delegate_->GetWhispernetClient()) {
-    // TODO(ckehoe): Use CancelableCallbacks instead.
-    delegate_->GetWhispernetClient()->RegisterTokensCallback(
-        WhispernetClient::TokensCallback());
-    delegate_->GetWhispernetClient()->RegisterSamplesCallback(
-        WhispernetClient::SamplesCallback());
-  }
+  // TODO(ckehoe): Use CancelableCallbacks instead.

[rkc, 2014/11/04 19:35:27]

We don't want to use CancelableCallbacks here. The *only* situation a cancelable
callback or weak pointer should be used in is if we *cannot* predict or control
the freeing of an object. This is true pretty much in one case till now (for
example, when sending a callback to the audio thread). This isn't one such
place; since we can clearly unregister our callbacks on our destruction.

[Charlie, 2014/11/04 19:57:04]

Ok. But I still do not like registering them in one place and unregistering in
another. I suppose that is technically a separate issue.

+  delegate_->GetWhispernetClient()->RegisterTokensCallback(
+      WhispernetClient::TokensCallback());
+  delegate_->GetWhispernetClient()->RegisterSamplesCallback(
+      WhispernetClient::SamplesCallback());
 }
 
 void RpcHandler::RegisterForToken(const std::string& auth_token,
                                   const SuccessCallback& init_done_callback) {
   if (IsRegisteredForToken(auth_token)) {
     LOG(WARNING) << "Attempted re-registration for the same auth token.";
     init_done_callback.Run(true);
     return;
   }
   scoped_ptr<RegisterDeviceRequest> request(new RegisterDeviceRequest);
@@ skipping 324 matching lines @@
       api_key,
       auth_token,
       command_line->GetSwitchValueASCII(switches::kCopresenceTracingToken),
       *request_proto);
 
   http_post->Start(base::Bind(callback, http_post));
   pending_posts_.insert(http_post);
 }
 
 }  // namespace copresence