Clean up PnaclTranslationCache interface

chrome/browser/nacl_host/pnacl_translation_cache.cc

Index: chrome/browser/nacl_host/pnacl_translation_cache.cc
diff --git a/chrome/browser/nacl_host/pnacl_translation_cache.cc b/chrome/browser/nacl_host/pnacl_translation_cache.cc
index 6492e36d7411ed807c6039167151f2847ea8329b..a5f8eb6e84b26cc1d41687a3a30d6d9939210330 100644
--- a/chrome/browser/nacl_host/pnacl_translation_cache.cc
+++ b/chrome/browser/nacl_host/pnacl_translation_cache.cc
@@ -8,12 +8,15 @@
 
 #include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_checker.h"
+#include "components/nacl/common/pnacl_types.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/disk_cache/disk_cache.h"
 
+using base::IntToString;
 using content::BrowserThread;
 
 namespace {
@@ -23,7 +26,7 @@ void CloseDiskCacheEntry(disk_cache::Entry* entry) { entry->Close(); }
 }  // namespace
 
 namespace pnacl {
-// These are in pnacl_cache namespace instead of static so they can be used
+// These are in pnacl namespace instead of static so they can be used
 // by the unit test.
 const int kMaxDiskCacheSize = 1000 * 1024 * 1024;
 const int kMaxMemCacheSize = 100 * 1024 * 1024;
@@ -38,12 +41,16 @@ const int kMaxMemCacheSize = 100 * 1024 * 1024;
 class PnaclTranslationCacheEntry
     : public base::RefCounted<PnaclTranslationCacheEntry> {
  public:
-  PnaclTranslationCacheEntry(base::WeakPtr<PnaclTranslationCache> cache,
-                             const std::string& key,
-                             std::string* read_nexe,
-                             const std::string& write_nexe,
-                             const CompletionCallback& callback,
-                             bool is_read);
+  static PnaclTranslationCacheEntry* GetReadEntry(
+      base::WeakPtr<PnaclTranslationCache> cache,
+      const std::string& key,
+      const GetNexeCallback& callback);
+  static PnaclTranslationCacheEntry* GetWriteEntry(
+      base::WeakPtr<PnaclTranslationCache> cache,
+      const std::string& key,
+      net::DrainableIOBuffer* write_nexe,
+      const CompletionCallback& callback);
+
   void Start();
 
   // Writes:                                ---
@@ -66,6 +73,9 @@ class PnaclTranslationCacheEntry
 
  private:
   friend class base::RefCounted<PnaclTranslationCacheEntry>;
+  PnaclTranslationCacheEntry(base::WeakPtr<PnaclTranslationCache> cache,
+                             const std::string& key,
+                             bool is_read);
   ~PnaclTranslationCacheEntry();
 
   // Try to open an existing entry in the backend
@@ -84,47 +94,57 @@ class PnaclTranslationCacheEntry
   // Used as the callback for all operations to the backend. Handle state
   // transitions, track bytes transferred, and call the other helper methods.
   void DispatchNext(int rv);
-  // Get the total transfer size. For reads, must be called after the backend
-  // entry has been opened.
-  int GetTransferSize();
 
   base::WeakPtr<PnaclTranslationCache> cache_;
-
   std::string key_;
-  std::string* read_nexe_;
-  std::string write_nexe_;
   disk_cache::Entry* entry_;
   CacheStep step_;
   bool is_read_;
-  int bytes_transferred_;
-  int bytes_to_transfer_;
-  scoped_refptr<net::IOBufferWithSize> read_buf_;
-  CompletionCallback finish_callback_;
+  GetNexeCallback read_callback_;
+  CompletionCallback write_callback_;
+  scoped_refptr<net::DrainableIOBuffer> io_buf_;
   base::ThreadChecker thread_checker_;
   DISALLOW_COPY_AND_ASSIGN(PnaclTranslationCacheEntry);
 };
 
+// static
+PnaclTranslationCacheEntry* PnaclTranslationCacheEntry::GetReadEntry(
+    base::WeakPtr<PnaclTranslationCache> cache,
+    const std::string& key,
+    const GetNexeCallback& callback) {
+  PnaclTranslationCacheEntry* entry(
+      new PnaclTranslationCacheEntry(cache, key, true));
+  entry->read_callback_ = callback;
+  return entry;
+}
+
+// static
+PnaclTranslationCacheEntry* PnaclTranslationCacheEntry::GetWriteEntry(
+    base::WeakPtr<PnaclTranslationCache> cache,
+    const std::string& key,
+    net::DrainableIOBuffer* write_nexe,
+    const CompletionCallback& callback) {
+  PnaclTranslationCacheEntry* entry(
+      new PnaclTranslationCacheEntry(cache, key, false));
+  entry->io_buf_ = write_nexe;
+  entry->write_callback_ = callback;
+  return entry;
+}
+
 PnaclTranslationCacheEntry::PnaclTranslationCacheEntry(
     base::WeakPtr<PnaclTranslationCache> cache,
     const std::string& key,
-    std::string* read_nexe,
-    const std::string& write_nexe,
-    const CompletionCallback& callback,
     bool is_read)
     : cache_(cache),
       key_(key),
-      read_nexe_(read_nexe),
-      write_nexe_(write_nexe),
       entry_(NULL),
       step_(UNINITIALIZED),
-      is_read_(is_read),
-      bytes_transferred_(0),
-      bytes_to_transfer_(-1),
-      finish_callback_(callback) {}
+      is_read_(is_read) {}
 
 PnaclTranslationCacheEntry::~PnaclTranslationCacheEntry() {
   // Ensure we have called the user's callback
-  DCHECK(finish_callback_.is_null());
+  DCHECK(read_callback_.is_null());
+  DCHECK(write_callback_.is_null());
 }
 
 void PnaclTranslationCacheEntry::Start() {
@@ -154,12 +174,11 @@ void PnaclTranslationCacheEntry::CreateEntry() {
 }
 
 void PnaclTranslationCacheEntry::WriteEntry(int offset, int len) {
-  scoped_refptr<net::StringIOBuffer> io_buf =
-      new net::StringIOBuffer(write_nexe_.substr(offset, len));
+  DCHECK(io_buf_->BytesRemaining() == len);
   int rv = entry_->WriteData(
       1,
       offset,
-      io_buf.get(),
+      io_buf_.get(),
       len,
       base::Bind(&PnaclTranslationCacheEntry::DispatchNext, this),
       false);
@@ -168,25 +187,16 @@ void PnaclTranslationCacheEntry::WriteEntry(int offset, int len) {
 }
 
 void PnaclTranslationCacheEntry::ReadEntry(int offset, int len) {
-  read_buf_ = new net::IOBufferWithSize(len);
   int rv = entry_->ReadData(
       1,
       offset,
-      read_buf_.get(),
+      io_buf_.get(),
       len,
       base::Bind(&PnaclTranslationCacheEntry::DispatchNext, this));
   if (rv != net::ERR_IO_PENDING)
     DispatchNext(rv);
 }
 
-int PnaclTranslationCacheEntry::GetTransferSize() {
-  if (is_read_) {
-    DCHECK(entry_);
-    return entry_->GetDataSize(1);
-  }
-  return write_nexe_.size();
-}
-
 void PnaclTranslationCacheEntry::CloseEntry(int rv) {
   DCHECK(entry_);
   if (rv < 0)
@@ -197,9 +207,16 @@ void PnaclTranslationCacheEntry::CloseEntry(int rv) {
 }
 
 void PnaclTranslationCacheEntry::Finish(int rv) {
-  if (!finish_callback_.is_null()) {
-    finish_callback_.Run(rv);
-    finish_callback_.Reset();
+  if (is_read_) {
+    if (!read_callback_.is_null()) {
+      read_callback_.Run(rv, io_buf_);
+      read_callback_.Reset();
+    }
+  } else {
+    if (!write_callback_.is_null()) {
+      write_callback_.Run(rv);
+      write_callback_.Reset();
+    }
   }
   cache_->OpComplete(this);
 }
@@ -217,26 +234,30 @@ void PnaclTranslationCacheEntry::DispatchNext(int rv) {
     case OPEN_ENTRY:
       if (rv == net::OK) {
         step_ = TRANSFER_ENTRY;
-        bytes_to_transfer_ = GetTransferSize();
-        is_read_ ? ReadEntry(0, bytes_to_transfer_)
-                 : WriteEntry(0, bytes_to_transfer_);
+        if (is_read_) {
+          int bytes_to_transfer = entry_->GetDataSize(1);
+          io_buf_ = new net::DrainableIOBuffer(
+              new net::IOBuffer(bytes_to_transfer), bytes_to_transfer);
+          ReadEntry(0, bytes_to_transfer);
+        } else {
+          WriteEntry(0, io_buf_->size());
+        }
       } else {
         if (is_read_) {
           // Just a cache miss, not necessarily an error.
           entry_ = NULL;
           Finish(rv);
-          break;
+        } else {
+          step_ = CREATE_ENTRY;
+          CreateEntry();
         }
-        step_ = CREATE_ENTRY;
-        CreateEntry();
       }
       break;
 
     case CREATE_ENTRY:
       if (rv == net::OK) {
         step_ = TRANSFER_ENTRY;
-        bytes_to_transfer_ = GetTransferSize();
-        WriteEntry(bytes_transferred_, bytes_to_transfer_ - bytes_transferred_);
+        WriteEntry(io_buf_->BytesConsumed(), io_buf_->BytesRemaining());
       } else {
         LOG(ERROR) << "Failed to Create a PNaCl Translation Cache Entry";
         Finish(rv);
@@ -254,19 +275,19 @@ void PnaclTranslationCacheEntry::DispatchNext(int rv) {
         CloseEntry(rv);
         break;
       } else if (rv > 0) {
-        // For reads, copy the data that was just returned
-        if (is_read_)
-          read_nexe_->append(read_buf_->data(), rv);
-        bytes_transferred_ += rv;
-        if (bytes_transferred_ < bytes_to_transfer_) {
-          int len = bytes_to_transfer_ - bytes_transferred_;
-          is_read_ ? ReadEntry(bytes_transferred_, len)
-                   : WriteEntry(bytes_transferred_, len);
+        io_buf_->DidConsume(rv);
+        if (io_buf_->BytesRemaining() > 0) {
+          is_read_
+              ? ReadEntry(io_buf_->BytesConsumed(), io_buf_->BytesRemaining())
+              : WriteEntry(io_buf_->BytesConsumed(), io_buf_->BytesRemaining());
           break;
         }
       }
       // rv == 0 or we fell through (i.e. we have transferred all the bytes)
       step_ = CLOSE_ENTRY;
+      DCHECK(io_buf_->BytesConsumed() == io_buf_->size());
+      if (is_read_)
+        io_buf_->SetOffset(0);
       CloseEntry(0);
       break;
 
@@ -334,24 +355,23 @@ void PnaclTranslationCache::OnCreateBackendComplete(int rv) {
 // High-level API
 
 void PnaclTranslationCache::StoreNexe(const std::string& key,
-                                      const std::string& nexe) {
-  StoreNexe(key, nexe, CompletionCallback());
+                                      net::DrainableIOBuffer* nexe_data) {
+  StoreNexe(key, nexe_data, CompletionCallback());
 }
 
 void PnaclTranslationCache::StoreNexe(const std::string& key,
-                                      const std::string& nexe,
+                                      net::DrainableIOBuffer* nexe_data,
                                       const CompletionCallback& callback) {
-  PnaclTranslationCacheEntry* entry = new PnaclTranslationCacheEntry(
-      AsWeakPtr(), key, NULL, nexe, callback, false);
+  PnaclTranslationCacheEntry* entry = PnaclTranslationCacheEntry::GetWriteEntry(
+      AsWeakPtr(), key, nexe_data, callback);
   open_entries_[entry] = entry;
   entry->Start();
 }
 
 void PnaclTranslationCache::GetNexe(const std::string& key,
-                                    std::string* nexe,
-                                    const CompletionCallback& callback) {
-  PnaclTranslationCacheEntry* entry = new PnaclTranslationCacheEntry(
-      AsWeakPtr(), key, nexe, std::string(), callback, true);
+                                    const GetNexeCallback& callback) {
+  PnaclTranslationCacheEntry* entry =
+      PnaclTranslationCacheEntry::GetReadEntry(AsWeakPtr(), key, callback);
   open_entries_[entry] = entry;
   entry->Start();
 }
@@ -364,9 +384,7 @@ int PnaclTranslationCache::InitCache(const base::FilePath& cache_directory,
   if (in_memory_) {
     rv = InitWithMemBackend(kMaxMemCacheSize, callback);
   } else {
-    rv = InitWithDiskBackend(cache_directory,
-                             kMaxDiskCacheSize,
-                             callback);
+    rv = InitWithDiskBackend(cache_directory, kMaxDiskCacheSize, callback);
   }
 
   return rv;
@@ -378,4 +396,36 @@ int PnaclTranslationCache::Size() {
   return disk_cache_->GetEntryCount();
 }
 
+// static
+std::string PnaclTranslationCache::GetKey(const nacl::PnaclCacheInfo& info) {
+  if (!info.pexe_url.is_valid() || info.abi_version < 0 || info.opt_level < 0)
+    return std::string();
+  std::string retval("ABI:");
+  retval += IntToString(info.abi_version) + ";" +
+      "opt:" + IntToString(info.opt_level) + ";" +
+      "URL:";
+  // Filter the username, password, and ref components from the URL
+  GURL::Replacements replacements;
+  replacements.ClearUsername();
+  replacements.ClearPassword();
+  replacements.ClearRef();
+  GURL key_url(info.pexe_url.ReplaceComponents(replacements));
+  retval += key_url.spec() + ";";
+  // You would think that there is already code to format base::Time values
+  // somewhere, but I haven't found it yet. In any case, doing it ourselves
+  // here means we can keep the format stable.
+  base::Time::Exploded exploded;
+  info.last_modified.UTCExplode(&exploded);
+  if (info.last_modified.is_null() || !exploded.HasValidValues()) {
+    memset(&exploded, 0, sizeof(exploded));
+  }
+  retval += "modified:" + IntToString(exploded.year) + ":" +
+      IntToString(exploded.month) + ":" +
+      IntToString(exploded.day_of_month) + ":" +
+      IntToString(exploded.hour) + ":" + IntToString(exploded.minute) + ":" +
+      IntToString(exploded.second) + ":" +
+      IntToString(exploded.millisecond) + ":UTC;";
+  retval += "etag:" + info.etag;
+  return retval;
+}
 }  // namespace pnacl