Unify the QUIC dispatcher and make the QuicServer work.

Initialize the QuicDispatcher with a PacketWriter instead of an int fd.
QuicDispatcher::Initialize() used to take an fd which chromium does not make
use of. Instead, it should simply take a PacketWriter* which the caller
should be responsible for creating. Renamed Initialize() to
InitializeWithWriter() to reflect this. Removed the CreateWriter() method
from the QuicDispatcher and moved it to QuicServer.
Merge internal change: 89495510

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

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

net/BUILD.gn

@@ -1145,8 +1145,6 @@ if (is_linux) {
       "tools/quic/quic_client.h",
       "tools/quic/quic_default_packet_writer.cc",
       "tools/quic/quic_default_packet_writer.h",
-      "tools/quic/quic_dispatcher.cc",
-      "tools/quic/quic_dispatcher.h",
       "tools/quic/quic_epoll_clock.cc",
       "tools/quic/quic_epoll_clock.h",
       "tools/quic/quic_epoll_connection_helper.cc",
@@ -1155,8 +1153,6 @@ if (is_linux) {
       "tools/quic/quic_packet_reader.h",
       "tools/quic/quic_packet_writer_wrapper.cc",
       "tools/quic/quic_packet_writer_wrapper.h",
-      "tools/quic/quic_per_connection_packet_writer.cc",
-      "tools/quic/quic_per_connection_packet_writer.h",
       "tools/quic/quic_server.cc",
       "tools/quic/quic_server.h",
       "tools/quic/quic_socket_utils.cc",
@@ -1229,8 +1225,6 @@ if (is_android || is_linux) {
 # such as when all dependees are not buildable for the chosen OS.
 source_set("quic_tools") {
   sources = [
-    "quic/quic_dispatcher.cc",
-    "quic/quic_dispatcher.h",
     "quic/quic_per_connection_packet_writer.cc",
     "quic/quic_per_connection_packet_writer.h",
     "quic/quic_server.cc",
@@ -1250,8 +1244,12 @@ source_set("simple_quic_tools") {
   sources = [
     "tools/quic/quic_client_session.cc",
     "tools/quic/quic_client_session.h",
+    "tools/quic/quic_dispatcher.cc",
+    "tools/quic/quic_dispatcher.h",
     "tools/quic/quic_in_memory_cache.cc",
     "tools/quic/quic_in_memory_cache.h",
+    "tools/quic/quic_per_connection_packet_writer.cc",
+    "tools/quic/quic_per_connection_packet_writer.h",
     "tools/quic/quic_server_session.cc",
     "tools/quic/quic_server_session.h",
     "tools/quic/quic_simple_client.cc",

net/net.gyp

@@ -785,8 +785,6 @@
         'simple_quic_tools',
       ],
       'sources': [
-        'quic/quic_dispatcher.cc',
-        'quic/quic_dispatcher.h',
         'quic/quic_per_connection_packet_writer.cc',
         'quic/quic_per_connection_packet_writer.h',
         'quic/quic_server.cc',
@@ -805,14 +803,18 @@
         'net',
       ],
       'sources': [
+        'tools/quic/quic_client_session.cc',
+        'tools/quic/quic_client_session.h',
+        'tools/quic/quic_dispatcher.cc',
+        'tools/quic/quic_dispatcher.h',
         'tools/quic/quic_in_memory_cache.cc',
         'tools/quic/quic_in_memory_cache.h',
+        'tools/quic/quic_per_connection_packet_writer.cc',
+        'tools/quic/quic_per_connection_packet_writer.h',
         'tools/quic/quic_server_session.cc',
         'tools/quic/quic_server_session.h',
         'tools/quic/quic_simple_client.cc',
         'tools/quic/quic_simple_client.h',
-        'tools/quic/quic_client_session.cc',
-        'tools/quic/quic_client_session.h',
         'tools/quic/quic_spdy_client_stream.cc',
         'tools/quic/quic_spdy_client_stream.h',
         'tools/quic/quic_spdy_server_stream.cc',
@@ -1240,8 +1242,6 @@
             'tools/quic/quic_client.h',
             'tools/quic/quic_default_packet_writer.cc',
             'tools/quic/quic_default_packet_writer.h',
-            'tools/quic/quic_dispatcher.cc',
-            'tools/quic/quic_dispatcher.h',
             'tools/quic/quic_epoll_clock.cc',
             'tools/quic/quic_epoll_clock.h',
             'tools/quic/quic_epoll_connection_helper.cc',
@@ -1250,8 +1250,6 @@
             'tools/quic/quic_packet_reader.h',
             'tools/quic/quic_packet_writer_wrapper.cc',
             'tools/quic/quic_packet_writer_wrapper.h',
-            'tools/quic/quic_per_connection_packet_writer.cc',
-            'tools/quic/quic_per_connection_packet_writer.h',
             'tools/quic/quic_server.cc',
             'tools/quic/quic_server.h',
             'tools/quic/quic_socket_utils.cc',

net/quic/quic_server.cc

@@ -12,13 +12,16 @@
 #include "net/quic/crypto/quic_random.h"
 #include "net/quic/quic_crypto_stream.h"
 #include "net/quic/quic_data_reader.h"
-#include "net/quic/quic_dispatcher.h"
+#include "net/quic/quic_per_connection_packet_writer.h"
 #include "net/quic/quic_protocol.h"
 #include "net/quic/quic_server_packet_writer.h"
+#include "net/tools/quic/quic_dispatcher.h"
 #include "net/udp/udp_server_socket.h"
 
 namespace net {
 
+using tools::QuicDispatcher;
+
 namespace {
 
 const char kSourceAddressTokenSecret[] = "secret";
@@ -27,6 +30,35 @@ const char kSourceAddressTokenSecret[] = "secret";
 // the limit.
 const int kReadBufferSize = 2 * kMaxPacketSize;
 
+// A packet writer factory which wraps a shared QuicServerPacketWriter
+// inside of a QuicPerConnectionPacketWriter. Instead of checking that
+// the shared_writer is the expected writer, this could instead cast
+// from QuicPacketWriter to QuicServerPacketWriter.
+class CustomPacketWriterFactory : public QuicDispatcher::PacketWriterFactory {
+ public:
+  ~CustomPacketWriterFactory() override {}
+
+  QuicPacketWriter* Create(QuicPacketWriter* writer,
+                           QuicConnection* connection) override {
+    if (writer == nullptr) {
+      LOG(DFATAL) << "shared_writer not initialized";
+      return nullptr;
+    }
+    if (writer != shared_writer_) {
+      LOG(DFATAL) << "writer mismatch";
+      return nullptr;
+    }
+    return new QuicPerConnectionPacketWriter(shared_writer_, connection);
+  }
+
+  void set_shared_writer(QuicServerPacketWriter* shared_writer) {
+    shared_writer_ = shared_writer;
+  }
+
+ private:
+  QuicServerPacketWriter* shared_writer_;  // Not owned.
+};
+
 } // namespace
 
 QuicServer::QuicServer(const QuicConfig& config,
@@ -111,16 +143,18 @@ int QuicServer::Listen(const IPEndPoint& address) {
 
   socket_.swap(socket);
 
+  CustomPacketWriterFactory* factory = new CustomPacketWriterFactory();
   dispatcher_.reset(
       new QuicDispatcher(config_,
                          crypto_config_,
                          supported_versions_,
-                         new QuicDispatcher::DefaultPacketWriterFactory(),
+                         factory,
                          &helper_));
   QuicServerPacketWriter* writer = new QuicServerPacketWriter(
       socket_.get(),
       dispatcher_.get());
-  dispatcher_->Initialize(writer);
+  factory->set_shared_writer(writer);
+  dispatcher_->InitializeWithWriter(writer);
 
   StartReading();
 

net/quic/quic_server.h

@@ -25,7 +25,10 @@ namespace test {
 class QuicServerPeer;
 }  // namespace test
 
+namespace tools {
 class QuicDispatcher;
+}  // namespace tools
+
 class UDPServerSocket;
 
 class QuicServer {
@@ -59,7 +62,7 @@ class QuicServer {
     crypto_config_.SetProofSource(source);
   }
 
-  QuicDispatcher* dispatcher() { return dispatcher_.get(); }
+  tools::QuicDispatcher* dispatcher() { return dispatcher_.get(); }
 
  private:
   friend class net::test::QuicServerPeer;
@@ -68,7 +71,7 @@ class QuicServer {
   void Initialize();
 
   // Accepts data from the framer and demuxes clients to sessions.
-  scoped_ptr<QuicDispatcher> dispatcher_;
+  scoped_ptr<tools::QuicDispatcher> dispatcher_;
 
   // Used by the helper_ to time alarms.
   QuicClock clock_;

net/quic/quic_server_test.cc

@@ -24,9 +24,9 @@ class QuicChromeServerDispatchPacketTest : public ::testing::Test {
       : crypto_config_("blah", QuicRandom::GetInstance()),
         dispatcher_(config_,
                     crypto_config_,
-                    new QuicDispatcher::DefaultPacketWriterFactory(),
-                    &helper_) {
-    dispatcher_.Initialize(nullptr);
+                    new tools::QuicDispatcher::DefaultPacketWriterFactory(),
+                    new MockHelper) {
+    dispatcher_.InitializeWithWriter(nullptr);
   }
 
   void DispatchPacket(const QuicEncryptedPacket& packet) {
@@ -37,7 +37,6 @@ class QuicChromeServerDispatchPacketTest : public ::testing::Test {
  protected:
   QuicConfig config_;
   QuicCryptoServerConfig crypto_config_;
-  MockHelper helper_;
   MockQuicDispatcher dispatcher_;
 };
 

net/quic/test_tools/mock_quic_dispatcher.h

@@ -8,14 +8,14 @@
 #include "net/base/ip_endpoint.h"
 #include "net/quic/crypto/quic_crypto_server_config.h"
 #include "net/quic/quic_config.h"
-#include "net/quic/quic_dispatcher.h"
 #include "net/quic/quic_protocol.h"
+#include "net/tools/quic/quic_dispatcher.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace net {
 namespace test {
 
-class MockQuicDispatcher : public QuicDispatcher {
+class MockQuicDispatcher : public tools::QuicDispatcher {
  public:
   MockQuicDispatcher(const QuicConfig& config,
                      const QuicCryptoServerConfig& crypto_config,

net/quic/test_tools/quic_test_utils.cc

@@ -19,6 +19,7 @@
 #include "net/quic/quic_utils.h"
 #include "net/quic/test_tools/quic_connection_peer.h"
 #include "net/spdy/spdy_frame_builder.h"
+#include "net/tools/quic/quic_per_connection_packet_writer.h"
 
 using base::StringPiece;
 using std::max;
@@ -667,7 +668,7 @@ QuicVersionVector SupportedVersions(QuicVersion version) {
 TestWriterFactory::TestWriterFactory() : current_writer_(nullptr) {}
 TestWriterFactory::~TestWriterFactory() {}
 
-QuicPacketWriter* TestWriterFactory::Create(QuicServerPacketWriter* writer,
+QuicPacketWriter* TestWriterFactory::Create(QuicPacketWriter* writer,
                                             QuicConnection* connection) {
   return new PerConnectionPacketWriter(this, writer, connection);
 }
@@ -687,7 +688,7 @@ void TestWriterFactory::Unregister(PerConnectionPacketWriter* writer) {
 
 TestWriterFactory::PerConnectionPacketWriter::PerConnectionPacketWriter(
     TestWriterFactory* factory,
-    QuicServerPacketWriter* writer,
+    QuicPacketWriter* writer,
     QuicConnection* connection)
     : QuicPerConnectionPacketWriter(writer, connection),
       factory_(factory) {
@@ -707,7 +708,7 @@ WriteResult TestWriterFactory::PerConnectionPacketWriter::WritePacket(
   // in a different way, so TestWriterFactory::OnPacketSent might never be
   // called.
   factory_->current_writer_ = this;
-  return QuicPerConnectionPacketWriter::WritePacket(buffer,
+  return tools::QuicPerConnectionPacketWriter::WritePacket(buffer,
                                                     buf_len,
                                                     self_address,
                                                     peer_address);

net/quic/test_tools/quic_test_utils.h

@@ -17,15 +17,15 @@
 #include "net/quic/quic_ack_notifier.h"
 #include "net/quic/quic_client_session_base.h"
 #include "net/quic/quic_connection.h"
-#include "net/quic/quic_dispatcher.h"
 #include "net/quic/quic_framer.h"
-#include "net/quic/quic_per_connection_packet_writer.h"
 #include "net/quic/quic_protocol.h"
 #include "net/quic/quic_sent_packet_manager.h"
 #include "net/quic/quic_session.h"
 #include "net/quic/test_tools/mock_clock.h"
 #include "net/quic/test_tools/mock_random.h"
 #include "net/spdy/spdy_framer.h"
+#include "net/tools/quic/quic_dispatcher.h"
+#include "net/tools/quic/quic_per_connection_packet_writer.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace net {
@@ -586,12 +586,12 @@ class MockNetworkChangeVisitor :
 // Creates per-connection packet writers that register themselves with the
 // TestWriterFactory on each write so that TestWriterFactory::OnPacketSent can
 // be routed to the appropriate QuicConnection.
-class TestWriterFactory : public QuicDispatcher::PacketWriterFactory {
+class TestWriterFactory : public tools::QuicDispatcher::PacketWriterFactory {
  public:
   TestWriterFactory();
   ~TestWriterFactory() override;
 
-  QuicPacketWriter* Create(QuicServerPacketWriter* writer,
+  QuicPacketWriter* Create(QuicPacketWriter* writer,
                            QuicConnection* connection) override;
 
   // Calls OnPacketSent on the last QuicConnection to write through one of the
@@ -599,10 +599,11 @@ class TestWriterFactory : public QuicDispatcher::PacketWriterFactory {
   void OnPacketSent(WriteResult result);
 
  private:
-  class PerConnectionPacketWriter : public QuicPerConnectionPacketWriter {
+  class PerConnectionPacketWriter
+      : public tools::QuicPerConnectionPacketWriter {
    public:
     PerConnectionPacketWriter(TestWriterFactory* factory,
-                              QuicServerPacketWriter* writer,
+                              QuicPacketWriter* writer,
                               QuicConnection* connection);
     ~PerConnectionPacketWriter() override;
 

net/tools/quic/quic_dispatcher.cc

@@ -11,11 +11,7 @@
 #include "net/quic/quic_blocked_writer_interface.h"
 #include "net/quic/quic_flags.h"
 #include "net/quic/quic_utils.h"
-#include "net/tools/epoll_server/epoll_server.h"
-#include "net/tools/quic/quic_default_packet_writer.h"
-#include "net/tools/quic/quic_epoll_connection_helper.h"
 #include "net/tools/quic/quic_per_connection_packet_writer.h"
-#include "net/tools/quic/quic_socket_utils.h"
 #include "net/tools/quic/quic_time_wait_list_manager.h"
 
 namespace net {
@@ -193,9 +189,9 @@ QuicDispatcher::~QuicDispatcher() {
   STLDeleteElements(&closed_session_list_);
 }
 
-void QuicDispatcher::Initialize(int fd) {
+void QuicDispatcher::InitializeWithWriter(QuicPacketWriter* writer) {
   DCHECK(writer_ == nullptr);
-  writer_.reset(CreateWriter(fd));
+  writer_.reset(writer);
   time_wait_list_manager_.reset(CreateQuicTimeWaitListManager());
 }
 
@@ -401,10 +397,6 @@ void QuicDispatcher::OnConnectionRemovedFromTimeWaitList(
   DVLOG(1) << "Connection " << connection_id << " removed from time wait list.";
 }
 
-QuicPacketWriter* QuicDispatcher::CreateWriter(int fd) {
-  return new QuicDefaultPacketWriter(fd);
-}
-
 QuicServerSession* QuicDispatcher::CreateQuicSession(
     QuicConnectionId connection_id,
     const IPEndPoint& server_address,

net/tools/quic/quic_dispatcher.h

@@ -41,7 +41,8 @@ class ProcessPacketInterface {
 };
 
 class QuicDispatcher : public QuicServerSessionVisitor,
-                       public ProcessPacketInterface {
+                       public ProcessPacketInterface,
+                       public QuicBlockedWriterInterface {
  public:
   // Creates per-connection packet writers out of the QuicDispatcher's shared
   // QuicPacketWriter. The per-connection writers' IsWriteBlocked() state must
@@ -80,7 +81,8 @@ class QuicDispatcher : public QuicServerSessionVisitor,
 
   ~QuicDispatcher() override;
 
-  virtual void Initialize(int fd);
+  // Takes ownership of |writer|.
+  void InitializeWithWriter(QuicPacketWriter* writer);
 
   // Process the incoming packet by creating a new session, passing it to
   // an existing session, or passing it to the TimeWaitListManager.
@@ -89,7 +91,7 @@ class QuicDispatcher : public QuicServerSessionVisitor,
                      const QuicEncryptedPacket& packet) override;
 
   // Called when the socket becomes writable to allow queued writes to happen.
-  virtual void OnCanWrite();
+  void OnCanWrite() override;
 
   // Returns true if there's anything in the blocked writer list.
   virtual bool HasPendingWrites() const;
@@ -122,10 +124,6 @@ class QuicDispatcher : public QuicServerSessionVisitor,
   void DeleteSessions();
 
  protected:
-  // Instantiates a new low-level packet writer. Caller takes ownership of the
-  // returned object.
-  virtual QuicPacketWriter* CreateWriter(int fd);
-
   virtual QuicServerSession* CreateQuicSession(
       QuicConnectionId connection_id,
       const IPEndPoint& server_address,

net/tools/quic/quic_dispatcher_test.cc

@@ -125,7 +125,7 @@ class QuicDispatcherTest : public ::testing::Test {
         time_wait_list_manager_(nullptr),
         session1_(nullptr),
         session2_(nullptr) {
-    dispatcher_.Initialize(1);
+    dispatcher_.InitializeWithWriter(new QuicDefaultPacketWriter(1));
   }
 
   ~QuicDispatcherTest() override {}

net/tools/quic/quic_server.cc

@@ -183,11 +183,15 @@ bool QuicServer::Listen(const IPEndPoint& address) {
 
   epoll_server_.RegisterFD(fd_, this, kEpollFlags);
   dispatcher_.reset(CreateQuicDispatcher());
-  dispatcher_->Initialize(fd_);
+  dispatcher_->InitializeWithWriter(CreateWriter(fd_));
 
   return true;
 }
 
+QuicDefaultPacketWriter* QuicServer::CreateWriter(int fd) {
+  return new QuicDefaultPacketWriter(fd);
+}
+
 QuicDispatcher* QuicServer::CreateQuicDispatcher() {
   return new QuicDispatcher(
       config_,

net/tools/quic/quic_server.h

@@ -16,6 +16,7 @@
 #include "net/quic/quic_connection_helper.h"
 #include "net/quic/quic_framer.h"
 #include "net/tools/epoll_server/epoll_server.h"
+#include "net/tools/quic/quic_default_packet_writer.h"
 
 namespace net {
 
@@ -81,6 +82,8 @@ class QuicServer : public EpollCallbackInterface {
   int port() { return port_; }
 
  protected:
+  virtual QuicDefaultPacketWriter* CreateWriter(int fd);
+
   virtual QuicDispatcher* CreateQuicDispatcher();
 
   const QuicConfig& config() const { return config_; }

net/tools/quic/quic_server_test.cc

@@ -26,7 +26,7 @@ class QuicServerDispatchPacketTest : public ::testing::Test {
                     crypto_config_,
                     new QuicDispatcher::DefaultPacketWriterFactory(),
                     new QuicEpollConnectionHelper(&eps_)) {
-    dispatcher_.Initialize(1234);
+    dispatcher_.InitializeWithWriter(new QuicDefaultPacketWriter(1234));
   }
 
   void DispatchPacket(const QuicEncryptedPacket& packet) {