Persist server's crypto config data to disk cache for 0-RTT

net/http/disk_cache_based_quic_server_info.cc

 // Copyright 2014 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.
 
 #include "net/http/disk_cache_based_quic_server_info.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "net/base/completion_callback.h"
@@ skipping 58 matching lines @@
 
 int DiskCacheBasedQuicServerInfo::WaitForDataReady(
     const CompletionCallback& callback) {
   DCHECK(CalledOnValidThread());
   DCHECK(state_ != GET_BACKEND);
 
   if (ready_)
     return OK;
 
   if (!callback.is_null()) {
-    DCHECK(user_callback_.is_null());
+    if (!user_callback_.is_null())
+      return ERR_INVALID_ARGUMENT;

[wtc, 2014/02/07 00:54:11]

Nit: we may want to use a different error code.

Can you explain why you want to allow user_callback_ to be null?

[ramant (doing other things), 2014/02/07 20:30:51]

If user_callback_ not null, then there is a pending callback. Overwriting a
previous user_callback_ with a new callback doesn't seem to be good.

Thought, if we give an error, then caller knows WaitForDataReady has failed and
can handle the error condition, instead of overwriting previous caller's
callback.

Would appreciate if you could recommend a better error code.

     user_callback_ = callback;
   }
 
   return ERR_IO_PENDING;
 }
 
 void DiskCacheBasedQuicServerInfo::Persist() {
   DCHECK(CalledOnValidThread());
-  DCHECK(state_ != GET_BACKEND);
+  DCHECK_NE(state_, GET_BACKEND);
 
   DCHECK(new_data_.empty());
   CHECK(ready_);
   DCHECK(user_callback_.is_null());
   new_data_ = Serialize();
 
   if (!backend_)
     return;
 
   state_ = CREATE_OR_OPEN;
   DoLoop(OK);
 }
 
 DiskCacheBasedQuicServerInfo::~DiskCacheBasedQuicServerInfo() {
   DCHECK(user_callback_.is_null());
   if (entry_)
     entry_->Close();
 }
 
 std::string DiskCacheBasedQuicServerInfo::key() const {
   return "quicserverinfo:" + hostname_;
 }
 
 void DiskCacheBasedQuicServerInfo::OnIOComplete(CacheOperationDataShim* unused,
                                                 int rv) {
+  DCHECK_NE(state_, NONE);
   rv = DoLoop(rv);
   if (rv != ERR_IO_PENDING && !user_callback_.is_null()) {
     CompletionCallback callback = user_callback_;
     user_callback_.Reset();
     callback.Run(rv);
   }
 }
 
 int DiskCacheBasedQuicServerInfo::DoLoop(int rv) {
   do {
@@ skipping 68 matching lines @@
 int DiskCacheBasedQuicServerInfo::DoReadComplete(int rv) {
   if (rv > 0)
     data_.assign(read_buffer_->data(), rv);
 
   state_ = WAIT_FOR_DATA_READY_DONE;
   return OK;
 }
 
 int DiskCacheBasedQuicServerInfo::DoWriteComplete(int rv) {
   state_ = SET_DONE;
+  new_data_.clear();

[wtc, 2014/02/07 00:54:11]

Another good place to do new_data_.clear() is in DoSetDone, because we can also
go to the SET_DONE state from DoCreateOrOpenComplete.

[ramant (doing other things), 2014/02/07 20:30:51]

Done.

   return OK;
 }
 
 int DiskCacheBasedQuicServerInfo::DoCreateOrOpenComplete(int rv) {
   if (rv != OK) {
     state_ = SET_DONE;
   } else {
     entry_ = data_shim_->entry;
     state_ = WRITE;
   }
@@ skipping 58 matching lines @@
 
 int DiskCacheBasedQuicServerInfo::DoSetDone() {
   if (entry_)
     entry_->Close();
   entry_ = NULL;
   state_ = NONE;
   return OK;
 }
 
 }  // namespace net