[Downloads] Fix UMA recording of invalid results from AttachmentServices

content/browser/download/quarantine_win.cc

 // Copyright 2016 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/quarantine.h"
 
 #include <windows.h>
 
 #include <cguid.h>
 #include <objbase.h>
@@ skipping 64 matching lines @@
                 zone_identifier_contents.size(), &actual_length, NULL))
     return false;
   base::StringPiece zone_identifier_string(&zone_identifier_contents.front(),
                                            actual_length);
   return zone_identifier_string.find("[ZoneTransfer]") == 0;
 }
 
 void RecordAttachmentServicesSaveResult(const base::FilePath& file,
                                         HRESULT hr) {
   bool file_exists = base::PathExists(file);
-  if (SUCCEEDED(hr)) {
-    bool motw_exists = file_exists && ZoneIdentifierPresentForFile(file);
-    RecordAttachmentServicesResult(
-        file_exists
-            ? motw_exists ? AttachmentServicesResult::SUCCESS_WITH_MOTW
-                          : AttachmentServicesResult::SUCCESS_WITHOUT_MOTW
-            : AttachmentServicesResult::SUCCESS_WITHOUT_FILE);
-    return;
-  }
-
   switch (hr) {
     case INET_E_SECURITY_PROBLEM:
       RecordAttachmentServicesResult(
           file_exists ? AttachmentServicesResult::BLOCKED_WITH_FILE
                       : AttachmentServicesResult::BLOCKED_WITHOUT_FILE);
       break;
 
     case E_FAIL:
       RecordAttachmentServicesResult(
           file_exists ? AttachmentServicesResult::INFECTED_WITH_FILE
                       : AttachmentServicesResult::INFECTED_WITHOUT_FILE);
       break;
 
     case E_ACCESSDENIED:
     case ERROR_ACCESS_DENIED:
+      // ERROR_ACCESS_DENIED is not a valid HRESULT. However,
+      // IAttachmentExecute::Save() is known to return it and other system error
+      // codes in practice.
       RecordAttachmentServicesResult(
           file_exists ? AttachmentServicesResult::ACCESS_DENIED_WITH_FILE
                       : AttachmentServicesResult::ACCESS_DENIED_WITHOUT_FILE);
       break;
 
     default:
+      if (SUCCEEDED(hr)) {
+        bool motw_exists = file_exists && ZoneIdentifierPresentForFile(file);
+        RecordAttachmentServicesResult(
+            file_exists
+                ? motw_exists ? AttachmentServicesResult::SUCCESS_WITH_MOTW
+                              : AttachmentServicesResult::SUCCESS_WITHOUT_MOTW
+                : AttachmentServicesResult::SUCCESS_WITHOUT_FILE);
+        return;
+      }
+
+      // Failure codes.
       RecordAttachmentServicesResult(
           file_exists ? AttachmentServicesResult::OTHER_WITH_FILE
                       : AttachmentServicesResult::OTHER_WITHOUT_FILE);
   }
 }
 
 // Sets the Zone Identifier on the file to "Internet" (3). Returns true if the
 // function succeeds, false otherwise. A failure is expected if alternate
 // streams are not supported, like a file on a FAT32 filesystem.  This function
 // does not invoke Windows Attachment Execution Services.
@@ skipping 198 matching lines @@
   //
   // If the file is still there, then the error could be due to Windows
   // Attachment Services not being available or some other error during the AES
   // invocation. In either case, we don't surface the error to the user.
   if (!base::PathExists(file))
     return FailedSaveResultToQuarantineResult(save_result);
   return QuarantineFileResult::OK;
 }
 
 }  // namespace content