Handle cache-control:no-store header in PNaCl translation cache

Pexe files with the cache-control:no-store header should not be cached.
Add a field to the PnaclCacheInfo struct, plumb the value all the way
from the plugin to the browser, and treat it basically the same way we
currently treat incognito translations (since we currently don't have an
off-the-record cache for those).
R=jvoung@chromium.org
BUG=none, noticed this was missing when doing cleanup

Review URL: https://chromiumcodereview.appspot.com/23458015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221275 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 111 insertions, 58 deletions

diff --git a/chrome/browser/nacl_host/pnacl_host.cc b/chrome/browser/nacl_host/pnacl_host.cc
index 2c3a082..59f579f9 100644
--- a/chrome/browser/nacl_host/pnacl_host.cc
+++ b/chrome/browser/nacl_host/pnacl_host.cc
@@ -45,6 +45,12 @@ PnaclHost::PendingTranslation::PendingTranslation()
       cache_info(nacl::PnaclCacheInfo()) {}
 PnaclHost::PendingTranslation::~PendingTranslation() {}
 
+bool PnaclHost::TranslationMayBeCached(
+    const PendingTranslationMap::iterator& entry) {
+  return !entry->second.is_incognito &&
+         !entry->second.cache_info.has_no_store_header;
+}
+
 /////////////////////////////////////// Initialization
 
 static base::FilePath GetCachePath() {
@@ -268,11 +274,10 @@ void PnaclHost::OnTempFileReturn(const TranslationID& id,
     // waiting for its result.
     LOG(ERROR) << "OnTempFileReturn: temp file creation failed";
     std::string key(entry->second.cache_key);
-    bool is_incognito = entry->second.is_incognito;
     entry->second.callback.Run(fd, false);
     pending_translations_.erase(entry);
-    // No translations will be waiting for an incongnito translation
-    if (!is_incognito)
+    // No translations will be waiting for entries that will not be stored.
+    if (TranslationMayBeCached(entry))
       RequeryMatchingTranslations(key);
     return;
   }
@@ -299,9 +304,9 @@ void PnaclHost::CheckCacheQueryReady(
       if (it->second.cache_key == entry->second.cache_key &&
           // and it's not this translation,
           it->first != entry->first &&
-          // and it's not incognito,
-          !it->second.is_incognito &&
-          // and if it's already gotten past this check and returned the miss.
+          // and it can be stored in the cache,
+          TranslationMayBeCached(it) &&
+          // and it's already gotten past this check and returned the miss.
           it->second.got_cache_reply &&
           it->second.got_nexe_fd) {
         return;
@@ -384,7 +389,7 @@ void PnaclHost::TranslationFinished(int render_process_id,
   // TODO(dschuff): use a separate in-memory cache for incognito
   // translations.
   if (!entry->second.got_nexe_fd || !entry->second.got_cache_reply ||
-      !success || entry->second.is_incognito) {
+      !success || !TranslationMayBeCached(entry)) {
     store_nexe = false;
   } else if (!base::PostTaskAndReplyWithResult(
                   BrowserThread::GetBlockingPool(),
@@ -525,10 +530,9 @@ void PnaclHost::RendererClosing(int render_process_id) {
           base::Bind(base::IgnoreResult(base::ClosePlatformFile),
                      to_erase->second.nexe_fd));
       std::string key(to_erase->second.cache_key);
-      bool is_incognito = to_erase->second.is_incognito;
       pending_translations_.erase(to_erase);
-      // No translations will be blocked waiting for an incongnito translation
-      if (!is_incognito)
+      // No translations will be waiting for entries that will not be stored.
+      if (TranslationMayBeCached(to_erase))
         RequeryMatchingTranslations(key);
     }
   }
diff --git a/chrome/browser/nacl_host/pnacl_host.h b/chrome/browser/nacl_host/pnacl_host.h
index b2f6984..91553f4 100644
--- a/chrome/browser/nacl_host/pnacl_host.h
+++ b/chrome/browser/nacl_host/pnacl_host.h
@@ -125,6 +125,8 @@ class PnaclHost {
 
   typedef std::pair<int, int> TranslationID;
   typedef std::map<TranslationID, PendingTranslation> PendingTranslationMap;
+  static bool TranslationMayBeCached(
+      const PendingTranslationMap::iterator& entry);
 
   void InitForTest(base::FilePath temp_dir);
   void OnCacheInitialized(int net_error);
diff --git a/chrome/browser/nacl_host/pnacl_host_unittest.cc b/chrome/browser/nacl_host/pnacl_host_unittest.cc
index 0ad9cd5..fc26d1a 100644
--- a/chrome/browser/nacl_host/pnacl_host_unittest.cc
+++ b/chrome/browser/nacl_host/pnacl_host_unittest.cc
@@ -47,12 +47,10 @@ class PnaclHostTest : public testing::Test {
     content::BrowserThread::GetBlockingPool()->FlushForTesting();
     base::RunLoop().RunUntilIdle();
   }
-  int GetCacheSize() {
-    return host_->disk_cache_->Size();
-  }
+  int GetCacheSize() { return host_->disk_cache_->Size(); }
 
  public:  // Required for derived classes to bind this method
-  // Callbacks used by tests which call GetNexeFd.
+          // Callbacks used by tests which call GetNexeFd.
   // CallbackExpectMiss checks that the fd is valid and a miss is reported,
   // and also writes some data into the file, which is read back by
   // CallbackExpectHit
@@ -99,21 +97,22 @@ static nacl::PnaclCacheInfo GetTestCacheInfo() {
   info.pexe_url = GURL("http://www.google.com");
   info.abi_version = 0;
   info.opt_level = 0;
+  info.has_no_store_header = false;
   return info;
 }
 
-#define GET_NEXE_FD(renderer, instance, incognito, info, expect_hit)\
-  do {                                                              \
-    SCOPED_TRACE("");                                               \
-    host_->GetNexeFd(                                               \
-        renderer,                                                   \
-        0, /* ignore render_view_id for now */                      \
-        instance,                                                   \
-        incognito,                                                  \
-        info,                                                       \
-        base::Bind(expect_hit ? &PnaclHostTest::CallbackExpectHit   \
-                              : &PnaclHostTest::CallbackExpectMiss, \
-                   base::Unretained(this)));                        \
+#define GET_NEXE_FD(renderer, instance, incognito, info, expect_hit) \
+  do {                                                               \
+    SCOPED_TRACE("");                                                \
+    host_->GetNexeFd(                                                \
+        renderer,                                                    \
+        0, /* ignore render_view_id for now */                       \
+        instance,                                                    \
+        incognito,                                                   \
+        info,                                                        \
+        base::Bind(expect_hit ? &PnaclHostTest::CallbackExpectHit    \
+                              : &PnaclHostTest::CallbackExpectMiss,  \
+                   base::Unretained(this)));                         \
   } while (0)
 
 TEST_F(PnaclHostTest, BasicMiss) {
@@ -164,7 +163,6 @@ TEST_F(PnaclHostTest, BasicHit) {
 
 TEST_F(PnaclHostTest, TranslationErrors) {
   nacl::PnaclCacheInfo info = GetTestCacheInfo();
-  info.pexe_url = GURL("http://www.google.com");
   GET_NEXE_FD(0, 0, false, info, false);
   // Early abort, before temp file request returns
   host_->TranslationFinished(0, 0, false);
@@ -347,6 +345,45 @@ TEST_F(PnaclHostTest, IncognitoSecondOverlappedMiss) {
   EXPECT_EQ(0U, host_->pending_translations());
 }
 
+// Test that pexes with the no-store header do not get cached.
+TEST_F(PnaclHostTest, CacheControlNoStore) {
+  nacl::PnaclCacheInfo info = GetTestCacheInfo();
+  info.has_no_store_header = true;
+  GET_NEXE_FD(0, 0, false, info, false);
+  FlushQueues();
+  EXPECT_EQ(1, temp_callback_count_);
+  host_->TranslationFinished(0, 0, true);
+  FlushQueues();
+  EXPECT_EQ(0U, host_->pending_translations());
+  EXPECT_EQ(0, GetCacheSize());
+}
+
+// Test that no-store pexes do not wait, but do duplicate translations
+TEST_F(PnaclHostTest, NoStoreOverlappedMiss) {
+  nacl::PnaclCacheInfo info = GetTestCacheInfo();
+  info.has_no_store_header = true;
+  GET_NEXE_FD(0, 0, false, info, false);
+  GET_NEXE_FD(0, 1, false, info, false);
+  FlushQueues();
+  // Check that both translations have returned misses, (i.e. that the
+  // second one has not blocked on the first one)
+  EXPECT_EQ(2, temp_callback_count_);
+  host_->TranslationFinished(0, 0, true);
+  host_->TranslationFinished(0, 1, true);
+  FlushQueues();
+  EXPECT_EQ(0U, host_->pending_translations());
+
+  // Same test, but issue the 2nd request after the first has returned a miss.
+  info.abi_version = 222;
+  GET_NEXE_FD(0, 0, false, info, false);
+  FlushQueues();
+  EXPECT_EQ(3, temp_callback_count_);
+  GET_NEXE_FD(0, 1, false, info, false);
+  FlushQueues();
+  EXPECT_EQ(4, temp_callback_count_);
+  host_->RendererClosing(0);
+}
+
 TEST_F(PnaclHostTest, ClearTranslationCache) {
   nacl::PnaclCacheInfo info = GetTestCacheInfo();
   // Add 2 entries in the cache
@@ -362,8 +399,8 @@ TEST_F(PnaclHostTest, ClearTranslationCache) {
   EXPECT_EQ(2, GetCacheSize());
   net::TestCompletionCallback cb;
   // Since we are using a memory backend, the clear should happen immediately.
-  host_->ClearTranslationCacheEntriesBetween(base::Time(), base::Time(),
-                                             base::Bind(cb.callback(), 0));
+  host_->ClearTranslationCacheEntriesBetween(
+      base::Time(), base::Time(), base::Bind(cb.callback(), 0));
   EXPECT_EQ(0, cb.GetResult(net::ERR_IO_PENDING));
   // Check that the translation cache has been cleared
   EXPECT_EQ(0, GetCacheSize());
diff --git a/chrome/renderer/pepper/ppb_nacl_private_impl.cc b/chrome/renderer/pepper/ppb_nacl_private_impl.cc
index 00a3b2c..967ea6d 100644
--- a/chrome/renderer/pepper/ppb_nacl_private_impl.cc
+++ b/chrome/renderer/pepper/ppb_nacl_private_impl.cc
@@ -258,6 +258,7 @@ int32_t GetNexeFd(PP_Instance instance,
                   uint32_t opt_level,
                   const char* last_modified,
                   const char* etag,
+                  PP_Bool has_no_store_header,
                   PP_Bool* is_hit,
                   PP_FileHandle* handle,
                   struct PP_CompletionCallback callback) {
@@ -280,6 +281,7 @@ int32_t GetNexeFd(PP_Instance instance,
   cache_info.opt_level = opt_level;
   cache_info.last_modified = last_modified_time;
   cache_info.etag = std::string(etag);
+  cache_info.has_no_store_header = PP_ToBool(has_no_store_header);
 
   g_pnacl_resource_host.Get()->RequestNexeFd(
       GetRoutingID(instance),
diff --git a/components/nacl/common/nacl_host_messages.h b/components/nacl/common/nacl_host_messages.h
index 0e3880a..4afa1c7 100644
--- a/components/nacl/common/nacl_host_messages.h
+++ b/components/nacl/common/nacl_host_messages.h
@@ -42,6 +42,7 @@ IPC_STRUCT_TRAITS_BEGIN(nacl::PnaclCacheInfo)
   IPC_STRUCT_TRAITS_MEMBER(opt_level)
   IPC_STRUCT_TRAITS_MEMBER(last_modified)
   IPC_STRUCT_TRAITS_MEMBER(etag)
+  IPC_STRUCT_TRAITS_MEMBER(has_no_store_header)
 IPC_STRUCT_TRAITS_END()
 
 // A renderer sends this to the browser process when it wants to start
diff --git a/components/nacl/common/pnacl_types.cc b/components/nacl/common/pnacl_types.cc
index 6c43319..75384b6 100644
--- a/components/nacl/common/pnacl_types.cc
+++ b/components/nacl/common/pnacl_types.cc
@@ -6,7 +6,8 @@
 
 namespace nacl {
 
-PnaclCacheInfo::PnaclCacheInfo() {}
+PnaclCacheInfo::PnaclCacheInfo()
+    : abi_version(0), opt_level(0), has_no_store_header(0) {}
 PnaclCacheInfo::~PnaclCacheInfo() {}
 
 // static
diff --git a/components/nacl/common/pnacl_types.h b/components/nacl/common/pnacl_types.h
index 3fc405a..d1a12c8 100644
--- a/components/nacl/common/pnacl_types.h
+++ b/components/nacl/common/pnacl_types.h
@@ -27,6 +27,7 @@ struct PnaclCacheInfo {
   int opt_level;
   base::Time last_modified;
   std::string etag;
+  bool has_no_store_header;
 };
 
 // Progress information for PNaCl on-demand installs.
diff --git a/ppapi/api/private/ppb_nacl_private.idl b/ppapi/api/private/ppb_nacl_private.idl
index 755b8d7..4bb602f 100644
--- a/ppapi/api/private/ppb_nacl_private.idl
+++ b/ppapi/api/private/ppb_nacl_private.idl
@@ -106,18 +106,19 @@ interface PPB_NaCl_Private {
    */
   PP_FileHandle CreateTemporaryFile([in] PP_Instance instance);
 
-  /* Create a temporary file, which will be deleted by the time the last
-   * handle is closed (or earlier on POSIX systems), to use for the nexe
-   * with the cache information given by |pexe_url|, |abi_version|, |opt_level|,
-   * |last_modified|, and |etag|. If the nexe is already present
-   * in the cache, |is_hit| is set to PP_TRUE and the contents of the nexe
-   * will be copied into the temporary file. Otherwise |is_hit| is set to
-   * PP_FALSE and the temporary file will be writeable.
-   * Currently the implementation is a stub, which always sets is_hit to false
-   * and calls the implementation of CreateTemporaryFile. In a subsequent CL
-   * it will call into the browser which will remember the association between
-   * the cache key and the fd, and copy the nexe into the cache after the
-   * translation finishes.
+  /* Create a temporary file, which will be deleted by the time the
+   * last handle is closed (or earlier on POSIX systems), to use for
+   * the nexe with the cache information given by |pexe_url|,
+   * |abi_version|, |opt_level|, |last_modified|, |etag|, and
+   * |has_no_store_header|. If the nexe is already present in the
+   * cache, |is_hit| is set to PP_TRUE and the contents of the nexe
+   * will be copied into the temporary file. Otherwise |is_hit| is set
+   * to PP_FALSE and the temporary file will be writeable.  Currently
+   * the implementation is a stub, which always sets is_hit to false
+   * and calls the implementation of CreateTemporaryFile. In a
+   * subsequent CL it will call into the browser which will remember
+   * the association between the cache key and the fd, and copy the
+   * nexe into the cache after the translation finishes.
    */
   int32_t GetNexeFd([in] PP_Instance instance,
                     [in] str_t pexe_url,
@@ -125,6 +126,7 @@ interface PPB_NaCl_Private {
                     [in] uint32_t opt_level,
                     [in] str_t last_modified,
                     [in] str_t etag,
+                    [in] PP_Bool has_no_store_header,
                     [out] PP_Bool is_hit,
                     [out] PP_FileHandle nexe_handle,
                     [in] PP_CompletionCallback callback);
diff --git a/ppapi/c/private/ppb_nacl_private.h b/ppapi/c/private/ppb_nacl_private.h
index 772998b..6441efd 100644
--- a/ppapi/c/private/ppb_nacl_private.h
+++ b/ppapi/c/private/ppb_nacl_private.h
@@ -3,7 +3,7 @@
  * found in the LICENSE file.
  */
 
-/* From private/ppb_nacl_private.idl modified Mon Aug 19 14:06:38 2013. */
+/* From private/ppb_nacl_private.idl modified Thu Aug 29 17:42:12 2013. */
 
 #ifndef PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
 #define PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
@@ -122,18 +122,19 @@ struct PPB_NaCl_Private_1_0 {
    * returns a posix handle to that temporary file.
    */
   PP_FileHandle (*CreateTemporaryFile)(PP_Instance instance);
-  /* Create a temporary file, which will be deleted by the time the last
-   * handle is closed (or earlier on POSIX systems), to use for the nexe
-   * with the cache information given by |pexe_url|, |abi_version|, |opt_level|,
-   * |last_modified|, and |etag|. If the nexe is already present
-   * in the cache, |is_hit| is set to PP_TRUE and the contents of the nexe
-   * will be copied into the temporary file. Otherwise |is_hit| is set to
-   * PP_FALSE and the temporary file will be writeable.
-   * Currently the implementation is a stub, which always sets is_hit to false
-   * and calls the implementation of CreateTemporaryFile. In a subsequent CL
-   * it will call into the browser which will remember the association between
-   * the cache key and the fd, and copy the nexe into the cache after the
-   * translation finishes.
+  /* Create a temporary file, which will be deleted by the time the
+   * last handle is closed (or earlier on POSIX systems), to use for
+   * the nexe with the cache information given by |pexe_url|,
+   * |abi_version|, |opt_level|, |last_modified|, |etag|, and
+   * |has_no_store_header|. If the nexe is already present in the
+   * cache, |is_hit| is set to PP_TRUE and the contents of the nexe
+   * will be copied into the temporary file. Otherwise |is_hit| is set
+   * to PP_FALSE and the temporary file will be writeable.  Currently
+   * the implementation is a stub, which always sets is_hit to false
+   * and calls the implementation of CreateTemporaryFile. In a
+   * subsequent CL it will call into the browser which will remember
+   * the association between the cache key and the fd, and copy the
+   * nexe into the cache after the translation finishes.
    */
   int32_t (*GetNexeFd)(PP_Instance instance,
                        const char* pexe_url,
@@ -141,6 +142,7 @@ struct PPB_NaCl_Private_1_0 {
                        uint32_t opt_level,
                        const char* last_modified,
                        const char* etag,
+                       PP_Bool has_no_store_header,
                        PP_Bool* is_hit,
                        PP_FileHandle* nexe_handle,
                        struct PP_CompletionCallback callback);
diff --git a/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc b/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
index 1a29c70..f4d30bf 100644
--- a/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
+++ b/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
@@ -521,6 +521,7 @@ void PnaclCoordinator::BitcodeStreamDidOpen(int32_t pp_error) {
           pnacl_options_.opt_level(),
           parser.GetHeader("last-modified").c_str(),
           parser.GetHeader("etag").c_str(),
+          PP_FromBool(parser.CacheControlNoStore()),
           &is_cache_hit_,
           temp_nexe_file_->existing_handle(),
           cb.pp_completion_callback());
diff --git a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
index 50268de..ab650c9 100644
--- a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
+++ b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
@@ -2848,9 +2848,9 @@ static PP_FileHandle Pnacl_M25_PPB_NaCl_Private_CreateTemporaryFile(PP_Instance
   return iface->CreateTemporaryFile(instance);
 }
 
-static int32_t Pnacl_M25_PPB_NaCl_Private_GetNexeFd(PP_Instance instance, const char* pexe_url, uint32_t abi_version, uint32_t opt_level, const char* last_modified, const char* etag, PP_Bool* is_hit, PP_FileHandle* nexe_handle, struct PP_CompletionCallback* callback) {
+static int32_t Pnacl_M25_PPB_NaCl_Private_GetNexeFd(PP_Instance instance, const char* pexe_url, uint32_t abi_version, uint32_t opt_level, const char* last_modified, const char* etag, PP_Bool has_no_store_header, PP_Bool* is_hit, PP_FileHandle* nexe_handle, struct PP_CompletionCallback* callback) {
   const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
-  return iface->GetNexeFd(instance, pexe_url, abi_version, opt_level, last_modified, etag, is_hit, nexe_handle, *callback);
+  return iface->GetNexeFd(instance, pexe_url, abi_version, opt_level, last_modified, etag, has_no_store_header, is_hit, nexe_handle, *callback);
 }
 
 static void Pnacl_M25_PPB_NaCl_Private_ReportTranslationFinished(PP_Instance instance, PP_Bool success) {
@@ -4684,7 +4684,7 @@ struct PPB_NaCl_Private_1_0 Pnacl_Wrappers_PPB_NaCl_Private_1_0 = {
     .EnsurePnaclInstalled = (int32_t (*)(PP_Instance instance, struct PP_CompletionCallback callback))&Pnacl_M25_PPB_NaCl_Private_EnsurePnaclInstalled,
     .GetReadonlyPnaclFd = (PP_FileHandle (*)(const char* filename))&Pnacl_M25_PPB_NaCl_Private_GetReadonlyPnaclFd,
     .CreateTemporaryFile = (PP_FileHandle (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_CreateTemporaryFile,
-    .GetNexeFd = (int32_t (*)(PP_Instance instance, const char* pexe_url, uint32_t abi_version, uint32_t opt_level, const char* last_modified, const char* etag, PP_Bool* is_hit, PP_FileHandle* nexe_handle, struct PP_CompletionCallback callback))&Pnacl_M25_PPB_NaCl_Private_GetNexeFd,
+    .GetNexeFd = (int32_t (*)(PP_Instance instance, const char* pexe_url, uint32_t abi_version, uint32_t opt_level, const char* last_modified, const char* etag, PP_Bool has_no_store_header, PP_Bool* is_hit, PP_FileHandle* nexe_handle, struct PP_CompletionCallback callback))&Pnacl_M25_PPB_NaCl_Private_GetNexeFd,
     .ReportTranslationFinished = (void (*)(PP_Instance instance, PP_Bool success))&Pnacl_M25_PPB_NaCl_Private_ReportTranslationFinished,
     .IsOffTheRecord = (PP_Bool (*)(void))&Pnacl_M25_PPB_NaCl_Private_IsOffTheRecord,
     .IsPnaclEnabled = (PP_Bool (*)(void))&Pnacl_M25_PPB_NaCl_Private_IsPnaclEnabled,