Add read support to PNaClTranslationCache

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 8316ba7052137881205682015efe67f1ba7f657e..e725e0b6f226bec081a8533d2f4f4916248ae63c 100644
--- a/chrome/browser/nacl_host/pnacl_translation_cache.cc
+++ b/chrome/browser/nacl_host/pnacl_translation_cache.cc
@@ -33,130 +33,182 @@ const int kMaxDiskCacheSize = 1000 * 1024 * 1024;
 const int kMaxMemCacheSize = 100 * 1024 * 1024;
 
 //////////////////////////////////////////////////////////////////////
-// Handle Storing to Cache.
+// Handle Reading/Writing to Cache.
 
-// PNaClTranslationCacheWriteEntry is a shim that provides storage for the
+// PNaClTranslationCacheEntry is a shim that provides storage for the
 // 'key' and 'data' strings as the disk_cache is performing various async
 // operations. It also tracks the open disk_cache::Entry
 // and ensures that the entry is closed.
-class PNaClTranslationCacheWriteEntry
-    : public base::RefCounted<PNaClTranslationCacheWriteEntry> {
+class PNaClTranslationCacheEntry
+    : public base::RefCounted<PNaClTranslationCacheEntry> {
  public:
-  PNaClTranslationCacheWriteEntry(base::WeakPtr<PNaClTranslationCache> cache,
-                                  const std::string& key,
-                                  const std::string& nexe,
-                                  const net::CompletionCallback& callback);
-
-  void Cache();
-
-  //                                        ---
+  PNaClTranslationCacheEntry(base::WeakPtr<PNaClTranslationCache> cache,
+                             const std::string& key,
+                             std::string* read_nexe,
+                             const std::string& write_nexe,
+                             const CompletionCallback& callback,
+                             bool is_read);
+  void Start();
+
+  // Writes:                                ---
   //                                        v  |
-  // Cache -> Open Existing --------------> Write ---> Close
+  // Start -> Open Existing --------------> Write ---> Close
   //                          \              ^
   //                           \             /
   //                            --> Create --
+  // Reads:
+  // Start -> Open --------Read ----> Close
+  //                       |  ^
+  //                       |__|
   enum CacheStep {
     UNINITIALIZED,
     OPEN_ENTRY,
     CREATE_ENTRY,
-    WRITE_ENTRY,
+    TRANSFER_ENTRY,
     CLOSE_ENTRY
   };
 
  private:
-  friend class base::RefCounted<PNaClTranslationCacheWriteEntry>;
-  ~PNaClTranslationCacheWriteEntry();
-
-  void CreateEntry();
+  friend class base::RefCounted<PNaClTranslationCacheEntry>;
+  ~PNaClTranslationCacheEntry();
 
+  // Try to open an existing entry in the backend
   void OpenEntry();
-
-  void WriteEntry(int bytes_to_skip);
-
+  // Create a new entry in the backend (for writes)
+  void CreateEntry();
+  // Write |len| bytes to the backend, starting at |offset|
+  void WriteEntry(int offset, int len);
+  // Read |len| bytes from the backend, starting at |offset|
+  void ReadEntry(int offset, int len);
+  // If there was an error, doom the entry. Then post a task to the IO
+  // thread to close (and delete) it.
   void CloseEntry(int rv);
-
+  // Call the user callback, and signal to the cache to delete this.
+  void Finish(int rv);
+  // 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 nexe_;
+  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_;
   base::ThreadChecker thread_checker_;
-  DISALLOW_COPY_AND_ASSIGN(PNaClTranslationCacheWriteEntry);
+  DISALLOW_COPY_AND_ASSIGN(PNaClTranslationCacheEntry);
 };
 
-PNaClTranslationCacheWriteEntry::PNaClTranslationCacheWriteEntry(
+PNaClTranslationCacheEntry::PNaClTranslationCacheEntry(
     base::WeakPtr<PNaClTranslationCache> cache,
     const std::string& key,
-    const std::string& nexe,
-    const net::CompletionCallback& callback)
+    std::string* read_nexe,
+    const std::string& write_nexe,
+    const CompletionCallback& callback,
+    bool is_read)
     : cache_(cache),
       key_(key),
-      nexe_(nexe),
+      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) {}
 
-PNaClTranslationCacheWriteEntry::~PNaClTranslationCacheWriteEntry() {
-  if (entry_)
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE, base::Bind(&CloseDiskCacheEntry, entry_));
+PNaClTranslationCacheEntry::~PNaClTranslationCacheEntry() {
+  // Ensure we have called the user's callback
+  DCHECK(finish_callback_.is_null());
 }
 
-void PNaClTranslationCacheWriteEntry::Cache() {
+void PNaClTranslationCacheEntry::Start() {
   DCHECK(thread_checker_.CalledOnValidThread());
   step_ = OPEN_ENTRY;
   OpenEntry();
 }
 
-// OpenEntry, CreateEntry, WriteEntry, and CloseEntry are only called from
-// DispatchNext, so they know that cache_ is still valid.
-void PNaClTranslationCacheWriteEntry::OpenEntry() {
-  int rv = cache_->backend()->OpenEntry(
-      key_,
-      &entry_,
-      base::Bind(&PNaClTranslationCacheWriteEntry::DispatchNext, this));
+// OpenEntry, CreateEntry, WriteEntry, ReadEntry and CloseEntry are only called
+// from DispatchNext, so they know that cache_ is still valid.
+void PNaClTranslationCacheEntry::OpenEntry() {
+  int rv = cache_->backend()
+      ->OpenEntry(key_,
+                  &entry_,
+                  base::Bind(&PNaClTranslationCacheEntry::DispatchNext, this));
   if (rv != net::ERR_IO_PENDING)
     DispatchNext(rv);
 }
 
-void PNaClTranslationCacheWriteEntry::CreateEntry() {
+void PNaClTranslationCacheEntry::CreateEntry() {
   int rv = cache_->backend()->CreateEntry(
       key_,
       &entry_,
-      base::Bind(&PNaClTranslationCacheWriteEntry::DispatchNext, this));
+      base::Bind(&PNaClTranslationCacheEntry::DispatchNext, this));
   if (rv != net::ERR_IO_PENDING)
     DispatchNext(rv);
 }
 
-void PNaClTranslationCacheWriteEntry::WriteEntry(int bytes_to_skip) {
-  nexe_ = nexe_.substr(bytes_to_skip);
-  scoped_refptr<net::StringIOBuffer> io_buf = new net::StringIOBuffer(nexe_);
+void PNaClTranslationCacheEntry::WriteEntry(int offset, int len) {
+  scoped_refptr<net::StringIOBuffer> io_buf =
+      new net::StringIOBuffer(write_nexe_.substr(offset, len));
   int rv = entry_->WriteData(
       1,
-      0,
+      offset,
       io_buf,
-      nexe_.length(),
-      base::Bind(&PNaClTranslationCacheWriteEntry::DispatchNext, this),
+      len,
+      base::Bind(&PNaClTranslationCacheEntry::DispatchNext, this),
       false);
   if (rv != net::ERR_IO_PENDING)
     DispatchNext(rv);
 }
 
-void PNaClTranslationCacheWriteEntry::CloseEntry(int rv) {
+void PNaClTranslationCacheEntry::ReadEntry(int offset, int len) {
+  read_buf_ = new net::IOBufferWithSize(len);
+  int rv = entry_->ReadData(
+      1,
+      offset,
+      read_buf_,
+      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)
     entry_->Doom();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE, base::Bind(&CloseDiskCacheEntry, entry_));
+  Finish(rv);
+}
+
+void PNaClTranslationCacheEntry::Finish(int rv) {
   if (!finish_callback_.is_null()) {
     finish_callback_.Run(rv);
     finish_callback_.Reset();
   }
-  cache_->WriteComplete(this);
+  cache_->OpComplete(this);
 }
 
-void PNaClTranslationCacheWriteEntry::DispatchNext(int rv) {
+void PNaClTranslationCacheEntry::DispatchNext(int rv) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!cache_)
     return;
@@ -168,9 +220,17 @@ void PNaClTranslationCacheWriteEntry::DispatchNext(int rv) {
 
     case OPEN_ENTRY:
       if (rv == net::OK) {
-        step_ = WRITE_ENTRY;
-        WriteEntry(0);
+        step_ = TRANSFER_ENTRY;
+        bytes_to_transfer_ = GetTransferSize();
+        is_read_ ? ReadEntry(0, bytes_to_transfer_)
+                 : WriteEntry(0, bytes_to_transfer_);
       } else {
+        if (is_read_) {
+          // Just a cache miss, not necessarily an error.
+          entry_ = NULL;
+          Finish(rv);
+          break;
+        }
         step_ = CREATE_ENTRY;
         CreateEntry();
       }
@@ -178,17 +238,18 @@ void PNaClTranslationCacheWriteEntry::DispatchNext(int rv) {
 
     case CREATE_ENTRY:
       if (rv == net::OK) {
-        step_ = WRITE_ENTRY;
-        WriteEntry(0);
+        step_ = TRANSFER_ENTRY;
+        bytes_to_transfer_ = GetTransferSize();
+        WriteEntry(bytes_transferred_, bytes_to_transfer_ - bytes_transferred_);
       } else {
-        LOG(ERROR) << "Failed to Open/Create a PNaCl Translation Cache Entry";
-        CloseEntry(rv);
+        LOG(ERROR) << "Failed to Create a PNaCl Translation Cache Entry";
+        Finish(rv);
       }
       break;
 
-    case WRITE_ENTRY:
+    case TRANSFER_ENTRY:
       if (rv < 0) {
-        // We do not call DispatchNext directly if WriteEntry returns
+        // We do not call DispatchNext directly if WriteEntry/ReadEntry returns
         // ERR_IO_PENDING, and the callback should not return that value either.
         LOG(ERROR)
             << "Failed to complete write to PNaCl Translation Cache Entry: "
@@ -196,15 +257,21 @@ void PNaClTranslationCacheWriteEntry::DispatchNext(int rv) {
         step_ = CLOSE_ENTRY;
         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);
+          break;
+        }
       }
-      if (rv == 0) {
-        step_ = CLOSE_ENTRY;
-        CloseEntry(rv);
-        break;
-      }
-      // rv bytes were written; call WriteEntry again to skip them and try to
-      // write the rest.
-      WriteEntry(rv);
+      // rv == 0 or we fell through (i.e. we have transferred all the bytes)
+      step_ = CLOSE_ENTRY;
+      CloseEntry(0);
       break;
 
     case CLOSE_ENTRY:
@@ -214,9 +281,8 @@ void PNaClTranslationCacheWriteEntry::DispatchNext(int rv) {
 }
 
 //////////////////////////////////////////////////////////////////////
-void PNaClTranslationCache::WriteComplete(
-    PNaClTranslationCacheWriteEntry* entry) {
-  write_entries_.erase(entry);
+void PNaClTranslationCache::OpComplete(PNaClTranslationCacheEntry* entry) {
+  open_entries_.erase(entry);
 }
 
 //////////////////////////////////////////////////////////////////////
@@ -224,27 +290,25 @@ void PNaClTranslationCache::WriteComplete(
 PNaClTranslationCache::PNaClTranslationCache()
     : disk_cache_(NULL), in_memory_(false) {}
 
-PNaClTranslationCache::~PNaClTranslationCache() {
-  delete disk_cache_;
-}
+PNaClTranslationCache::~PNaClTranslationCache() { delete disk_cache_; }
 
 int PNaClTranslationCache::InitWithDiskBackend(
     const base::FilePath& cache_dir,
     int cache_size,
-    const net::CompletionCallback& callback) {
+    const CompletionCallback& callback) {
   return Init(net::DISK_CACHE, cache_dir, cache_size, callback);
 }
 
 int PNaClTranslationCache::InitWithMemBackend(
     int cache_size,
-    const net::CompletionCallback& callback) {
+    const CompletionCallback& callback) {
   return Init(net::MEMORY_CACHE, base::FilePath(), cache_size, callback);
 }
 
 int PNaClTranslationCache::Init(net::CacheType cache_type,
                                 const base::FilePath& cache_dir,
                                 int cache_size,
-                                const net::CompletionCallback& callback) {
+                                const CompletionCallback& callback) {
   int rv = disk_cache::CreateCacheBackend(
       cache_type,
       net::CACHE_BACKEND_DEFAULT,
@@ -273,35 +337,32 @@ void PNaClTranslationCache::OnCreateBackendComplete(int rv) {
 //////////////////////////////////////////////////////////////////////
 // High-level API
 
-// TODO(dschuff): Surely there must be a way to just create a null callback?
-static void NullCallback(int ignored) {}
-
 void PNaClTranslationCache::StoreNexe(const std::string& key,
                                       const std::string& nexe) {
-  StoreNexe(key, nexe, base::Bind(NullCallback));
+  StoreNexe(key, nexe, CompletionCallback());
 }
 
 void PNaClTranslationCache::StoreNexe(const std::string& key,
                                       const std::string& nexe,
-                                      const net::CompletionCallback& callback) {
-  PNaClTranslationCacheWriteEntry* entry =
-      new PNaClTranslationCacheWriteEntry(AsWeakPtr(), key, nexe, callback);
-  write_entries_[entry] = entry;
-  entry->Cache();
+                                      const CompletionCallback& callback) {
+  PNaClTranslationCacheEntry* entry = new PNaClTranslationCacheEntry(
+      AsWeakPtr(), key, NULL, nexe, callback, false);
+  open_entries_[entry] = entry;
+  entry->Start();
 }
 
-int PNaClTranslationCache::GetNexe(const std::string& key,
-                                   std::string* nexe,
-                                   const net::CompletionCallback& callback) {
-  // TODO(dschuff): Actually find the entry, and do the right thing.
-  // Shader cache ended up making a separate ReadHelper, analogous
-  // to the PNaClTranslationCacheWriteEntry.
-  return net::OK;
+void PNaClTranslationCache::GetNexe(const std::string& key,
+                                    std::string* nexe,
+                                    const CompletionCallback& callback) {
+  PNaClTranslationCacheEntry* entry = new PNaClTranslationCacheEntry(
+      AsWeakPtr(), key, nexe, std::string(), callback, true);
+  open_entries_[entry] = entry;
+  entry->Start();
 }
 
 int PNaClTranslationCache::InitCache(const base::FilePath& cache_directory,
                                      bool in_memory,
-                                     const net::CompletionCallback& callback) {
+                                     const CompletionCallback& callback) {
   int rv;
   in_memory_ = in_memory;
   if (in_memory_) {
@@ -321,4 +382,4 @@ int PNaClTranslationCache::Size() {
   return disk_cache_->GetEntryCount();
 }
 
-}  // namespace nacl_cache
+}  // namespace pnacl_cache