Convert DnsTransactionTest to use SequencedSocketData

net/dns/dns_transaction_unittest.cc

Index: net/dns/dns_transaction_unittest.cc
diff --git a/net/dns/dns_transaction_unittest.cc b/net/dns/dns_transaction_unittest.cc
index 2693a5ca9cf4dbf3126c244e1923f5800a616c2e..e30c53c8a902213f415b08ddce292d7069a85a67 100644
--- a/net/dns/dns_transaction_unittest.cc
+++ b/net/dns/dns_transaction_unittest.cc
@@ -46,12 +46,12 @@ class DnsSocketData {
       *length = base::HostToNet16(query_->io_buffer()->size());
       writes_.push_back(MockWrite(mode,
                                   reinterpret_cast<const char*>(length.get()),
-                                  sizeof(uint16)));
+                                  sizeof(uint16), num_reads_and_writes()));
       lengths_.push_back(length.release());
     }
-    writes_.push_back(MockWrite(mode,
-                                query_->io_buffer()->data(),
-                                query_->io_buffer()->size()));
+    writes_.push_back(MockWrite(mode, query_->io_buffer()->data(),
+                                query_->io_buffer()->size(),
+                                num_reads_and_writes()));
   }
   ~DnsSocketData() {}
 
@@ -66,12 +66,12 @@ class DnsSocketData {
       *length = base::HostToNet16(tcp_length);
       reads_.push_back(MockRead(mode,
                                 reinterpret_cast<const char*>(length.get()),
-                                sizeof(uint16)));
+                                sizeof(uint16), num_reads_and_writes()));
       lengths_.push_back(length.release());
     }
-    reads_.push_back(MockRead(mode,
-                              response->io_buffer()->data(),
-                              response->io_buffer()->size()));
+    reads_.push_back(MockRead(mode, response->io_buffer()->data(),
+                              response->io_buffer()->size(),
+                              num_reads_and_writes()));
     responses_.push_back(response.release());
   }
 
@@ -102,7 +102,7 @@ class DnsSocketData {
 
   // Add error response.
   void AddReadError(int error, IoMode mode) {
-    reads_.push_back(MockRead(mode, error));
+    reads_.push_back(MockRead(mode, error, num_reads_and_writes()));
   }
 
   // Build, if needed, and return the SocketDataProvider. No new responses
@@ -112,9 +112,10 @@ class DnsSocketData {
       return provider_.get();
     // Terminate the reads with ERR_IO_PENDING to prevent overrun and default to
     // timeout.
-    reads_.push_back(MockRead(ASYNC, ERR_IO_PENDING));
-    provider_.reset(new DelayedSocketData(1, &reads_[0], reads_.size(),
-                                          &writes_[0], writes_.size()));
+    reads_.push_back(
+        MockRead(ASYNC, ERR_IO_PENDING, writes_.size() + reads_.size()));
+    provider_.reset(new SequencedSocketData(&reads_[0], reads_.size(),
+                                            &writes_[0], writes_.size()));
     if (use_tcp_) {
       provider_->set_connect_data(MockConnect(reads_[0].mode, OK));
     }
@@ -128,17 +129,19 @@ class DnsSocketData {
   // Returns true if the expected query was written to the socket.
   bool was_written() const {
     CHECK(provider_.get());
-    return provider_->write_index() > 0;
+    return provider_->WasDataWritten();

[mmenke, 2015/05/04 19:10:57]

For the TCP case, where there are two writes, this isn't write...err...right. 
WasAllDataWritten?  Or just call base::RunLoop().RunAllPending() first (Which is
ugly, but works).

[mmenke, 2015/05/04 19:10:57]

optional:  Seems a little weird for a lower-case named method to be calling a
capitalized one.  Suggest either renaming this, or renaming and inlining
was_data_written.  Latter seems simpler.

Google C++ style guide now says "You may also use lowercase letters for other
very short inlined functions. For example if a function were so cheap you would
not cache the value if you were calling it in a loop, then lowercase naming
would be acceptable."

That doesn't completely disallow this, but I'm assuming this is using lowercase
as an instance of the example, which does make it weird.

[Ryan Hamilton, 2015/05/04 19:20:30]

Good point! I've inlined WasDataWritten().

[Ryan Hamilton, 2015/05/04 19:24:46]

Whoops, sorry, missed this. I don't think I changed the semantics here, do you?
In the old code, we simply checked that write index was > 0, which happens as
soon as the write happen (even if it returns async). Now, the write index
doesn't move until the callback completes so we need to change the
implementation. Do you think I changed the semantics, or do you think that this
was a latent bug?

(What would calling RunAllPending do?)

[mmenke, 2015/05/04 19:34:49]

Ahh...You're right!  I saw the "at_write_eof" just below  WasDataWritten(), and
was assuming that's what the old code did.  I think this was a bug in the test. 
This CL is small enough that I'd like to see if we can fix here, if we can do so
easily and you don't mind (Since these tests are probably never going to be
updated otherwise).  If you prefer not to, I can live with that.
If we aren't at the "write eof", because there's a pending write, it would cause
the write to complete, so we *would* be at the write eof, so you can then just
use at_write_eof (And can probably check the read one, too, though that's less
important).

   }
 
  private:
+  size_t num_reads_and_writes() { return reads_.size() + writes_.size(); }

[mmenke, 2015/05/04 19:10:57]

nit:  const

[Ryan Hamilton, 2015/05/04 19:20:30]

Done.

+
   scoped_ptr<DnsQuery> query_;
   bool use_tcp_;
   ScopedVector<uint16> lengths_;
   ScopedVector<DnsResponse> responses_;
   std::vector<MockWrite> writes_;
   std::vector<MockRead> reads_;
-  scoped_ptr<DelayedSocketData> provider_;
+  scoped_ptr<SequencedSocketData> provider_;
 
   DISALLOW_COPY_AND_ASSIGN(DnsSocketData);
 };