Add PNaCl translation cache based on Chrome disk_cache

Add PNaCl translation cache based on Chrome disk_cache
The current translation cache for Portable Native Client executables is visible
to the user and implemented by the plugin, which is eventually going away.
The new implementation will re-use existing cache code and be controlled by the
browser rather than the plugin.
This first CL implements a simple interface for the nacl host which hides the
various stages of the disk_cache implementation. It only does stores; loads
will be a future CL.

R=jvoung@chromium.org
BUG= https://code.google.com/p/nativeclient/issues/detail?id=3372
TEST= unit_tests --gtest_filter=PNaClTranslationCacheTest.*

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203591

[jvoung (off chromium), 2013-05-31 01:12:14]

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
File chrome/browser/nacl_host/pnacl_translation_cache.cc (right):

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:11: #include "base/time.h"
no need for base/time.h if you remove the start_time_ stuff below

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:82: base::Time start_time_;
You could remove start_time_, since this was just for timing the write in the
old CL?

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:118: step_ = OPEN_ENTRY;
could move the step_ setting outside of this and into Cache(), where it's
called, to be consistent with WriteEntry and CloseEntry, which don't set the
step_ to the current step

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:131: step_ = CREATE_ENTRY;
step is already set by the DispatchNext.

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:205: CloseEntry(rv);
could set step_ to CLOSE_ENTRY before these calls, to be consistent

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
File chrome/browser/nacl_host/pnacl_translation_cache_unittest.cc (right):

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache_unittest.cc:75: //EXPECT_EQ(0,
cache_->Size());
It only hits an error after it's created the entry and then tried to write,
right?

When there's an error on the write, it currently doesn't try to DoomEntry.  It
just closes the entry.

[Derek Schuff, 2013-05-31 23:15:31]

also removed the stuff from net/ for now so we don't need any extra OWNERS

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
File chrome/browser/nacl_host/pnacl_translation_cache.cc (right):

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:11: #include "base/time.h"
On 2013/05/31 01:12:14, jvoung (cr) wrote:
> no need for base/time.h if you remove the start_time_ stuff below

Done.

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:82: base::Time start_time_;
On 2013/05/31 01:12:14, jvoung (cr) wrote:
> You could remove start_time_, since this was just for timing the write in the
> old CL?

Done.

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:118: step_ = OPEN_ENTRY;
On 2013/05/31 01:12:14, jvoung (cr) wrote:
> could move the step_ setting outside of this and into Cache(), where it's
> called, to be consistent with WriteEntry and CloseEntry, which don't set the
> step_ to the current step

Done.

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:131: step_ = CREATE_ENTRY;
On 2013/05/31 01:12:14, jvoung (cr) wrote:
> step is already set by the DispatchNext.

Done.

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache.cc:205: CloseEntry(rv);
On 2013/05/31 01:12:14, jvoung (cr) wrote:
> could set step_ to CLOSE_ENTRY before these calls, to be consistent

Done.

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
File chrome/browser/nacl_host/pnacl_translation_cache_unittest.cc (right):

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache_unittest.cc:75: //EXPECT_EQ(0,
cache_->Size());
On 2013/05/31 01:12:14, jvoung (cr) wrote:
> It only hits an error after it's created the entry and then tried to write,
> right?
> 
> When there's an error on the write, it currently doesn't try to DoomEntry.  It
> just closes the entry.

OK. Now we do doom the entry in case of an error, but still leave the closing to
the destructor. The destructor still posts a task to the IO thread, since the
translation cache itself might be (?) allocated and freed on some other thread.
Also have a ThreadChecker to sanity check all that.

[jvoung (off chromium), 2013-06-01 00:48:41]

LGTM

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
File chrome/browser/nacl_host/pnacl_translation_cache_unittest.cc (right):

https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
chrome/browser/nacl_host/pnacl_translation_cache_unittest.cc:75: //EXPECT_EQ(0,
cache_->Size());
On 2013/05/31 23:15:31, Derek Schuff wrote:
> On 2013/05/31 01:12:14, jvoung (cr) wrote:
> > It only hits an error after it's created the entry and then tried to write,
> > right?
> > 
> > When there's an error on the write, it currently doesn't try to DoomEntry. 
It
> > just closes the entry.
> 
> OK. Now we do doom the entry in case of an error, but still leave the closing
to
> the destructor. The destructor still posts a task to the IO thread, since the
> translation cache itself might be (?) allocated and freed on some other
thread.
> Also have a ThreadChecker to sanity check all that.

Does the EXPECT_EQ(0, cache_->Size()) work, or does Doom not change the "Size"
until it's actually evicted?

[Derek Schuff, 2013-06-01 01:47:57]

On 2013/06/01 00:48:41, jvoung (cr) wrote:
> LGTM
> 
>
https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
> File chrome/browser/nacl_host/pnacl_translation_cache_unittest.cc (right):
> 
>
https://codereview.chromium.org/16232011/diff/15001/chrome/browser/nacl_host/...
> chrome/browser/nacl_host/pnacl_translation_cache_unittest.cc:75:
//EXPECT_EQ(0,
> cache_->Size());
> On 2013/05/31 23:15:31, Derek Schuff wrote:
> > On 2013/05/31 01:12:14, jvoung (cr) wrote:
> > > It only hits an error after it's created the entry and then tried to
write,
> > > right?
> > > 
> > > When there's an error on the write, it currently doesn't try to DoomEntry.

> It
> > > just closes the entry.
> > 
> > OK. Now we do doom the entry in case of an error, but still leave the
closing
> to
> > the destructor. The destructor still posts a task to the IO thread, since
the
> > translation cache itself might be (?) allocated and freed on some other
> thread.
> > Also have a ThreadChecker to sanity check all that.
> 
> Does the EXPECT_EQ(0, cache_->Size()) work, or does Doom not change the "Size"
> until it's actually evicted?

It doesn't work, at least in this case. Although here we still have the entry
open until the destructor is called, so I would expect it to at least stay until
it's closed.

[commit-bot: I haz the power, 2013-06-01 01:48:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dschuff@chromium.org/16232011/27001

[commit-bot: I haz the power, 2013-06-01 05:05:26]

Change committed as 203591