QUIC - close the disk cache entry after write is completed.

BUG=417835
R=rch@chromium.org

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

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

2 files changed, 89 insertions, 4 deletions

diff --git a/net/http/disk_cache_based_quic_server_info.cc b/net/http/disk_cache_based_quic_server_info.cc
index 13c53b7..0155700 100644
--- a/net/http/disk_cache_based_quic_server_info.cc
+++ b/net/http/disk_cache_based_quic_server_info.cc
@@ -92,6 +92,10 @@ bool DiskCacheBasedQuicServerInfo::IsDataReady() {
 }
 
 bool DiskCacheBasedQuicServerInfo::IsReadyToPersist() {
+  // TODO(rtenneti): Handle updates while a write is pending. Change
+  // Persist() to save the data to be written into a temporary buffer
+  // and then persist that data when we are ready to persist.
+  //
   // 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();
@@ -213,9 +217,7 @@ int DiskCacheBasedQuicServerInfo::DoReadComplete(int rv) {
 }
 
 int DiskCacheBasedQuicServerInfo::DoWriteComplete(int rv) {
-  // Keep the entry open for future writes.
-  new_data_.clear();
-  state_ = NONE;
+  state_ = SET_DONE;
   return OK;
 }
 
@@ -223,8 +225,11 @@ int DiskCacheBasedQuicServerInfo::DoCreateOrOpenComplete(int rv) {
   if (rv != OK) {
     state_ = SET_DONE;
   } else {
-    if (!entry_)
+    if (!entry_) {
       entry_ = data_shim_->entry;
+      found_entry_ = true;
+    }
+    DCHECK(entry_);
     state_ = WRITE;
   }
   return OK;
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 52b832d..9bd7462 100644
--- a/net/http/disk_cache_based_quic_server_info_unittest.cc
+++ b/net/http/disk_cache_based_quic_server_info_unittest.cc
@@ -279,4 +279,84 @@ TEST(DiskCacheBasedQuicServerInfo, IsReadyToPersist) {
   RemoveMockTransaction(&kHostInfoTransaction1);
 }
 
+// Test multiple calls to Persist.
+TEST(DiskCacheBasedQuicServerInfo, MultiplePersist) {
+  MockHttpCache cache;
+  AddMockTransaction(&kHostInfoTransaction1);
+  TestCompletionCallback callback;
+
+  QuicServerId server_id("www.google.com", 443, true, PRIVACY_MODE_DISABLED);
+  scoped_ptr<QuicServerInfo> quic_server_info(
+      new DiskCacheBasedQuicServerInfo(server_id, cache.http_cache()));
+  EXPECT_FALSE(quic_server_info->IsDataReady());
+  quic_server_info->Start();
+  int rv = quic_server_info->WaitForDataReady(callback.callback());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+  EXPECT_TRUE(quic_server_info->IsDataReady());
+
+  // Persist data once.
+  QuicServerInfo::State* state = quic_server_info->mutable_state();
+  EXPECT_TRUE(state->certs.empty());
+  const string server_config_init = "server_config_init";
+  const string source_address_token_init = "source_address_token_init";
+  const string server_config_sig_init = "server_config_sig_init";
+  const string cert_init = "cert_init";
+
+  state->server_config = server_config_init;
+  state->source_address_token = source_address_token_init;
+  state->server_config_sig = server_config_sig_init;
+  state->certs.push_back(cert_init);
+  EXPECT_TRUE(quic_server_info->IsReadyToPersist());
+  quic_server_info->Persist();
+
+  // 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->IsReadyToPersist());
+
+  // Persist one more time using the same |quic_server_info| object and without
+  // doing another Start() and WaitForDataReady.
+  const string server_config_a = "server_config_a";
+  const string source_address_token_a = "source_address_token_a";
+  const string server_config_sig_a = "server_config_sig_a";
+  const string cert_a = "cert_a";
+
+  state->server_config = server_config_a;
+  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->IsReadyToPersist());
+  quic_server_info->Persist();
+
+  // 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->IsReadyToPersist());
+
+  // Verify that the state was updated.
+  quic_server_info.reset(
+      new DiskCacheBasedQuicServerInfo(server_id, cache.http_cache()));
+  quic_server_info->Start();
+  rv = quic_server_info->WaitForDataReady(callback.callback());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+  EXPECT_TRUE(quic_server_info->IsDataReady());
+
+  const QuicServerInfo::State& state1 = quic_server_info->state();
+  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);
+  EXPECT_EQ(1U, state1.certs.size());
+  EXPECT_EQ(cert_a, state1.certs[0]);
+
+  RemoveMockTransaction(&kHostInfoTransaction1);
+}
+
 }  // namespace net