Reporting of policy errors via host-offline-reason: part 2b

remoting/host/heartbeat_sender_unittest.cc

 // 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.
 
 #include "remoting/host/heartbeat_sender.h"
 
 #include <set>
 
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"

[Lambros, 2014/11/18 21:57:55]

Do you need weark_ptr.h ?

[Łukasz Anforowicz, 2014/11/18 22:16:54]

I don't.  Thanks for pointing this out (I've taken out this include from
mock_callback.h;  I am not sure why I've also had it here;  I'll go through the
overall diff once more).

 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "remoting/base/constants.h"
 #include "remoting/base/rsa_key_pair.h"
 #include "remoting/base/test_rsa_key_pair.h"
+#include "remoting/host/mock_callback.h"
 #include "remoting/signaling/iq_sender.h"
 #include "remoting/signaling/mock_signal_strategy.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/libjingle/source/talk/xmpp/constants.h"
 #include "third_party/webrtc/libjingle/xmllite/xmlelement.h"
 
 using buzz::QName;
 using buzz::XmlElement;
 
 using testing::_;
 using testing::DeleteArg;
 using testing::DoAll;
 using testing::Invoke;
 using testing::NotNull;
 using testing::Return;
 using testing::SaveArg;
 
 namespace remoting {
 
 namespace {
 
 const char kTestBotJid[] = "remotingunittest@bot.talk.google.com";
 const char kHostId[] = "0";
 const char kTestJid[] = "user@gmail.com/chromoting123";
 const char kStanzaId[] = "123";
 const int kTestInterval = 123;
 
-class MockListener : public HeartbeatSender::Listener {
- public:
-  // Overridden from HeartbeatSender::Listener
-  virtual void OnUnknownHostIdError() override {
-    NOTREACHED();
-  }
-
-  // Overridden from HeartbeatSender::Listener
-  MOCK_METHOD0(OnHeartbeatSuccessful, void());
-};
-
 }  // namespace
 
 ACTION_P(AddListener, list) {
   list->insert(arg0);
 }
 ACTION_P(RemoveListener, list) {
   EXPECT_TRUE(list->find(arg0) != list->end());
   list->erase(arg0);
 }
 
 class HeartbeatSenderTest
     : public testing::Test {
  protected:
   virtual void SetUp() override {
     key_pair_ = RsaKeyPair::FromString(kTestRsaKeyPair);
     ASSERT_TRUE(key_pair_.get());
 
     EXPECT_CALL(signal_strategy_, GetState())
         .WillOnce(Return(SignalStrategy::DISCONNECTED));
     EXPECT_CALL(signal_strategy_, AddListener(NotNull()))
         .WillRepeatedly(AddListener(&signal_strategy_listeners_));
     EXPECT_CALL(signal_strategy_, RemoveListener(NotNull()))
         .WillRepeatedly(RemoveListener(&signal_strategy_listeners_));
     EXPECT_CALL(signal_strategy_, GetLocalJid())
         .WillRepeatedly(Return(kTestJid));
+    EXPECT_CALL(mock_unknown_host_id_error_callback_, Run())
+        .Times(0);
 
     heartbeat_sender_.reset(new HeartbeatSender(
-        &mock_listener_, kHostId, &signal_strategy_, key_pair_, kTestBotJid));
+        mock_heartbeat_successful_callback_.GetCallback(),
+        mock_unknown_host_id_error_callback_.GetCallback(),
+        kHostId, &signal_strategy_, key_pair_, kTestBotJid));
   }
 
   virtual void TearDown() override {
     heartbeat_sender_.reset();
     EXPECT_TRUE(signal_strategy_listeners_.empty());
   }
 
   void ValidateHeartbeatStanza(XmlElement* stanza,
                                const char* expected_sequence_id,
                                const char* expected_host_offline_reason);
 
   void ProcessResponseWithInterval(
     bool is_offline_heartbeat_response,
     int interval);
 
   base::MessageLoop message_loop_;
   MockSignalStrategy signal_strategy_;
-  MockListener mock_listener_;
+  MockClosure mock_heartbeat_successful_callback_;
+  MockClosure mock_unknown_host_id_error_callback_;
   std::set<SignalStrategy::Listener*> signal_strategy_listeners_;
   scoped_refptr<RsaKeyPair> key_pair_;
   scoped_ptr<HeartbeatSender> heartbeat_sender_;
 };
 
 // Call Start() followed by Stop(), and make sure a valid heartbeat is sent.
 TEST_F(HeartbeatSenderTest, DoSendStanza) {
   XmlElement* sent_iq = NULL;
   EXPECT_CALL(signal_strategy_, GetLocalJid())
       .WillRepeatedly(Return(kTestJid));
@@ skipping 122 matching lines @@
   result->AddElement(set_interval);
 
   set_interval->AddText(base::IntToString(interval));
 
   heartbeat_sender_->ProcessResponse(
       is_offline_heartbeat_response, NULL, response.get());
 }
 
 // Verify that ProcessResponse parses set-interval result.
 TEST_F(HeartbeatSenderTest, ProcessResponseSetInterval) {
-  EXPECT_CALL(mock_listener_, OnHeartbeatSuccessful());
+  EXPECT_CALL(mock_heartbeat_successful_callback_, Run());
 
   ProcessResponseWithInterval(false, kTestInterval);
 
   EXPECT_EQ(kTestInterval * 1000, heartbeat_sender_->interval_ms_);
 }
 
 // Make sure SetHostOfflineReason sends a correct stanza.
 TEST_F(HeartbeatSenderTest, DoSetHostOfflineReason) {
   XmlElement* sent_iq = NULL;
   EXPECT_CALL(signal_strategy_, GetLocalJid())
@@ skipping 13 matching lines @@
   base::RunLoop().RunUntilIdle();
 
   scoped_ptr<XmlElement> stanza(sent_iq);
   ASSERT_TRUE(stanza != NULL);
   ValidateHeartbeatStanza(stanza.get(), "0", "test_error");
 
   heartbeat_sender_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED);
   base::RunLoop().RunUntilIdle();
 }
 
-static void MarkCallbackAsRun(bool* didCallbackRun) {
-  *didCallbackRun = true;
-}
-
 // Make sure SetHostOfflineReason triggers a callback when bot responds.
 TEST_F(HeartbeatSenderTest, ProcessHostOfflineResponses) {
+  MockClosure mock_ack_callback;
+
   EXPECT_CALL(signal_strategy_, GetLocalJid())
       .WillRepeatedly(Return(kTestJid));
   EXPECT_CALL(signal_strategy_, GetNextId())
       .WillOnce(Return(kStanzaId));
   EXPECT_CALL(signal_strategy_, SendStanzaPtr(NotNull()))
       .WillOnce(Return(true));
   EXPECT_CALL(signal_strategy_, GetState())
       .WillOnce(Return(SignalStrategy::DISCONNECTED))
       .WillRepeatedly(Return(SignalStrategy::CONNECTED));
-  EXPECT_CALL(mock_listener_, OnHeartbeatSuccessful())
+  EXPECT_CALL(mock_heartbeat_successful_callback_, Run())
       .WillRepeatedly(Return());
 
-  bool didCallbackRun = false;
+  // Callback should not run, until response to offline-reason.
+  EXPECT_CALL(mock_ack_callback, Run()).Times(0);
+
   heartbeat_sender_->SetHostOfflineReason(
       "test_error",
-      base::Bind(MarkCallbackAsRun, base::Unretained(&didCallbackRun)));
-  ASSERT_FALSE(didCallbackRun);
+      mock_ack_callback.GetCallback());
   heartbeat_sender_->OnSignalStrategyStateChange(SignalStrategy::CONNECTED);
   base::RunLoop().RunUntilIdle();
-  ASSERT_FALSE(didCallbackRun) << "Callback shouldn't run before ack from bot";
 
   ProcessResponseWithInterval(
-      false /* <- this not a response to offline-reason*/,
+      false,  // <- This is not a response to offline-reason.
       kTestInterval);
   base::RunLoop().RunUntilIdle();
-  ASSERT_FALSE(didCallbackRun) << "Callback shouldn't run "
-      << "when getting a response to an earlier, non-offline stanza";
 
+  // Callback should run once, when we get response to offline-reason.
+  EXPECT_CALL(mock_ack_callback, Run()).Times(1);
+  ProcessResponseWithInterval(
+      true,  // <- This is a response to offline-reason.
+      kTestInterval);
+  base::RunLoop().RunUntilIdle();
+
+  // When subsequent responses to offline-reason come,
+  // the callback should not be called again.
+  EXPECT_CALL(mock_ack_callback, Run()).Times(0);
   ProcessResponseWithInterval(true, kTestInterval);
   base::RunLoop().RunUntilIdle();
-  ASSERT_TRUE(didCallbackRun) << "Ack from bot should trigger the callback";
-
-  didCallbackRun = false;
-  ProcessResponseWithInterval(true, kTestInterval);
-  base::RunLoop().RunUntilIdle();
-  ASSERT_FALSE(didCallbackRun) << "Callback should only run once";
 
   heartbeat_sender_->OnSignalStrategyStateChange(SignalStrategy::DISCONNECTED);
   base::RunLoop().RunUntilIdle();
 }
 
 // Validate a heartbeat stanza.
 void HeartbeatSenderTest::ValidateHeartbeatStanza(
     XmlElement* stanza,
     const char* expected_sequence_id,
     const char* expected_host_offline_reason) {
@@ skipping 21 matching lines @@
   EXPECT_TRUE(heartbeat_stanza->NextNamed(signature_tag) == NULL);
 
   scoped_refptr<RsaKeyPair> key_pair = RsaKeyPair::FromString(kTestRsaKeyPair);
   ASSERT_TRUE(key_pair.get());
   std::string expected_signature =
       key_pair->SignMessage(std::string(kTestJid) + ' ' + expected_sequence_id);
   EXPECT_EQ(expected_signature, signature->BodyText());
 }
 
 }  // namespace remoting