RpcHandlerTest cleanup

Review URL: https://codereview.chromium.org/685983007

Cr-Commit-Position: refs/heads/master@{#302676}

components/copresence.gypi

@@ -65,6 +65,8 @@
       'sources': [
         'copresence/test/audio_test_support.cc',
         'copresence/test/audio_test_support.h',
+        'copresence/test/stub_whispernet_client.cc',
+        'copresence/test/stub_whispernet_client.h',
       ],
     },
     {

components/copresence/rpc/rpc_handler.cc

@@ -153,16 +153,21 @@ void AddTokenToRequest(const AudioToken& token, ReportRequest* request) {
 // 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() {
@@ -170,13 +175,11 @@ RpcHandler::~RpcHandler() {
     delete post;
   }
 
-  if (delegate_->GetWhispernetClient()) {
-    // TODO(ckehoe): Use CancelableCallbacks instead.
-    delegate_->GetWhispernetClient()->RegisterTokensCallback(
-        WhispernetClient::TokensCallback());
-    delegate_->GetWhispernetClient()->RegisterSamplesCallback(
-        WhispernetClient::SamplesCallback());
-  }
+  // TODO(ckehoe): Register and cancel these callbacks in the same class.
+  delegate_->GetWhispernetClient()->RegisterTokensCallback(
+      WhispernetClient::TokensCallback());
+  delegate_->GetWhispernetClient()->RegisterSamplesCallback(
+      WhispernetClient::SamplesCallback());
 }
 
 void RpcHandler::RegisterForToken(const std::string& auth_token,

components/copresence/rpc/rpc_handler.h

@@ -31,12 +31,38 @@ class RpcHandler {
   // A callback to indicate whether handler initialization succeeded.
   typedef base::Callback<void(bool)> SuccessCallback;
 
+  // An HttpPost::ResponseCallback along with an HttpPost object to be deleted.
+  // Arguments:
+  // HttpPost*: The handler should take ownership of (i.e. delete) this object.
+  // int: The HTTP status code of the response.
+  // string: The contents of the response.
+  typedef base::Callback<void(HttpPost*, int, const std::string&)>
+      PostCleanupCallback;
+
+  // Callback to allow tests to stub out HTTP POST behavior.
+  // Arguments:
+  // URLRequestContextGetter: Context for the HTTP POST request.
+  // string: Name of the rpc to invoke. URL format: server.google.com/rpc_name
+  // string: The API key to pass in the request.
+  // string: The auth token to pass with the request.
+  // MessageLite: Contents of POST request to be sent. This needs to be
+  //     a (scoped) pointer to ease handling of the abstract MessageLite class.
+  // PostCleanupCallback: Receives the response to the request.
+  typedef base::Callback<void(net::URLRequestContextGetter*,
+                              const std::string&,
+                              const std::string&,
+                              const std::string&,
+                              scoped_ptr<google::protobuf::MessageLite>,
+                              const PostCleanupCallback&)> PostCallback;
+
   // Report rpc name to send to Apiary.
   static const char kReportRequestRpcName[];
 
   // Constructor. |delegate| and |directive_handler|
   // are owned by the caller and must outlive the RpcHandler.
-  RpcHandler(CopresenceDelegate* delegate, DirectiveHandler* directive_handler);
+  RpcHandler(CopresenceDelegate* delegate,
+             DirectiveHandler* directive_handler,
+             const PostCallback& server_post_callback = PostCallback());
 
   virtual ~RpcHandler();
 
@@ -66,30 +92,6 @@ class RpcHandler {
   void ReportTokens(const std::vector<AudioToken>& tokens);
 
  private:
-  // An HttpPost::ResponseCallback along with an HttpPost object to be deleted.
-  // Arguments:
-  // HttpPost*: The handler should take ownership of (i.e. delete) this object.
-  // int: The HTTP status code of the response.
-  // string: The contents of the response.
-  typedef base::Callback<void(HttpPost*, int, const std::string&)>
-      PostCleanupCallback;
-
-  // Callback to allow tests to stub out HTTP POST behavior.
-  // Arguments:
-  // URLRequestContextGetter: Context for the HTTP POST request.
-  // string: Name of the rpc to invoke. URL format: server.google.com/rpc_name
-  // string: The API key to pass in the request.
-  // string: The auth token to pass with the request.
-  // MessageLite: Contents of POST request to be sent. This needs to be
-  //     a (scoped) pointer to ease handling of the abstract MessageLite class.
-  // PostCleanupCallback: Receives the response to the request.
-  typedef base::Callback<void(net::URLRequestContextGetter*,
-                              const std::string&,
-                              const std::string&,
-                              const std::string&,
-                              scoped_ptr<google::protobuf::MessageLite>,
-                              const PostCleanupCallback&)> PostCallback;
-
   friend class RpcHandlerTest;
 
   // Server call response handlers.
@@ -143,7 +145,6 @@ class RpcHandler {
 
   TimedMap<std::string, bool> invalid_audio_token_cache_;
 
-  // TODO(ckehoe): Allow passing this into the constructor for testing.
   PostCallback server_post_callback_;
 
   std::map<std::string, std::string> device_id_by_auth_token_;

components/copresence/rpc/rpc_handler_unittest.cc

@@ -17,15 +17,16 @@
 #include "components/copresence/proto/data.pb.h"
 #include "components/copresence/proto/enums.pb.h"
 #include "components/copresence/proto/rpcs.pb.h"
+#include "components/copresence/test/stub_whispernet_client.h"
 #include "net/http/http_status_code.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 using google::protobuf::MessageLite;
 using google::protobuf::RepeatedPtrField;
 
+using testing::ElementsAre;
 using testing::Property;
 using testing::SizeIs;
-using testing::ElementsAre;
 
 namespace copresence {
 
@@ -43,21 +44,20 @@ void CreateSubscribedMessage(const std::vector<std::string>& subscription_ids,
 }
 
 // TODO(ckehoe): Make DirectiveHandler an interface.
-class FakeDirectiveHandler : public DirectiveHandler {
+class FakeDirectiveHandler final : public DirectiveHandler {
  public:
   FakeDirectiveHandler() {}
-  ~FakeDirectiveHandler() override {}
 
-  const std::vector<Directive>& added_directives() const {
+  const std::vector<std::string>& added_directives() const {
     return added_directives_;
   }
 
-  void Start(WhispernetClient* whispernet_client) override {
+  void Start(WhispernetClient* /* whispernet_client */) override {
     NOTREACHED();
   }
 
   void AddDirective(const Directive& directive) override {
-    added_directives_.push_back(directive);
+    added_directives_.push_back(directive.subscription_id());
   }
 
   void RemoveDirectives(const std::string& op_id) override {
@@ -69,7 +69,7 @@ class FakeDirectiveHandler : public DirectiveHandler {
   }
 
  private:
-  std::vector<Directive> added_directives_;
+  std::vector<std::string> added_directives_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeDirectiveHandler);
 };
@@ -78,18 +78,23 @@ class FakeDirectiveHandler : public DirectiveHandler {
 
 class RpcHandlerTest : public testing::Test, public CopresenceDelegate {
  public:
-  RpcHandlerTest() : rpc_handler_(this, &directive_handler_), status_(SUCCESS) {
-    rpc_handler_.server_post_callback_ =
-        base::Bind(&RpcHandlerTest::CaptureHttpPost, base::Unretained(this));
-  }
+  RpcHandlerTest()
+    : whispernet_client_(new StubWhispernetClient),
+      rpc_handler_(this,
+                   &directive_handler_,
+                   base::Bind(&RpcHandlerTest::CaptureHttpPost,
+                              base::Unretained(this))),
+      status_(SUCCESS) {}
 
   // CopresenceDelegate implementation
 
-  void HandleMessages(const std::string& app_id,
+  void HandleMessages(const std::string& /* app_id */,
                       const std::string& subscription_id,
                       const std::vector<Message>& messages) override {
     // app_id is unused for now, pending a server fix.
-    messages_by_subscription_[subscription_id] = messages;
+    for (const Message& message : messages) {
+      messages_by_subscription_[subscription_id].push_back(message.payload());
+    }
   }
 
   net::URLRequestContextGetter* GetRequestContext() const override {
@@ -108,7 +113,9 @@ class RpcHandlerTest : public testing::Test, public CopresenceDelegate {
     return auth_token_;
   }
 
-  WhispernetClient* GetWhispernetClient() override { return nullptr; }
+  WhispernetClient* GetWhispernetClient() override {
+    return whispernet_client_.get();
+  }
 
  protected:
   void InvokeReportResponseHandler(int status_code,
@@ -137,15 +144,16 @@ class RpcHandlerTest : public testing::Test, public CopresenceDelegate {
   // For rpc_handler_.invalid_audio_token_cache_
   base::MessageLoop message_loop_;
 
+  scoped_ptr<WhispernetClient> whispernet_client_;
   FakeDirectiveHandler directive_handler_;
   RpcHandler rpc_handler_;
-  CopresenceStatus status_;
 
+  CopresenceStatus status_;
   std::string rpc_name_;
   std::string api_key_;
   std::string auth_token_;
   ScopedVector<MessageLite> request_protos_;
-  std::map<std::string, std::vector<Message>> messages_by_subscription_;
+  std::map<std::string, std::vector<std::string>> messages_by_subscription_;
 
  private:
   void CaptureHttpPost(
@@ -272,17 +280,13 @@ TEST_F(RpcHandlerTest, ReportResponseHandler) {
   EXPECT_EQ(SUCCESS, status_);
   EXPECT_TRUE(TokenIsInvalid("bad token"));
 
-  EXPECT_THAT(messages_by_subscription_["Subscription 1"], ElementsAre(
-      Property(&Message::payload, "Message A"),
-      Property(&Message::payload, "Message C")));
-
-  EXPECT_THAT(messages_by_subscription_["Subscription 2"], ElementsAre(
-      Property(&Message::payload, "Message B"),
-      Property(&Message::payload, "Message C")));
+  EXPECT_THAT(messages_by_subscription_["Subscription 1"],
+              ElementsAre("Message A", "Message C"));
+  EXPECT_THAT(messages_by_subscription_["Subscription 2"],
+              ElementsAre("Message B", "Message C"));
 
-  EXPECT_THAT(directive_handler_.added_directives(), ElementsAre(
-      Property(&Directive::subscription_id, "Subscription 1"),
-      Property(&Directive::subscription_id, "Subscription 2")));
+  EXPECT_THAT(directive_handler_.added_directives(),
+              ElementsAre("Subscription 1", "Subscription 2"));
 }
 
 }  // namespace copresence

components/copresence/test/audio_test_support.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_COPRESENCE_COMMON_AUDIO_TEST_SUPPORT_
-#define COMPONENTS_COPRESENCE_COMMON_AUDIO_TEST_SUPPORT_
+#ifndef COMPONENTS_COPRESENCE_TEST_AUDIO_TEST_SUPPORT_
+#define COMPONENTS_COPRESENCE_TEST_AUDIO_TEST_SUPPORT_
 
 #include <cstddef>
 
@@ -31,4 +31,4 @@ scoped_refptr<media::AudioBusRefCounted>
 
 }  // namespace copresence
 
-#endif  // COMPONENTS_COPRESENCE_COMMON_AUDIO_TEST_SUPPORT_
+#endif  // COMPONENTS_COPRESENCE_TEST_AUDIO_TEST_SUPPORT_

components/copresence/test/stub_whispernet_client.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/test/stub_whispernet_client.h"
+
+namespace copresence {
+
+WhispernetClient::TokensCallback StubWhispernetClient::GetTokensCallback() {
+  return TokensCallback();
+}
+
+WhispernetClient::SamplesCallback StubWhispernetClient::GetSamplesCallback() {
+  return SamplesCallback();
+}
+
+WhispernetClient::SuccessCallback
+StubWhispernetClient::GetDetectBroadcastCallback() {
+  return SuccessCallback();
+}
+
+WhispernetClient::SuccessCallback
+StubWhispernetClient::GetInitializedCallback() {
+  return SuccessCallback();
+}
+
+}  // namespace copresence

components/copresence/test/stub_whispernet_client.h

+// 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.
+
+#ifndef COMPONENTS_COPRESENCE_TEST_STUB_WHISPERNET_CLIENT_H_
+#define COMPONENTS_COPRESENCE_TEST_STUB_WHISPERNET_CLIENT_H_
+
+#include "components/copresence/public/whispernet_client.h"
+
+namespace copresence {
+
+// An empty WhispernetClient for testing.
+class StubWhispernetClient final : public WhispernetClient {
+ public:
+  StubWhispernetClient() {}
+
+  void Initialize(const SuccessCallback& /* init_callback */) override {}
+  void Shutdown() override {}
+  void EncodeToken(const std::string& /* token */, AudioType /* type */)
+      override {}
+  void DecodeSamples(AudioType /* type */, const std::string& /* samples */)
+      override {}
+  void DetectBroadcast() override {}
+  void RegisterTokensCallback(
+      const TokensCallback& /* tokens_callback */) override {}
+  void RegisterSamplesCallback(
+      const SamplesCallback& /* samples_callback */) override {}
+  void RegisterDetectBroadcastCallback(
+      const SuccessCallback& /* db_callback */) override {}
+  TokensCallback GetTokensCallback() override;
+  SamplesCallback GetSamplesCallback() override;
+  SuccessCallback GetDetectBroadcastCallback() override;
+  SuccessCallback GetInitializedCallback() override;
+};
+
+}  // namespace copresence
+
+#endif  // COMPONENTS_COPRESENCE_TEST_STUB_WHISPERNET_CLIENT_H_