[Chromoting] Add SessionPlugin in JingleSession

remoting/protocol/jingle_session_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/protocol/jingle_session.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
 #include "base/test/test_timeouts.h"
 #include "base/time/time.h"
 #include "net/socket/socket.h"
 #include "net/socket/stream_socket.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "remoting/base/constants.h"
 #include "remoting/protocol/authenticator.h"
 #include "remoting/protocol/channel_authenticator.h"
 #include "remoting/protocol/chromium_port_allocator_factory.h"
 #include "remoting/protocol/connection_tester.h"
 #include "remoting/protocol/fake_authenticator.h"
 #include "remoting/protocol/jingle_session_manager.h"
 #include "remoting/protocol/network_settings.h"
+#include "remoting/protocol/session_plugin.h"
 #include "remoting/protocol/transport.h"
 #include "remoting/protocol/transport_context.h"
 #include "remoting/signaling/fake_signal_strategy.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/webrtc/libjingle/xmpp/constants.h"
 
 using testing::_;
 using testing::AtLeast;
 using testing::AtMost;
@@ skipping 56 matching lines @@
       on_message_callback_.Run();
     return true;
   }
 
  private:
   SendTransportInfoCallback send_transport_info_callback_;
   std::vector<std::unique_ptr<buzz::XmlElement>> received_messages_;
   base::Closure on_message_callback_;
 };
 
+class FakePlugin : public SessionPlugin {
+ public:
+   std::unique_ptr<buzz::XmlElement> OnOutgoing(
+       Session::State state,
+       JingleMessage::ActionType action) override {
+    on_outgoing_count_++;
+    return nullptr;
+  }
+
+  void OnIncoming(Session::State state,
+                  JingleMessage::ActionType action,
+                  const buzz::XmlElement* attachments) override {
+    on_incoming_count_++;
+  }
+
+  int on_outgoing_count() const {
+    return on_outgoing_count_;
+  }
+
+  int on_incoming_count() const {
+    return on_incoming_count_;
+  }
+
+ private:
+  int on_outgoing_count_ = 0;
+  int on_incoming_count_ = 0;
+};
+
 std::unique_ptr<buzz::XmlElement> CreateTransportInfo(const std::string& id) {
   std::unique_ptr<buzz::XmlElement> result(
       buzz::XmlElement::ForStr("<transport xmlns='google:remoting:ice'/>"));
   result->AddAttr(buzz::QN_ID, id);
   return result;
 }
 
 }  // namespace
 
 class JingleSessionTest : public testing::Test {
  public:
   JingleSessionTest() {
     message_loop_.reset(new base::MessageLoopForIO());
     network_settings_ =
         NetworkSettings(NetworkSettings::NAT_TRAVERSAL_OUTGOING);
   }
 
   // Helper method that handles OnIncomingSession().
   void SetHostSession(Session* session) {
     DCHECK(session);
     host_session_.reset(session);
     host_session_->SetEventHandler(&host_session_event_handler_);
     host_session_->SetTransport(&host_transport_);
+    std::unique_ptr<FakePlugin> plugin(new FakePlugin());
+    host_plugin_ = plugin.get();
+    host_session_->Attach(std::move(plugin));
   }
 
   void DeleteHostSession() { host_session_.reset(); }
 
   void DeleteClientSession() { client_session_.reset(); }
 
  protected:
   void TearDown() override {
     CloseSessions();
     CloseSessionManager();
@@ skipping 93 matching lines @@
                   OnSessionStateChange(Session::CLOSED))
           .Times(AtMost(1));
     }
   }
 
   void ConnectClient(std::unique_ptr<Authenticator> authenticator) {
     client_session_ =
         client_server_->Connect(host_jid_, std::move(authenticator));
     client_session_->SetEventHandler(&client_session_event_handler_);
     client_session_->SetTransport(&client_transport_);
+    std::unique_ptr<FakePlugin> plugin(new FakePlugin());
+    client_plugin_ = plugin.get();
+    client_session_->Attach(std::move(plugin));
     base::RunLoop().RunUntilIdle();
   }
 
   void InitiateConnection(int auth_round_trips,
                           FakeAuthenticator::Action auth_action,
                           bool expect_fail) {
     SetHostExpectation(expect_fail);
     SetClientExpectation(expect_fail);
     ConnectClient(base::MakeUnique<FakeAuthenticator>(
         FakeAuthenticator::CLIENT, auth_round_trips, auth_action, true));
@@ skipping 20 matching lines @@
   std::unique_ptr<JingleSessionManager> host_server_;
   MockSessionManagerListener host_server_listener_;
   std::unique_ptr<JingleSessionManager> client_server_;
 
   std::unique_ptr<Session> host_session_;
   MockSessionEventHandler host_session_event_handler_;
   FakeTransport host_transport_;
   std::unique_ptr<Session> client_session_;
   MockSessionEventHandler client_session_event_handler_;
   FakeTransport client_transport_;
+
+  FakePlugin* host_plugin_;
+  FakePlugin* client_plugin_;
 };
 
 
 // Verify that we can create and destroy session managers without a
 // connection.
 TEST_F(JingleSessionTest, CreateAndDestoy) {
   CreateSessionManagers(1, FakeAuthenticator::ACCEPT);
 }
 
 // Verify that an incoming session can be rejected, and that the
@@ skipping 258 matching lines @@
   // Resume authentication.
   authenticator->Resume();
   base::RunLoop().RunUntilIdle();
 
   // Verify that transport-info that the first transport-info message was
   // received.
   ASSERT_EQ(client_transport_.received_messages().size(), 1U);
   EXPECT_EQ("1", client_transport_.received_messages()[0]->Attr(buzz::QN_ID));
 }
 
+TEST_F(JingleSessionTest, TestSessionPlugin) {

[Sergey Ulanov, 2016/12/21 01:38:34]

This test doesn't verify that the messages are being passed between the plugins,
and I think it's the most important part.

[Hzj_jie, 2016/12/22 00:27:11]

Done.

+  CreateSessionManagers(3, FakeAuthenticator::ACCEPT);
+  ASSERT_NO_FATAL_FAILURE(
+      InitiateConnection(3, FakeAuthenticator::ACCEPT, false));
+  // It's expected the plugins won't be able to be attached to the Session
+  // before the first connect message has been sent, which is controlled by
+  // JingleSessionManager.

[Sergey Ulanov, 2016/12/21 01:38:34]

I think this limitation needs to be mentioned somewhere beside the test. Maybe
in the comments for Session::AddPlugin()?
FWIW it should be easy to fix that, e.g. by sending session-initiate message
asynchronously after SessionManager::Connect() returns.

[Hzj_jie, 2016/12/22 00:27:10]

Done.

+  ASSERT_EQ(client_plugin_->on_outgoing_count(), 2);

[Sergey Ulanov, 2016/12/21 01:38:34]

This essentially verifies how many messages are sent in each direction, but this
isn't related to plugins in any way. It would break if there are any changes
that affect number of messages and this looks wrong to me. The test should
verify that the plugins can send messages to each other and that the session can
still be connected. 

[Hzj_jie, 2016/12/22 00:27:10]

I have updated the test case to compare the xml elements from both side.
I think whether the session is possible to be connected can be covered by other
test cases, since plugins are always attached to Session in SetHostSession() and
ConnectClient().

[Sergey Ulanov, 2016/12/23 00:14:51]

Currently the test doesn't verify that JingleSession attaches messages and can
process them on the receiving end. That's the main problem here. It doesn't test
all plugin-related functionality.

[Hzj_jie, 2016/12/24 00:09:54]

The test has been updated in the last version already.

+  ASSERT_EQ(client_plugin_->on_incoming_count(), 3);
+  ASSERT_EQ(host_plugin_->on_outgoing_count(), 3);
+  ASSERT_EQ(host_plugin_->on_incoming_count(), 3);
+}
+
 }  // namespace protocol
 }  // namespace remoting