QUIC - Added IsReadyToPersist method to check if the QuicServerInfo is ready to persist data.

Fixed comments from wtc:
  https://codereview.chromium.org/196343017/diff/1/net/http/disk_cache_based_quic_server_info.cc

R=wtc@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258231 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 25 insertions, 15 deletions

diff --git a/net/http/disk_cache_based_quic_server_info.cc b/net/http/disk_cache_based_quic_server_info.cc
index 7c92b0c..1c7147d 100644
--- a/net/http/disk_cache_based_quic_server_info.cc
+++ b/net/http/disk_cache_based_quic_server_info.cc
@@ -88,7 +88,13 @@ int DiskCacheBasedQuicServerInfo::WaitForDataReady(
 }
 
 bool DiskCacheBasedQuicServerInfo::IsDataReady() {
-  return ready_ && state_ == NONE;
+  return ready_;
+}
+
+bool DiskCacheBasedQuicServerInfo::IsReadyToPersist() {
+  // The data can be persisted if it has been loaded from the disk cache
+  // and there are no pending writes.
+  return ready_ && new_data_.empty();
 }
 
 void DiskCacheBasedQuicServerInfo::Persist() {
diff --git a/net/http/disk_cache_based_quic_server_info.h b/net/http/disk_cache_based_quic_server_info.h
index a8c55690..3c384cc 100644
--- a/net/http/disk_cache_based_quic_server_info.h
+++ b/net/http/disk_cache_based_quic_server_info.h
@@ -34,6 +34,7 @@ class NET_EXPORT_PRIVATE DiskCacheBasedQuicServerInfo
   virtual void Start() OVERRIDE;
   virtual int WaitForDataReady(const CompletionCallback& callback) OVERRIDE;
   virtual bool IsDataReady() OVERRIDE;
+  virtual bool IsReadyToPersist() OVERRIDE;
   virtual void Persist() OVERRIDE;
 
  private:
diff --git a/net/http/disk_cache_based_quic_server_info_unittest.cc b/net/http/disk_cache_based_quic_server_info_unittest.cc
index 366f73b..33212400 100644
--- a/net/http/disk_cache_based_quic_server_info_unittest.cc
+++ b/net/http/disk_cache_based_quic_server_info_unittest.cc
@@ -118,9 +118,9 @@ TEST(DiskCacheBasedQuicServerInfo, Update) {
   quic_server_info->Start();
   rv = quic_server_info->WaitForDataReady(callback.callback());
   EXPECT_EQ(net::OK, callback.GetResult(rv));
+  EXPECT_TRUE(quic_server_info->IsDataReady());
 
   const net::QuicServerInfo::State& state1 = quic_server_info->state();
-  EXPECT_TRUE(quic_server_info->IsDataReady());
   EXPECT_EQ(server_config_a, state1.server_config);
   EXPECT_EQ(source_address_token_a, state1.source_address_token);
   EXPECT_EQ(server_config_sig_a, state1.server_config_sig);
@@ -192,9 +192,9 @@ TEST(DiskCacheBasedQuicServerInfo, UpdateDifferentPorts) {
   quic_server_info->Start();
   rv = quic_server_info->WaitForDataReady(callback.callback());
   EXPECT_EQ(net::OK, callback.GetResult(rv));
+  EXPECT_TRUE(quic_server_info->IsDataReady());
 
   const net::QuicServerInfo::State& state_a = quic_server_info->state();
-  EXPECT_TRUE(quic_server_info->IsDataReady());
   EXPECT_EQ(server_config_a, state_a.server_config);
   EXPECT_EQ(source_address_token_a, state_a.source_address_token);
   EXPECT_EQ(server_config_sig_a, state_a.server_config_sig);
@@ -207,9 +207,9 @@ TEST(DiskCacheBasedQuicServerInfo, UpdateDifferentPorts) {
   quic_server_info->Start();
   rv = quic_server_info->WaitForDataReady(callback.callback());
   EXPECT_EQ(net::OK, callback.GetResult(rv));
+  EXPECT_TRUE(quic_server_info->IsDataReady());
 
   const net::QuicServerInfo::State& state_b = quic_server_info->state();
-  EXPECT_TRUE(quic_server_info->IsDataReady());
   EXPECT_EQ(server_config_b, state_b.server_config);
   EXPECT_EQ(source_address_token_b, state_b.source_address_token);
   EXPECT_EQ(server_config_sig_b, state_b.server_config_sig);
@@ -220,8 +220,8 @@ TEST(DiskCacheBasedQuicServerInfo, UpdateDifferentPorts) {
   RemoveMockTransaction(&kHostInfoTransaction1);
 }
 
-// Test IsDataReady when there is a pending write.
-TEST(DiskCacheBasedQuicServerInfo, IsDataReady) {
+// Test IsReadyToPersist when there is a pending write.
+TEST(DiskCacheBasedQuicServerInfo, IsReadyToPersist) {
   MockHttpCache cache;
   AddMockTransaction(&kHostInfoTransaction1);
   net::TestCompletionCallback callback;
@@ -229,11 +229,11 @@ TEST(DiskCacheBasedQuicServerInfo, IsDataReady) {
   net::QuicSessionKey server_key("www.google.com", 443, true);
   scoped_ptr<net::QuicServerInfo> quic_server_info(
       new net::DiskCacheBasedQuicServerInfo(server_key, cache.http_cache()));
+  EXPECT_FALSE(quic_server_info->IsDataReady());
   quic_server_info->Start();
-  EXPECT_TRUE(quic_server_info->IsDataReady());
   int rv = quic_server_info->WaitForDataReady(callback.callback());
-  EXPECT_TRUE(quic_server_info->IsDataReady());
   EXPECT_EQ(net::OK, callback.GetResult(rv));
+  EXPECT_TRUE(quic_server_info->IsDataReady());
 
   net::QuicServerInfo::State* state = quic_server_info->mutable_state();
   EXPECT_TRUE(state->certs.empty());
@@ -246,17 +246,17 @@ TEST(DiskCacheBasedQuicServerInfo, IsDataReady) {
   state->source_address_token = source_address_token_a;
   state->server_config_sig = server_config_sig_a;
   state->certs.push_back(cert_a);
-  EXPECT_TRUE(quic_server_info->IsDataReady());
+  EXPECT_TRUE(quic_server_info->IsReadyToPersist());
   quic_server_info->Persist();
 
-  // Once we call Persist, IsDataReady should return false until Persist has
-  // completed.
-  EXPECT_FALSE(quic_server_info->IsDataReady());
+  // Once we call Persist, IsReadyToPersist should return false until Persist
+  // has completed.
+  EXPECT_FALSE(quic_server_info->IsReadyToPersist());
 
   // Wait until Persist() does the work.
   base::MessageLoop::current()->RunUntilIdle();
 
-  EXPECT_TRUE(quic_server_info->IsDataReady());
+  EXPECT_TRUE(quic_server_info->IsReadyToPersist());
 
   // Verify that the state was updated.
   quic_server_info.reset(
@@ -267,7 +267,6 @@ TEST(DiskCacheBasedQuicServerInfo, IsDataReady) {
   EXPECT_TRUE(quic_server_info->IsDataReady());
 
   const net::QuicServerInfo::State& state1 = quic_server_info->state();
-  EXPECT_TRUE(quic_server_info->IsDataReady());
   EXPECT_EQ(server_config_a, state1.server_config);
   EXPECT_EQ(source_address_token_a, state1.source_address_token);
   EXPECT_EQ(server_config_sig_a, state1.server_config_sig);
diff --git a/net/quic/crypto/quic_server_info.h b/net/quic/crypto/quic_server_info.h
index 51533ec..0f09f3b 100644
--- a/net/quic/crypto/quic_server_info.h
+++ b/net/quic/crypto/quic_server_info.h
@@ -49,6 +49,10 @@ class NET_EXPORT_PRIVATE QuicServerInfo {
   // doesn't have a pending callback).
   virtual bool IsDataReady() = 0;
 
+  // Returns true if the object is ready to persist data, in other words, if
+  // data is loaded from disk cache and ready and there are no pending writes.
+  virtual bool IsReadyToPersist() = 0;
+
   // Persist allows for the server information to be updated for future users.
   // This is a fire and forget operation: the caller may drop its reference
   // from this object and the store operation will still complete. This can
diff --git a/net/quic/quic_client_session.cc b/net/quic/quic_client_session.cc
index 5146b95..865c9b0 100644
--- a/net/quic/quic_client_session.cc
+++ b/net/quic/quic_client_session.cc
@@ -466,7 +466,7 @@ void QuicClientSession::OnProofValid(
     const QuicCryptoClientConfig::CachedState& cached) {
   DCHECK(cached.proof_valid());
 
-  if (!server_info_ || !server_info_->IsDataReady()) {
+  if (!server_info_ || !server_info_->IsReadyToPersist()) {
     return;
   }