Move error translation to a new class

This change moves error translation to a standalone class and adds tests.
Moving this function out creates a home for other translation functions in
the future, enables testing and makes these functions more reusable across
SMB related code without creating a dependence on SmbFileSystem.

Test=unittests

Bug: chromium:757625
Change-Id: I4d763cde59c0918b6517b2d3d48cb15807f69426
Reviewed-on: https://chromium-review.googlesource.com/1056321
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558986}

chrome/browser/chromeos/BUILD.gn

@@ -1573,6 +1573,8 @@ source_set("chromeos") {
     "smb_client/discovery/network_scanner.h",
     "smb_client/smb_constants.cc",
     "smb_client/smb_constants.h",
+    "smb_client/smb_errors.cc",
+    "smb_client/smb_errors.h",
     "smb_client/smb_file_system.cc",
     "smb_client/smb_file_system.h",
     "smb_client/smb_file_system_id.cc",
@@ -2086,6 +2088,7 @@ source_set("unit_tests") {
     "smb_client/discovery/in_memory_host_locator_unittest.cc",
     "smb_client/discovery/mdns_host_locator_unittest.cc",
     "smb_client/discovery/network_scanner_unittest.cc",
+    "smb_client/smb_errors_unittest.cc",
     "smb_client/smb_file_system_id_test.cc",
     "smb_client/smb_service_helper_unittest.cc",
     "smb_client/smb_service_unittest.cc",

chrome/browser/chromeos/smb_client/smb_errors.cc

+// Copyright 2018 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 "chrome/browser/chromeos/smb_client/smb_errors.h"
+
+namespace chromeos {
+namespace smb_client {
+
+base::File::Error TranslateToFileError(smbprovider::ErrorType error) {
+  DCHECK_NE(smbprovider::ERROR_NONE, error);
+
+  switch (error) {
+    case smbprovider::ERROR_OK:
+      return base::File::FILE_OK;
+    case smbprovider::ERROR_FAILED:
+      return base::File::FILE_ERROR_FAILED;
+    case smbprovider::ERROR_IN_USE:
+      return base::File::FILE_ERROR_IN_USE;
+    case smbprovider::ERROR_EXISTS:
+      return base::File::FILE_ERROR_EXISTS;
+    case smbprovider::ERROR_NOT_FOUND:
+      return base::File::FILE_ERROR_NOT_FOUND;
+    case smbprovider::ERROR_ACCESS_DENIED:
+      return base::File::FILE_ERROR_ACCESS_DENIED;
+    case smbprovider::ERROR_TOO_MANY_OPENED:
+      return base::File::FILE_ERROR_TOO_MANY_OPENED;
+    case smbprovider::ERROR_NO_MEMORY:
+      return base::File::FILE_ERROR_NO_MEMORY;
+    case smbprovider::ERROR_NO_SPACE:
+      return base::File::FILE_ERROR_NO_SPACE;
+    case smbprovider::ERROR_NOT_A_DIRECTORY:
+      return base::File::FILE_ERROR_NOT_A_DIRECTORY;
+    case smbprovider::ERROR_INVALID_OPERATION:
+      return base::File::FILE_ERROR_INVALID_OPERATION;
+    case smbprovider::ERROR_SECURITY:
+      return base::File::FILE_ERROR_SECURITY;
+    case smbprovider::ERROR_ABORT:
+      return base::File::FILE_ERROR_ABORT;
+    case smbprovider::ERROR_NOT_A_FILE:
+      return base::File::FILE_ERROR_NOT_A_FILE;
+    case smbprovider::ERROR_NOT_EMPTY:
+      return base::File::FILE_ERROR_NOT_EMPTY;
+    case smbprovider::ERROR_INVALID_URL:
+      return base::File::FILE_ERROR_INVALID_URL;
+    case smbprovider::ERROR_IO:
+      return base::File::FILE_ERROR_IO;
+    case smbprovider::ERROR_DBUS_PARSE_FAILED:
+      // DBUS_PARSE_FAILED maps to generic ERROR_FAILED
+      LOG(ERROR) << "DBUS PARSE FAILED";
+      return base::File::FILE_ERROR_FAILED;
+    default:
+      break;
+  }
+
+  NOTREACHED();
+  return base::File::FILE_ERROR_FAILED;
+}
+
+}  // namespace smb_client
+}  // namespace chromeos

chrome/browser/chromeos/smb_client/smb_errors.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_SMB_CLIENT_SMB_ERRORS_H_
+#define CHROME_BROWSER_CHROMEOS_SMB_CLIENT_SMB_ERRORS_H_
+
+#include "base/files/file.h"
+#include "chromeos/dbus/smb_provider_client.h"
+
+namespace chromeos {
+namespace smb_client {
+
+// Translates an smbprovider::ErrorType to a base::File::Error. Since
+// smbprovider::ErrorType is a superset of base::File::Error, errors that do not
+// map directly are logged and mapped to the generic failed error.
+base::File::Error TranslateToFileError(smbprovider::ErrorType error);
+
+}  // namespace smb_client
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_SMB_CLIENT_SMB_ERRORS_H_

chrome/browser/chromeos/smb_client/smb_errors_unittest.cc

+// Copyright 2018 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 "chrome/browser/chromeos/smb_client/smb_errors.h"
+
+#include "base/files/file.h"
+#include "chromeos/dbus/smbprovider/directory_entry.pb.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace smb_client {
+
+class SmbErrorsTest : public ::testing::Test {
+ public:
+  SmbErrorsTest() = default;
+  ~SmbErrorsTest() override = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SmbErrorsTest);
+};
+
+TEST_F(SmbErrorsTest, SmbErrorToFileError) {
+  EXPECT_EQ(base::File::FILE_OK, TranslateToFileError(smbprovider::ERROR_OK));
+  EXPECT_EQ(base::File::FILE_ERROR_FAILED,
+            TranslateToFileError(smbprovider::ERROR_FAILED));
+  EXPECT_EQ(base::File::FILE_ERROR_IN_USE,
+            TranslateToFileError(smbprovider::ERROR_IN_USE));
+  EXPECT_EQ(base::File::FILE_ERROR_EXISTS,
+            TranslateToFileError(smbprovider::ERROR_EXISTS));
+  EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND,
+            TranslateToFileError(smbprovider::ERROR_NOT_FOUND));
+  EXPECT_EQ(base::File::FILE_ERROR_ACCESS_DENIED,
+            TranslateToFileError(smbprovider::ERROR_ACCESS_DENIED));
+  EXPECT_EQ(base::File::FILE_ERROR_TOO_MANY_OPENED,
+            TranslateToFileError(smbprovider::ERROR_TOO_MANY_OPENED));
+  EXPECT_EQ(base::File::FILE_ERROR_NO_MEMORY,
+            TranslateToFileError(smbprovider::ERROR_NO_MEMORY));
+  EXPECT_EQ(base::File::FILE_ERROR_NO_SPACE,
+            TranslateToFileError(smbprovider::ERROR_NO_SPACE));
+  EXPECT_EQ(base::File::FILE_ERROR_NOT_A_DIRECTORY,
+            TranslateToFileError(smbprovider::ERROR_NOT_A_DIRECTORY));
+  EXPECT_EQ(base::File::FILE_ERROR_INVALID_OPERATION,
+            TranslateToFileError(smbprovider::ERROR_INVALID_OPERATION));
+  EXPECT_EQ(base::File::FILE_ERROR_SECURITY,
+            TranslateToFileError(smbprovider::ERROR_SECURITY));
+  EXPECT_EQ(base::File::FILE_ERROR_ABORT,
+            TranslateToFileError(smbprovider::ERROR_ABORT));
+  EXPECT_EQ(base::File::FILE_ERROR_NOT_A_FILE,
+            TranslateToFileError(smbprovider::ERROR_NOT_A_FILE));
+  EXPECT_EQ(base::File::FILE_ERROR_NOT_EMPTY,
+            TranslateToFileError(smbprovider::ERROR_NOT_EMPTY));
+  EXPECT_EQ(base::File::FILE_ERROR_INVALID_URL,
+            TranslateToFileError(smbprovider::ERROR_INVALID_URL));
+  EXPECT_EQ(base::File::FILE_ERROR_IO,
+            TranslateToFileError(smbprovider::ERROR_IO));
+
+  EXPECT_EQ(base::File::FILE_ERROR_FAILED,
+            TranslateToFileError(smbprovider::ERROR_DBUS_PARSE_FAILED));
+}
+
+}  // namespace smb_client
+}  // namespace chromeos

chrome/browser/chromeos/smb_client/smb_file_system.cc

@@ -11,6 +11,7 @@
 #include "base/posix/eintr_wrapper.h"
 #include "base/task_scheduler/post_task.h"
 #include "chrome/browser/chromeos/file_system_provider/service.h"
+#include "chrome/browser/chromeos/smb_client/smb_errors.h"
 #include "chrome/browser/chromeos/smb_client/smb_file_system_id.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/smb_provider_client.h"
@@ -91,57 +92,6 @@ SmbFileSystem::SmbFileSystem(
 
 SmbFileSystem::~SmbFileSystem() {}
 
-// static
-base::File::Error SmbFileSystem::TranslateError(smbprovider::ErrorType error) {
-  DCHECK_NE(smbprovider::ERROR_NONE, error);
-
-  switch (error) {
-    case smbprovider::ERROR_OK:
-      return base::File::FILE_OK;
-    case smbprovider::ERROR_FAILED:
-      return base::File::FILE_ERROR_FAILED;
-    case smbprovider::ERROR_IN_USE:
-      return base::File::FILE_ERROR_IN_USE;
-    case smbprovider::ERROR_EXISTS:
-      return base::File::FILE_ERROR_EXISTS;
-    case smbprovider::ERROR_NOT_FOUND:
-      return base::File::FILE_ERROR_NOT_FOUND;
-    case smbprovider::ERROR_ACCESS_DENIED:
-      return base::File::FILE_ERROR_ACCESS_DENIED;
-    case smbprovider::ERROR_TOO_MANY_OPENED:
-      return base::File::FILE_ERROR_TOO_MANY_OPENED;
-    case smbprovider::ERROR_NO_MEMORY:
-      return base::File::FILE_ERROR_NO_MEMORY;
-    case smbprovider::ERROR_NO_SPACE:
-      return base::File::FILE_ERROR_NO_SPACE;
-    case smbprovider::ERROR_NOT_A_DIRECTORY:
-      return base::File::FILE_ERROR_NOT_A_DIRECTORY;
-    case smbprovider::ERROR_INVALID_OPERATION:
-      return base::File::FILE_ERROR_INVALID_OPERATION;
-    case smbprovider::ERROR_SECURITY:
-      return base::File::FILE_ERROR_SECURITY;
-    case smbprovider::ERROR_ABORT:
-      return base::File::FILE_ERROR_ABORT;
-    case smbprovider::ERROR_NOT_A_FILE:
-      return base::File::FILE_ERROR_NOT_A_FILE;
-    case smbprovider::ERROR_NOT_EMPTY:
-      return base::File::FILE_ERROR_NOT_EMPTY;
-    case smbprovider::ERROR_INVALID_URL:
-      return base::File::FILE_ERROR_INVALID_URL;
-    case smbprovider::ERROR_IO:
-      return base::File::FILE_ERROR_IO;
-    case smbprovider::ERROR_DBUS_PARSE_FAILED:
-      // DBUS_PARSE_FAILED maps to generic ERROR_FAILED
-      LOG(ERROR) << "DBUS PARSE FAILED";
-      return base::File::FILE_ERROR_FAILED;
-    default:
-      break;
-  }
-
-  NOTREACHED();
-  return base::File::FILE_ERROR_FAILED;
-}
-
 int32_t SmbFileSystem::GetMountId() const {
   return GetMountIdFromFileSystemId(file_system_info_.file_system_id());
 }
@@ -197,7 +147,7 @@ void SmbFileSystem::HandleRequestUnmountCallback(
     storage::AsyncFileUtil::StatusCallback callback,
     smbprovider::ErrorType error) {
   task_queue_.TaskFinished();
-  base::File::Error result = TranslateError(error);
+  base::File::Error result = TranslateToFileError(error);
   if (result == base::File::FILE_OK) {
     result =
         RunUnmountCallback(file_system_info_.file_system_id(),
@@ -270,7 +220,7 @@ void SmbFileSystem::HandleRequestOpenFileCallback(
     smbprovider::ErrorType error,
     int32_t file_id) const {
   task_queue_.TaskFinished();
-  callback.Run(file_id, TranslateError(error));
+  callback.Run(file_id, TranslateToFileError(error));
 }
 
 AbortCallback SmbFileSystem::CloseFile(
@@ -529,7 +479,7 @@ void SmbFileSystem::HandleRequestReadDirectoryCallback(
     }
   }
 
-  callback.Run(TranslateError(error), entry_list, false /* has_more */);
+  callback.Run(TranslateToFileError(error), entry_list, false /* has_more */);
 }
 
 void SmbFileSystem::HandleGetDeleteListCallback(
@@ -541,7 +491,7 @@ void SmbFileSystem::HandleGetDeleteListCallback(
   if (delete_list.entries_size() == 0) {
     // There are no entries to delete.
     DCHECK_NE(smbprovider::ERROR_OK, list_error);
-    std::move(callback).Run(TranslateError(list_error));
+    std::move(callback).Run(TranslateToFileError(list_error));
     return;
   }
 
@@ -572,7 +522,7 @@ void SmbFileSystem::HandleDeleteEntryCallback(
     if (list_error != smbprovider::ERROR_OK) {
       delete_error = list_error;
     }
-    std::move(callback).Run(TranslateError(delete_error));
+    std::move(callback).Run(TranslateToFileError(delete_error));
   }
 }
 
@@ -583,7 +533,7 @@ void SmbFileSystem::HandleRequestGetMetadataEntryCallback(
     const smbprovider::DirectoryEntryProto& entry) const {
   task_queue_.TaskFinished();
   if (error != smbprovider::ERROR_OK) {
-    std::move(callback).Run(nullptr, TranslateError(error));
+    std::move(callback).Run(nullptr, TranslateToFileError(error));
     return;
   }
   std::unique_ptr<file_system_provider::EntryMetadata> metadata =
@@ -626,7 +576,7 @@ void SmbFileSystem::HandleRequestReadFileCallback(
 
   if (error != smbprovider::ERROR_OK) {
     callback.Run(0 /* chunk_length */, false /* has_more */,
-                 TranslateError(error));
+                 TranslateToFileError(error));
     return;
   }
 
@@ -655,7 +605,7 @@ void SmbFileSystem::HandleStatusCallback(
     smbprovider::ErrorType error) const {
   task_queue_.TaskFinished();
 
-  std::move(callback).Run(TranslateError(error));
+  std::move(callback).Run(TranslateToFileError(error));
 }
 
 void SmbFileSystem::InitTempFileManager() {

chrome/browser/chromeos/smb_client/smb_file_system.h

@@ -54,8 +54,6 @@ class SmbFileSystem : public file_system_provider::ProvidedFileSystemInterface,
       UnmountCallback unmount_callback);
   ~SmbFileSystem() override;
 
-  static base::File::Error TranslateError(smbprovider::ErrorType);
-
   // ProvidedFileSystemInterface overrides.
   file_system_provider::AbortCallback RequestUnmount(
       storage::AsyncFileUtil::StatusCallback callback) override;

chrome/browser/chromeos/smb_client/smb_service.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/smb_client/smb_constants.h"
+#include "chrome/browser/chromeos/smb_client/smb_errors.h"
 #include "chrome/browser/chromeos/smb_client/smb_file_system.h"
 #include "chrome/browser/chromeos/smb_client/smb_file_system_id.h"
 #include "chrome/browser/chromeos/smb_client/smb_provider.h"
@@ -134,7 +135,7 @@ void SmbService::OnMountResponse(
     smbprovider::ErrorType error,
     int32_t mount_id) {
   if (error != smbprovider::ERROR_OK) {
-    std::move(callback).Run(SmbFileSystem::TranslateError(error));
+    std::move(callback).Run(TranslateToFileError(error));
     return;
   }