Add unittests for BufferedSocketWriter and fix some bugs in that code.

remoting/protocol/buffered_socket_writer.h

 // 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.
 
 #ifndef REMOTING_PROTOCOL_BUFFERED_SOCKET_WRITER_H_
 #define REMOTING_PROTOCOL_BUFFERED_SOCKET_WRITER_H_
 
 #include <list>
 
 #include "base/callback.h"
@@ skipping 49 matching lines @@
 
  protected:
   class PendingPacket;
   typedef std::list<PendingPacket*> DataQueue;
 
   DataQueue queue_;
   int buffer_size_;
 
   // Removes element from the front of the queue and calls |done_task|
   // for that element.
-  void PopQueue();
+  base::Closure PopQueue();
 

[simonmorris, 2012/07/31 20:19:50]

Maybe document what the caller may/should/must do with the returned
base::Closure?

[Sergey Ulanov, 2012/07/31 21:37:53]

Done.

   // Following three methods must be implemented in child classes.
-  // GetNextPacket() returns next packet that needs to be written to the
-  // socket. |buffer| must be set to NULL if there is nothing left in the queue.
+
+  // Returns next packet that needs to be written to the socket. |buffer| must

[simonmorris, 2012/07/31 20:19:50]

"|buffer| must" -> "*buffer will"? "Must" sounds like a constraint on the
caller. Or if the comment is addressed to the implementer of a derived class,
not a client of the base class, say "An implementation must...".

[Sergey Ulanov, 2012/07/31 21:37:53]

Done.

+  // be set to NULL if there is nothing left in the queue.
   virtual void GetNextPacket(net::IOBuffer** buffer, int* size) = 0;
-  virtual void AdvanceBufferPosition(int written) = 0;
+
+  // Returns closure that must be executed or null closure if the last write
+  // didn't complete any messages.
+  virtual base::Closure AdvanceBufferPosition(int written) = 0;
 
   // This method is called whenever there is an error writing to the socket.
   virtual void OnError(int result) = 0;
 
  private:
   void DoWrite();
+  void HandleWriteResult(int result, bool* write_again);
   void OnWritten(int result);
 
   // This method is called when an error is encountered.
   void HandleError(int result);
 
   net::Socket* socket_;
   WriteFailedCallback write_failed_callback_;
 
   bool write_pending_;
 
   bool closed_;
+
+  bool* destroyed_flag_;
 };
 
 class BufferedSocketWriter : public BufferedSocketWriterBase {
  public:
   BufferedSocketWriter();
   virtual ~BufferedSocketWriter();
 
  protected:
   virtual void GetNextPacket(net::IOBuffer** buffer, int* size) OVERRIDE;
-  virtual void AdvanceBufferPosition(int written) OVERRIDE;
+  virtual base::Closure AdvanceBufferPosition(int written) OVERRIDE;
   virtual void OnError(int result) OVERRIDE;
 
  private:
   scoped_refptr<net::DrainableIOBuffer> current_buf_;
 };
 
 class BufferedDatagramWriter : public BufferedSocketWriterBase {
  public:
   BufferedDatagramWriter();
   virtual ~BufferedDatagramWriter();
 
  protected:
   virtual void GetNextPacket(net::IOBuffer** buffer, int* size) OVERRIDE;
-  virtual void AdvanceBufferPosition(int written) OVERRIDE;
+  virtual base::Closure AdvanceBufferPosition(int written) OVERRIDE;
   virtual void OnError(int result) OVERRIDE;
 };
 
 }  // namespace protocol
 }  // namespace remoting
 
 #endif  // REMOTING_PROTOCOL_BUFFERED_SOCKET_WRITER_H_