[Downloads] Retry renames after transient failures.

content/browser/download/base_file_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/download/base_file.h"
 
 #include <windows.h>
 #include <cguid.h>
 #include <objbase.h>
 #include <shellapi.h>
 
+#include "base/files/file.h"
 #include "base/files/file_util.h"
 #include "base/guid.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "content/browser/download/download_interrupt_reasons_impl.h"
 #include "content/browser/download/download_stats.h"
 #include "content/browser/safe_util_win.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace content {
 namespace {
 
 const int kAllSpecialShFileOperationCodes[] = {
   // Should be kept in sync with the case statement below.
   ERROR_ACCESS_DENIED,
+  ERROR_SHARING_VIOLATION,
+  ERROR_INVALID_PARAMETER,
   0x71,
   0x72,
   0x73,
   0x74,
   0x75,
   0x76,
   0x78,
   0x79,
   0x7A,
   0x7C,
@@ skipping 23 matching lines @@
 //
 // See http://msdn.microsoft.com/en-us/library/bb762164(VS.85).aspx.
 DownloadInterruptReason MapShFileOperationCodes(int code) {
   DownloadInterruptReason result = DOWNLOAD_INTERRUPT_REASON_NONE;
 
   // Check these pre-Win32 error codes first, then check for matches
   // in Winerror.h.
   // This switch statement should be kept in sync with the list of codes
   // above.
   switch (code) {
-    // Not a pre-Win32 error code; here so that this particular
-    // case shows up in our histograms.  This is redundant with the
-    // mapping function net::MapSystemError used later.
+    // Not a pre-Win32 error code; here so that this particular case shows up in
+    // our histograms. Unfortunately, it is used not just to signal actual
+    // ACCESS_DENIED errors, but many other errors as well. So we treat it as a
+    // transient error.
     case ERROR_ACCESS_DENIED:  // Access is denied.
-      result = DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED;
+      result = DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR;
+      break;
+
+    // This isn't documented but returned from SHFileOperation. Sharing
+    // violations indicate that another process had the file open while we were
+    // trying to rename. Anti-virus is believed to be the cause of this error in
+    // the wild. Treated as a transient error on the assumption that the file
+    // will be made available for renaming at a later time.
+    case ERROR_SHARING_VIOLATION:
+      result = DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR;
+      break;
+
+    // This is also not a documented return value of SHFileOperation, but has
+    // been observed in the wild. We are treating it as a transient error based
+    // on the cases we have seen so far.  See http://crbug.com/368455.
+    case ERROR_INVALID_PARAMETER:
+      result = DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR;
       break;
 
     // The source and destination files are the same file.
     // DE_SAMEFILE == 0x71
     case 0x71:
       result = DOWNLOAD_INTERRUPT_REASON_FILE_FAILED;
       break;
 
     // The operation was canceled by the user, or silently canceled if the
     // appropriate flags were supplied to SHFileOperation.
@@ skipping 157 matching lines @@
   }
 
   if (result == DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED) {
     UMA_HISTOGRAM_CUSTOM_ENUMERATION(
         "Download.MapWinShErrorAccessDenied", code,
         base::CustomHistogram::ArrayToCustomRanges(
             kAllSpecialShFileOperationCodes,
             arraysize(kAllSpecialShFileOperationCodes)));
   }
 
+  if (result == DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+        "Download.MapWinShErrorTransientError", code,
+        base::CustomHistogram::ArrayToCustomRanges(
+            kAllSpecialShFileOperationCodes,
+            arraysize(kAllSpecialShFileOperationCodes)));
+  }
+
   if (result != DOWNLOAD_INTERRUPT_REASON_NONE)
     return result;
 
   // If not one of the above codes, it should be a standard Windows error code.
-  return ConvertNetErrorToInterruptReason(
-      net::MapSystemError(code), DOWNLOAD_INTERRUPT_FROM_DISK);
+  return ConvertFileErrorToInterruptReason(
+      base::File::OSErrorToFileError(code));
 }
 
 // Maps a return code from ScanAndSaveDownloadedFile() to a
 // DownloadInterruptReason. The return code in |result| is usually from the
 // final IAttachmentExecute::Save() call.
 DownloadInterruptReason MapScanAndSaveErrorCodeToInterruptReason(
     HRESULT result) {
   if (SUCCEEDED(result))
     return DOWNLOAD_INTERRUPT_REASON_NONE;
 
@@ skipping 34 matching lines @@
   source.append(1, L'\0');
   target.append(1, L'\0');
 
   SHFILEOPSTRUCT move_info = {0};
   move_info.wFunc = FO_MOVE;
   move_info.pFrom = source.c_str();
   move_info.pTo = target.c_str();
   move_info.fFlags = FOF_SILENT | FOF_NOCONFIRMATION | FOF_NOERRORUI |
       FOF_NOCONFIRMMKDIR | FOF_NOCOPYSECURITYATTRIBS;
 
+  base::TimeTicks now = base::TimeTicks::Now();
   int result = SHFileOperation(&move_info);
   DownloadInterruptReason interrupt_reason = DOWNLOAD_INTERRUPT_REASON_NONE;
 
   if (result == 0 && move_info.fAnyOperationsAborted)
     interrupt_reason = DOWNLOAD_INTERRUPT_REASON_FILE_FAILED;
   else if (result != 0)
     interrupt_reason = MapShFileOperationCodes(result);
 
   if (interrupt_reason != DOWNLOAD_INTERRUPT_REASON_NONE)
     return LogInterruptReason("SHFileOperation", result, interrupt_reason);
@@ skipping 35 matching lines @@
       RecordDownloadCount(FILE_MISSING_AFTER_SUCCESSFUL_SCAN_COUNT);
       result = DOWNLOAD_INTERRUPT_REASON_FILE_SECURITY_CHECK_FAILED;
     }
     LogInterruptReason("ScanAndSaveDownloadedFile", hr, result);
   }
   bound_net_log_.EndEvent(net::NetLog::TYPE_DOWNLOAD_FILE_ANNOTATED);
   return result;
 }
 
 }  // namespace content