Revert 204354 "Add read support to PNaClTranslationCache"

trunk/src/chrome/browser/nacl_host/pnacl_translation_cache.cc

Index: trunk/src/chrome/browser/nacl_host/pnacl_translation_cache.cc
===================================================================
--- trunk/src/chrome/browser/nacl_host/pnacl_translation_cache.cc	(revision 204381)
+++ trunk/src/chrome/browser/nacl_host/pnacl_translation_cache.cc	(working copy)
@@ -33,182 +33,130 @@
 const int kMaxMemCacheSize = 100 * 1024 * 1024;
 
 //////////////////////////////////////////////////////////////////////
-// Handle Reading/Writing to Cache.
+// Handle Storing to Cache.
 
-// PNaClTranslationCacheEntry is a shim that provides storage for the
+// PNaClTranslationCacheWriteEntry 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 PNaClTranslationCacheEntry
-    : public base::RefCounted<PNaClTranslationCacheEntry> {
+class PNaClTranslationCacheWriteEntry
+    : public base::RefCounted<PNaClTranslationCacheWriteEntry> {
  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);
-  void Start();
+  PNaClTranslationCacheWriteEntry(base::WeakPtr<PNaClTranslationCache> cache,
+                                  const std::string& key,
+                                  const std::string& nexe,
+                                  const net::CompletionCallback& callback);
 
-  // Writes:                                ---
+  void Cache();
+
+  //                                        ---
   //                                        v  |
-  // Start -> Open Existing --------------> Write ---> Close
+  // Cache -> Open Existing --------------> Write ---> Close
   //                          \              ^
   //                           \             /
   //                            --> Create --
-  // Reads:
-  // Start -> Open --------Read ----> Close
-  //                       |  ^
-  //                       |__|
   enum CacheStep {
     UNINITIALIZED,
     OPEN_ENTRY,
     CREATE_ENTRY,
-    TRANSFER_ENTRY,
+    WRITE_ENTRY,
     CLOSE_ENTRY
   };
 
  private:
-  friend class base::RefCounted<PNaClTranslationCacheEntry>;
-  ~PNaClTranslationCacheEntry();
+  friend class base::RefCounted<PNaClTranslationCacheWriteEntry>;
+  ~PNaClTranslationCacheWriteEntry();
 
-  // Try to open an existing entry in the backend
-  void OpenEntry();
-  // 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 OpenEntry();
+
+  void WriteEntry(int bytes_to_skip);
+
   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* read_nexe_;
-  std::string write_nexe_;
+  std::string 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(PNaClTranslationCacheEntry);
+  DISALLOW_COPY_AND_ASSIGN(PNaClTranslationCacheWriteEntry);
 };
 
-PNaClTranslationCacheEntry::PNaClTranslationCacheEntry(
+PNaClTranslationCacheWriteEntry::PNaClTranslationCacheWriteEntry(
     base::WeakPtr<PNaClTranslationCache> cache,
     const std::string& key,
-    std::string* read_nexe,
-    const std::string& write_nexe,
-    const CompletionCallback& callback,
-    bool is_read)
+    const std::string& nexe,
+    const net::CompletionCallback& callback)
     : cache_(cache),
       key_(key),
-      read_nexe_(read_nexe),
-      write_nexe_(write_nexe),
+      nexe_(nexe),
       entry_(NULL),
       step_(UNINITIALIZED),
-      is_read_(is_read),
-      bytes_transferred_(0),
-      bytes_to_transfer_(-1),
       finish_callback_(callback) {}
 
-PNaClTranslationCacheEntry::~PNaClTranslationCacheEntry() {
-  // Ensure we have called the user's callback
-  DCHECK(finish_callback_.is_null());
+PNaClTranslationCacheWriteEntry::~PNaClTranslationCacheWriteEntry() {
+  if (entry_)
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE, base::Bind(&CloseDiskCacheEntry, entry_));
 }
 
-void PNaClTranslationCacheEntry::Start() {
+void PNaClTranslationCacheWriteEntry::Cache() {
   DCHECK(thread_checker_.CalledOnValidThread());
   step_ = OPEN_ENTRY;
   OpenEntry();
 }
 
-// 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));
+// 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));
   if (rv != net::ERR_IO_PENDING)
     DispatchNext(rv);
 }
 
-void PNaClTranslationCacheEntry::CreateEntry() {
+void PNaClTranslationCacheWriteEntry::CreateEntry() {
   int rv = cache_->backend()->CreateEntry(
       key_,
       &entry_,
-      base::Bind(&PNaClTranslationCacheEntry::DispatchNext, this));
+      base::Bind(&PNaClTranslationCacheWriteEntry::DispatchNext, this));
   if (rv != net::ERR_IO_PENDING)
     DispatchNext(rv);
 }
 
-void PNaClTranslationCacheEntry::WriteEntry(int offset, int len) {
-  scoped_refptr<net::StringIOBuffer> io_buf =
-      new net::StringIOBuffer(write_nexe_.substr(offset, len));
+void PNaClTranslationCacheWriteEntry::WriteEntry(int bytes_to_skip) {
+  nexe_ = nexe_.substr(bytes_to_skip);
+  scoped_refptr<net::StringIOBuffer> io_buf = new net::StringIOBuffer(nexe_);
   int rv = entry_->WriteData(
       1,
-      offset,
+      0,
       io_buf,
-      len,
-      base::Bind(&PNaClTranslationCacheEntry::DispatchNext, this),
+      nexe_.length(),
+      base::Bind(&PNaClTranslationCacheWriteEntry::DispatchNext, this),
       false);
   if (rv != net::ERR_IO_PENDING)
     DispatchNext(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_);
+void PNaClTranslationCacheWriteEntry::CloseEntry(int rv) {
   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_->OpComplete(this);
+  cache_->WriteComplete(this);
 }
 
-void PNaClTranslationCacheEntry::DispatchNext(int rv) {
+void PNaClTranslationCacheWriteEntry::DispatchNext(int rv) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!cache_.get())
     return;
@@ -220,17 +168,9 @@
 
     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_);
+        step_ = WRITE_ENTRY;
+        WriteEntry(0);
       } else {
-        if (is_read_) {
-          // Just a cache miss, not necessarily an error.
-          entry_ = NULL;
-          Finish(rv);
-          break;
-        }
         step_ = CREATE_ENTRY;
         CreateEntry();
       }
@@ -238,18 +178,17 @@
 
     case CREATE_ENTRY:
       if (rv == net::OK) {
-        step_ = TRANSFER_ENTRY;
-        bytes_to_transfer_ = GetTransferSize();
-        WriteEntry(bytes_transferred_, bytes_to_transfer_ - bytes_transferred_);
+        step_ = WRITE_ENTRY;
+        WriteEntry(0);
       } else {
-        LOG(ERROR) << "Failed to Create a PNaCl Translation Cache Entry";
-        Finish(rv);
+        LOG(ERROR) << "Failed to Open/Create a PNaCl Translation Cache Entry";
+        CloseEntry(rv);
       }
       break;
 
-    case TRANSFER_ENTRY:
+    case WRITE_ENTRY:
       if (rv < 0) {
-        // We do not call DispatchNext directly if WriteEntry/ReadEntry returns
+        // We do not call DispatchNext directly if WriteEntry returns
         // ERR_IO_PENDING, and the callback should not return that value either.
         LOG(ERROR)
             << "Failed to complete write to PNaCl Translation Cache Entry: "
@@ -257,21 +196,15 @@
         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;
-        }
       }
-      // rv == 0 or we fell through (i.e. we have transferred all the bytes)
-      step_ = CLOSE_ENTRY;
-      CloseEntry(0);
+      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);
       break;
 
     case CLOSE_ENTRY:
@@ -281,8 +214,9 @@
 }
 
 //////////////////////////////////////////////////////////////////////
-void PNaClTranslationCache::OpComplete(PNaClTranslationCacheEntry* entry) {
-  open_entries_.erase(entry);
+void PNaClTranslationCache::WriteComplete(
+    PNaClTranslationCacheWriteEntry* entry) {
+  write_entries_.erase(entry);
 }
 
 //////////////////////////////////////////////////////////////////////
@@ -290,25 +224,27 @@
 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 CompletionCallback& callback) {
+    const net::CompletionCallback& callback) {
   return Init(net::DISK_CACHE, cache_dir, cache_size, callback);
 }
 
 int PNaClTranslationCache::InitWithMemBackend(
     int cache_size,
-    const CompletionCallback& callback) {
+    const net::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 CompletionCallback& callback) {
+                                const net::CompletionCallback& callback) {
   int rv = disk_cache::CreateCacheBackend(
       cache_type,
       net::CACHE_BACKEND_DEFAULT,
@@ -337,32 +273,35 @@
 //////////////////////////////////////////////////////////////////////
 // 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, CompletionCallback());
+  StoreNexe(key, nexe, base::Bind(NullCallback));
 }
 
 void PNaClTranslationCache::StoreNexe(const std::string& key,
                                       const std::string& nexe,
-                                      const CompletionCallback& callback) {
-  PNaClTranslationCacheEntry* entry = new PNaClTranslationCacheEntry(
-      AsWeakPtr(), key, NULL, nexe, callback, false);
-  open_entries_[entry] = entry;
-  entry->Start();
+                                      const net::CompletionCallback& callback) {
+  PNaClTranslationCacheWriteEntry* entry =
+      new PNaClTranslationCacheWriteEntry(AsWeakPtr(), key, nexe, callback);
+  write_entries_[entry] = entry;
+  entry->Cache();
 }
 
-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::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;
 }
 
 int PNaClTranslationCache::InitCache(const base::FilePath& cache_directory,
                                      bool in_memory,
-                                     const CompletionCallback& callback) {
+                                     const net::CompletionCallback& callback) {
   int rv;
   in_memory_ = in_memory;
   if (in_memory_) {