Break out values of UNKNOWN_FAILURE metrics bucket

This change breaks up the UNKNOWN_FAILURE bucket into more detailed
values so that we can better determine where errors are happening.

Bug: chromium:902923
Change-Id: I814466b081ce5ad0371fff7cd843ed8dd9e9c2e5
Reviewed-on: https://chromium-review.googlesource.com/c/1340976
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611369}

chrome/browser/chromeos/smb_client/smb_errors.cc

@@ -120,16 +120,22 @@ SmbMountResult TranslateErrorToMountResult(smbprovider::ErrorType error) {
       return SmbMountResult::UNSUPPORTED_DEVICE;
     case smbprovider::ERROR_INVALID_URL:
       return SmbMountResult::INVALID_URL;
-    case smbprovider::ERROR_FAILED:
+    case smbprovider::ERROR_IO:
+      return SmbMountResult::IO_ERROR;
     case smbprovider::ERROR_TOO_MANY_OPENED:
+      return SmbMountResult::TOO_MANY_OPENED;
     case smbprovider::ERROR_NO_MEMORY:
     case smbprovider::ERROR_NO_SPACE:
+      return SmbMountResult::OUT_OF_MEMORY;
     case smbprovider::ERROR_INVALID_OPERATION:
+      return SmbMountResult::INVALID_OPERATION;
     case smbprovider::ERROR_ABORT:
+      return SmbMountResult::ABORTED;
+    case smbprovider::ERROR_DBUS_PARSE_FAILED:
+      return SmbMountResult::DBUS_PARSE_FAILED;
     case smbprovider::ERROR_NOT_A_FILE:
     case smbprovider::ERROR_NOT_EMPTY:
-    case smbprovider::ERROR_IO:
-    case smbprovider::ERROR_DBUS_PARSE_FAILED:
+    case smbprovider::ERROR_FAILED:
       return SmbMountResult::UNKNOWN_FAILURE;
     default:
       break;

chrome/browser/chromeos/smb_client/smb_errors.h

@@ -14,17 +14,24 @@ namespace smb_client {
 // These values are written to logs. New enum values may be added, but existing
 // enums must never be renumbered or values reused. Must be kept in sync
 // with the SmbMountResult enum in
-// chrome/browser/resources/settings/downloads_page/smb_browser_proxy.js.
+// ui/webui/resources/cr_components/chromeos/smb_shares/smb_browser_proxy.js
+// and the NativeSmbFileShare_MountResult enum in enums.xml.
 enum class SmbMountResult {
   // TODO(allenvic): Change syntax to kConstantSyntax.
-  SUCCESS = 0,                // Mount succeeded.
-  UNKNOWN_FAILURE = 1,        // Mount failed in an unrecognized way.
-  AUTHENTICATION_FAILED = 2,  // Authentication to the share failed.
-  NOT_FOUND = 3,              // The specified share was not found.
-  UNSUPPORTED_DEVICE = 4,     // The specified share is not supported.
-  MOUNT_EXISTS = 5,           // The specified share is already mounted.
-  INVALID_URL = 6,            // The mount URL is an invalid SMB URL.
-  kMaxValue = INVALID_URL     // Max enum value for use in metrics.
+  SUCCESS = 0,                 // Mount succeeded.
+  UNKNOWN_FAILURE = 1,         // Mount failed in an unrecognized way.
+  AUTHENTICATION_FAILED = 2,   // Authentication to the share failed.
+  NOT_FOUND = 3,               // The specified share was not found.
+  UNSUPPORTED_DEVICE = 4,      // The specified share is not supported.
+  MOUNT_EXISTS = 5,            // The specified share is already mounted.
+  INVALID_URL = 6,             // The mount URL is an invalid SMB URL.
+  INVALID_OPERATION = 7,       // libsmbclient returned invalid operation.
+  DBUS_PARSE_FAILED = 8,       // Parsing the D-Bus message or response failed.
+  OUT_OF_MEMORY = 9,           // The share is out of memory or storage.
+  ABORTED = 10,                // The operation was aborted.
+  IO_ERROR = 11,               // An I/O error occured.
+  TOO_MANY_OPENED = 12,        // There are too many shares open.
+  kMaxValue = TOO_MANY_OPENED  // Max enum value for use in metrics.
 };
 
 // Translates an smbprovider::ErrorType to a base::File::Error. Since

chrome/browser/chromeos/smb_client/smb_errors_unittest.cc

@@ -109,19 +109,19 @@ TEST_F(SmbErrorsTest, SmbErrorToMountResult) {
             TranslateErrorToMountResult(smbprovider::ERROR_NOT_FOUND));
   EXPECT_EQ(SmbMountResult::AUTHENTICATION_FAILED,
             TranslateErrorToMountResult(smbprovider::ERROR_ACCESS_DENIED));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::TOO_MANY_OPENED,
             TranslateErrorToMountResult(smbprovider::ERROR_TOO_MANY_OPENED));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::OUT_OF_MEMORY,
             TranslateErrorToMountResult(smbprovider::ERROR_NO_MEMORY));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::OUT_OF_MEMORY,
             TranslateErrorToMountResult(smbprovider::ERROR_NO_SPACE));
   EXPECT_EQ(SmbMountResult::NOT_FOUND,
             TranslateErrorToMountResult(smbprovider::ERROR_NOT_A_DIRECTORY));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::INVALID_OPERATION,
             TranslateErrorToMountResult(smbprovider::ERROR_INVALID_OPERATION));
   EXPECT_EQ(SmbMountResult::AUTHENTICATION_FAILED,
             TranslateErrorToMountResult(smbprovider::ERROR_SECURITY));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::ABORTED,
             TranslateErrorToMountResult(smbprovider::ERROR_ABORT));
   EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
             TranslateErrorToMountResult(smbprovider::ERROR_NOT_A_FILE));
@@ -129,9 +129,9 @@ TEST_F(SmbErrorsTest, SmbErrorToMountResult) {
             TranslateErrorToMountResult(smbprovider::ERROR_NOT_EMPTY));
   EXPECT_EQ(SmbMountResult::INVALID_URL,
             TranslateErrorToMountResult(smbprovider::ERROR_INVALID_URL));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::IO_ERROR,
             TranslateErrorToMountResult(smbprovider::ERROR_IO));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::DBUS_PARSE_FAILED,
             TranslateErrorToMountResult(smbprovider::ERROR_DBUS_PARSE_FAILED));
 }
 
@@ -149,21 +149,21 @@ TEST_F(SmbErrorsTest, FileErrorToMountResult) {
   EXPECT_EQ(SmbMountResult::AUTHENTICATION_FAILED,
             TranslateErrorToMountResult(base::File::FILE_ERROR_ACCESS_DENIED));
   EXPECT_EQ(
-      SmbMountResult::UNKNOWN_FAILURE,
+      SmbMountResult::TOO_MANY_OPENED,
       TranslateErrorToMountResult(base::File::FILE_ERROR_TOO_MANY_OPENED));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::OUT_OF_MEMORY,
             TranslateErrorToMountResult(base::File::FILE_ERROR_NO_MEMORY));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::OUT_OF_MEMORY,
             TranslateErrorToMountResult(base::File::FILE_ERROR_NO_SPACE));
   EXPECT_EQ(
       SmbMountResult::NOT_FOUND,
       TranslateErrorToMountResult(base::File::FILE_ERROR_NOT_A_DIRECTORY));
   EXPECT_EQ(
-      SmbMountResult::UNKNOWN_FAILURE,
+      SmbMountResult::INVALID_OPERATION,
       TranslateErrorToMountResult(base::File::FILE_ERROR_INVALID_OPERATION));
   EXPECT_EQ(SmbMountResult::AUTHENTICATION_FAILED,
             TranslateErrorToMountResult(base::File::FILE_ERROR_SECURITY));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::ABORTED,
             TranslateErrorToMountResult(base::File::FILE_ERROR_ABORT));
   EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
             TranslateErrorToMountResult(base::File::FILE_ERROR_NOT_A_FILE));
@@ -171,7 +171,7 @@ TEST_F(SmbErrorsTest, FileErrorToMountResult) {
             TranslateErrorToMountResult(base::File::FILE_ERROR_NOT_EMPTY));
   EXPECT_EQ(SmbMountResult::INVALID_URL,
             TranslateErrorToMountResult(base::File::FILE_ERROR_INVALID_URL));
-  EXPECT_EQ(SmbMountResult::UNKNOWN_FAILURE,
+  EXPECT_EQ(SmbMountResult::IO_ERROR,
             TranslateErrorToMountResult(base::File::FILE_ERROR_IO));
 }
 

tools/metrics/histograms/enums.xml

@@ -34531,6 +34531,13 @@ Called by update_use_counter_css.py.-->
   <int value="3" label="Not found"/>
   <int value="4" label="Unsupported device"/>
   <int value="5" label="Mount exists"/>
+  <int value="6" label="Invalid URL format"/>
+  <int value="7" label="Invalid operation"/>
+  <int value="8" label="D-Bus failure"/>
+  <int value="9" label="Out of memory"/>
+  <int value="10" label="Operation aborted"/>
+  <int value="11" label="I/O error"/>
+  <int value="12" label="Too many opened"/>
 </enum>
 
 <enum name="NatTypeCounters">

ui/webui/resources/cr_components/chromeos/smb_shares/smb_browser_proxy.js

@@ -20,6 +20,12 @@ const SmbMountResult = {
   UNSUPPORTED_DEVICE: 4,
   MOUNT_EXISTS: 5,
   INVALID_URL: 6,
+  INVALID_OPERATION: 7,
+  DBUS_PARSE_FAILED: 8,
+  OUT_OF_MEMORY: 9,
+  ABORTED: 10,
+  IO_ERROR: 11,
+  TOO_MANY_OPENED: 12,
 };
 
 /** @enum {string} */