Remember to delete the not-yet-committed-to-cache file if it's only p…

…artial.

Also fix up a few printf formatting warnings that are detected when
you attempt to turn PLUGIN_PRINTF() off.

BUG=none


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

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

ppapi/native_client/src/trusted/plugin/plugin.cc

@@ -908,7 +908,7 @@ void Plugin::CopyCrashLogToJsConsole() {
   size_t ix_start = 0;
   size_t ix_end;
 
-  PLUGIN_PRINTF(("Plugin::CopyCrashLogToJsConsole: got %d bytes\n",
+  PLUGIN_PRINTF(("Plugin::CopyCrashLogToJsConsole: got %"NACL_PRIuS" bytes\n",
                  fatal_msg.size()));
   while (nacl::string::npos != (ix_end = fatal_msg.find('\n', ix_start))) {
     LogLineToConsole(this, fatal_msg.substr(ix_start, ix_end - ix_start));

ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc

@@ -284,7 +284,7 @@ PnaclCoordinator* PnaclCoordinator::BitcodeToNative(
   coordinator->off_the_record_ =
       plugin->nacl_interface()->IsOffTheRecord();
   PLUGIN_PRINTF(("PnaclCoordinator::BitcodeToNative (manifest=%p, "
-                 "off_the_record=%b)\n",
+                 "off_the_record=%d)\n",
                  reinterpret_cast<const void*>(coordinator->manifest_.get()),
                  coordinator->off_the_record_));
 
@@ -597,23 +597,12 @@ void PnaclCoordinator::DidCopyNexeToCachePartial(int32_t pp_error,
 
 void PnaclCoordinator::NexeWasCopiedToCache(int32_t pp_error) {
   if (pp_error != PP_OK) {
-    // TODO(jvoung): This should try to delete the partially written
-    // cache file before returning...
-    if (pp_error == PP_ERROR_NOQUOTA) {
-      ReportPpapiError(ERROR_PNACL_CACHE_FINALIZE_COPY_NOQUOTA,
-                       pp_error,
-                       "Failed to copy translated nexe to cache (no quota).");
-      return;
-    }
-    if (pp_error == PP_ERROR_NOSPACE) {
-      ReportPpapiError(ERROR_PNACL_CACHE_FINALIZE_COPY_NOSPACE,
-                       pp_error,
-                       "Failed to copy translated nexe to cache (no space).");
-      return;
-    }
-    ReportPpapiError(ERROR_PNACL_CACHE_FINALIZE_COPY_OTHER,
-                     pp_error,
-                     "Failed to copy translated nexe to cache.");
+    // Try to delete the partially written not-yet-committed cache file before
+    // returning. We pass the current pp_error along so that it can be reported
+    // before returning.
+    pp::CompletionCallback cb = callback_factory_.NewCallback(
+        &PnaclCoordinator::CorruptCacheFileWasDeleted, pp_error);
+    cached_nexe_file_->Delete(cb);
     return;
   }
   // Rename the cached_nexe_file_ file to the cache id, to finalize.
@@ -622,6 +611,36 @@ void PnaclCoordinator::NexeWasCopiedToCache(int32_t pp_error) {
   cached_nexe_file_->Rename(cache_identity_, cb);
 }
 
+void PnaclCoordinator::CorruptCacheFileWasDeleted(int32_t delete_pp_error,
+                                                  int32_t orig_pp_error) {
+  if (delete_pp_error != PP_OK) {
+    // The cache file was certainly already opened by the time we tried
+    // to write to it, so it should certainly be deletable.
+    PLUGIN_PRINTF(("PnaclCoordinator::CorruptCacheFileWasDeleted "
+                   "delete failed with pp_error=%"NACL_PRId32"\n",
+                   delete_pp_error));
+    // fall through and report the original error.
+  }
+  // Report the original error that caused us to consider the
+  // cache file corrupted.
+  if (orig_pp_error == PP_ERROR_NOQUOTA) {
+    ReportPpapiError(ERROR_PNACL_CACHE_FINALIZE_COPY_NOQUOTA,
+                     orig_pp_error,
+                     "Failed to copy translated nexe to cache (no quota).");
+    return;
+  }
+  if (orig_pp_error == PP_ERROR_NOSPACE) {
+    ReportPpapiError(ERROR_PNACL_CACHE_FINALIZE_COPY_NOSPACE,
+                     orig_pp_error,
+                     "Failed to copy translated nexe to cache (no space).");
+      return;
+  }
+  ReportPpapiError(ERROR_PNACL_CACHE_FINALIZE_COPY_OTHER,
+                   orig_pp_error,
+                   "Failed to copy translated nexe to cache.");
+  return;
+}
+
 void PnaclCoordinator::NexeFileWasRenamed(int32_t pp_error) {
   PLUGIN_PRINTF(("PnaclCoordinator::NexeFileWasRenamed (pp_error=%"
                  NACL_PRId32")\n", pp_error));
@@ -640,9 +659,7 @@ void PnaclCoordinator::NexeFileWasRenamed(int32_t pp_error) {
       // NOTE: if the file already existed, it looks like the rename will
       // happily succeed.  However, we should add a test for this.
       // Could be a hash collision, or it could also be two tabs racing to
-      // translate the same pexe.  The file could also be a corrupt left-over,
-      // but that case can be removed by doing the TODO for cleanup.
-      // We may want UMA stats to know if this happens.
+      // translate the same pexe. We may want UMA stats to know if this happens.
       // For now, assume that it is a race and try to continue.
       // If there is truly a corrupted file, then sel_ldr should prevent the
       // file from loading due to the file size not matching the ELF header.

ppapi/native_client/src/trusted/plugin/pnacl_coordinator.h

@@ -187,6 +187,11 @@ class PnaclCoordinator: public CallbackSource<FileStreamData> {
   void DidCopyNexeToCachePartial(int32_t pp_error, int32_t num_read_prev,
                                  int64_t cur_offset);
   void NexeWasCopiedToCache(int32_t pp_error);
+  // If the copy of the nexe to the not-yet-committed-to-cache file
+  // failed after partial writes, we attempt to delete the partially written
+  // file. This callback is invoked when the delete is completed.
+  void CorruptCacheFileWasDeleted(int32_t delete_pp_error,
+                                  int32_t orig_pp_error);
   // Invoked when the nexe_file_ temporary has been renamed to the nexe name.
   void NexeFileWasRenamed(int32_t pp_error);
   // Invoked when the read descriptor for nexe_file_ is created.

ppapi/native_client/src/trusted/plugin/pnacl_translate_thread.cc

@@ -74,8 +74,8 @@ void PnaclTranslateThread::RunTranslate(
 // Called from main thread to send bytes to the translator.
 void PnaclTranslateThread::PutBytes(std::vector<char>* bytes,
                                              int count) {
-  PLUGIN_PRINTF(("PutBytes, this %p bytes %p, size %d, count %d\n", this, bytes,
-                 bytes ? bytes->size(): 0, count));
+  PLUGIN_PRINTF(("PutBytes (this=%p, bytes=%p, size=%"NACL_PRIuS", count=%d)\n",
+                 this, bytes, bytes ? bytes->size() : 0, count));
   size_t buffer_size = 0;
   // If we are done (error or not), Signal the translation thread to stop.
   if (count <= PP_OK) {
@@ -179,7 +179,7 @@ void PnaclTranslateThread::DoTranslate() {
     while(!done_ && data_buffers_.size() == 0) {
       NaClXCondVarWait(&buffer_cond_, &cond_mu_);
     }
-    PLUGIN_PRINTF(("PnaclTranslateThread awake, done %d, size %d\n",
+    PLUGIN_PRINTF(("PnaclTranslateThread awake (done=%d, size=%"NACL_PRIuS")\n",
                    done_, data_buffers_.size()));
     if (data_buffers_.size() > 0) {
       std::vector<char> data;