Replace (bool, const std::string&) with base::Optional<std::string>

As per the attached bug, replace usage of Replace (bool, const std::string&) in
filesystem_api_util.cc with base::Optional<std::string> for code health.

Also, change from RepeatingCallback to OnceCallback for all filesystem_api_util
defined methods.

Also, fix linter warnings.

There are no logic changes.

Bug: 729442, 875700
Change-Id: Ib27634b9ba1372cb007ba6a190608ec4128bbeb3
Reviewed-on: https://chromium-review.googlesource.com/1215425Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589842}

chrome/browser/chromeos/file_manager/filesystem_api_util.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/file_manager/filesystem_api_util.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/callback.h"
 #include "base/files/file.h"
@@ -35,53 +36,52 @@ namespace {
 // Helper function used to implement GetNonNativeLocalPathMimeType. It extracts
 // the mime type from the passed Drive resource entry.
 void GetMimeTypeAfterGetResourceEntryForDrive(
-    const base::Callback<void(bool, const std::string&)>& callback,
+    base::OnceCallback<void(const base::Optional<std::string>&)> callback,
     drive::FileError error,
     std::unique_ptr<drive::ResourceEntry> entry) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (error != drive::FILE_ERROR_OK || !entry->has_file_specific_info() ||
       entry->file_specific_info().content_mime_type().empty()) {
-    callback.Run(false, std::string());
+    std::move(callback).Run(base::nullopt);
     return;
   }
-  callback.Run(true, entry->file_specific_info().content_mime_type());
+  std::move(callback).Run(entry->file_specific_info().content_mime_type());
 }
 
 // Helper function used to implement GetNonNativeLocalPathMimeType. It extracts
 // the mime type from the passed metadata from a providing extension.
 void GetMimeTypeAfterGetMetadataForProvidedFileSystem(
-    const base::Callback<void(bool, const std::string&)>& callback,
+    base::OnceCallback<void(const base::Optional<std::string>&)> callback,
     std::unique_ptr<chromeos::file_system_provider::EntryMetadata> metadata,
     base::File::Error result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (result != base::File::FILE_OK || !metadata->mime_type.get()) {
-    callback.Run(false, std::string());
+    std::move(callback).Run(base::nullopt);
     return;
   }
-  callback.Run(true, *metadata->mime_type);
+  std::move(callback).Run(*metadata->mime_type);
 }
 
 // Helper function used to implement GetNonNativeLocalPathMimeType. It passes
 // the returned mime type to the callback.
 void GetMimeTypeAfterGetMimeTypeForArcContentFileSystem(
-    const base::Callback<void(bool, const std::string&)>& callback,
+    base::OnceCallback<void(const base::Optional<std::string>&)> callback,
     const base::Optional<std::string>& mime_type) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (mime_type.has_value()) {
-    callback.Run(true, mime_type.value());
+    std::move(callback).Run(mime_type.value());
   } else {
-    callback.Run(false, std::string());
+    std::move(callback).Run(base::nullopt);
   }
 }
 
 // Helper function to converts a callback that takes boolean value to that takes
 // File::Error, by regarding FILE_OK as the only successful value.
-void BoolCallbackAsFileErrorCallback(
-    const base::Callback<void(bool)>& callback,
-    base::File::Error error) {
-  return callback.Run(error == base::File::FILE_OK);
+void BoolCallbackAsFileErrorCallback(base::OnceCallback<void(bool)> callback,
+                                     base::File::Error error) {
+  return std::move(callback).Run(error == base::File::FILE_OK);
 }
 
 // Part of PrepareFileOnIOThread. It tries to create a new file if the given
@@ -112,15 +112,14 @@ void PrepareFileAfterCheckExistOnIOThread(
 void PrepareFileOnIOThread(
     scoped_refptr<storage::FileSystemContext> file_system_context,
     const storage::FileSystemURL& url,
-    const base::Callback<void(bool)>& callback) {
+    base::OnceCallback<void(bool)> callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   file_system_context->operation_runner()->FileExists(
-      url,
-      base::Bind(&PrepareFileAfterCheckExistOnIOThread,
-                 file_system_context,
-                 url,
-                 base::Bind(&BoolCallbackAsFileErrorCallback, callback)));
+      url, base::Bind(&PrepareFileAfterCheckExistOnIOThread,
+                      file_system_context, url,
+                      base::Bind(&BoolCallbackAsFileErrorCallback,
+                                 base::Passed(std::move(callback)))));
 }
 
 }  // namespace
@@ -160,7 +159,7 @@ bool IsUnderNonNativeLocalPath(Profile* profile,
 void GetNonNativeLocalPathMimeType(
     Profile* profile,
     const base::FilePath& path,
-    const base::Callback<void(bool, const std::string&)>& callback) {
+    base::OnceCallback<void(const base::Optional<std::string>&)> callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(IsUnderNonNativeLocalPath(profile, path));
 
@@ -170,13 +169,14 @@ void GetNonNativeLocalPathMimeType(
     if (!file_system) {
       content::BrowserThread::PostTask(
           content::BrowserThread::UI, FROM_HERE,
-          base::BindOnce(callback, false, std::string()));
+          base::BindOnce(std::move(callback), base::nullopt));
       return;
     }
 
     file_system->GetResourceEntry(
         drive::util::ExtractDrivePath(path),
-        base::BindOnce(&GetMimeTypeAfterGetResourceEntryForDrive, callback));
+        base::BindOnce(&GetMimeTypeAfterGetResourceEntryForDrive,
+                       std::move(callback)));
     return;
   }
 
@@ -186,7 +186,7 @@ void GetNonNativeLocalPathMimeType(
     if (!parser.Parse()) {
       content::BrowserThread::PostTask(
           content::BrowserThread::UI, FROM_HERE,
-          base::BindOnce(callback, false, std::string()));
+          base::BindOnce(std::move(callback), base::nullopt));
       return;
     }
 
@@ -194,8 +194,8 @@ void GetNonNativeLocalPathMimeType(
         parser.file_path(),
         chromeos::file_system_provider::ProvidedFileSystemInterface::
             METADATA_FIELD_MIME_TYPE,
-        base::Bind(&GetMimeTypeAfterGetMetadataForProvidedFileSystem,
-                   callback));
+        base::BindOnce(&GetMimeTypeAfterGetMetadataForProvidedFileSystem,
+                       std::move(callback)));
     return;
   }
 
@@ -207,12 +207,13 @@ void GetNonNativeLocalPathMimeType(
     if (!runner) {
       content::BrowserThread::PostTask(
           content::BrowserThread::UI, FROM_HERE,
-          base::BindOnce(callback, false, std::string()));
+          base::BindOnce(std::move(callback), base::nullopt));
       return;
     }
     runner->GetMimeType(
-        arc_url, base::Bind(&GetMimeTypeAfterGetMimeTypeForArcContentFileSystem,
-                            callback));
+        arc_url,
+        base::BindOnce(&GetMimeTypeAfterGetMimeTypeForArcContentFileSystem,
+                       std::move(callback)));
     return;
   }
 
@@ -221,25 +222,25 @@ void GetNonNativeLocalPathMimeType(
   // file extensions.
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
-      base::BindOnce(callback, false /* failure */, std::string()));
+      base::BindOnce(std::move(callback), base::nullopt));
 }
 
-void IsNonNativeLocalPathDirectory(
-    Profile* profile,
-    const base::FilePath& path,
-    const base::Callback<void(bool)>& callback) {
+void IsNonNativeLocalPathDirectory(Profile* profile,
+                                   const base::FilePath& path,
+                                   base::OnceCallback<void(bool)> callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(IsUnderNonNativeLocalPath(profile, path));
 
   util::CheckIfDirectoryExists(
       GetFileSystemContextForExtensionId(profile, kFileManagerAppId), path,
-      base::Bind(&BoolCallbackAsFileErrorCallback, callback));
+      base::Bind(&BoolCallbackAsFileErrorCallback,
+                 base::Passed(std::move(callback))));
 }
 
 void PrepareNonNativeLocalFileForWritableApp(
     Profile* profile,
     const base::FilePath& path,
-    const base::Callback<void(bool)>& callback) {
+    base::OnceCallback<void(bool)> callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(IsUnderNonNativeLocalPath(profile, path));
 
@@ -248,8 +249,9 @@ void PrepareNonNativeLocalFileForWritableApp(
            profile, path, kFileManagerAppId, &url)) {
     // Posting to the current thread, so that we always call back asynchronously
     // independent from whether or not the operation succeeds.
-    content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-                                     base::BindOnce(callback, false));
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::BindOnce(std::move(callback), false));
     return;
   }
 
@@ -265,7 +267,7 @@ void PrepareNonNativeLocalFileForWritableApp(
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::BindOnce(&PrepareFileOnIOThread, file_system_context, internal_url,
-                     google_apis::CreateRelayCallback(callback)));
+                     google_apis::CreateRelayCallback(std::move(callback))));
 }
 
 }  // namespace util

chrome/browser/chromeos/file_manager/filesystem_api_util.h

@@ -13,6 +13,7 @@
 #include <vector>
 
 #include "base/callback_forward.h"
+#include "base/optional.h"
 #include "storage/common/fileapi/file_system_types.h"
 
 class Profile;
@@ -36,14 +37,13 @@ bool IsUnderNonNativeLocalPath(Profile* profile, const base::FilePath& path);
 void GetNonNativeLocalPathMimeType(
     Profile* profile,
     const base::FilePath& path,
-    const base::Callback<void(bool, const std::string&)>& callback);
+    base::OnceCallback<void(const base::Optional<std::string>&)> callback);
 
 // Checks whether the |path| points to a directory, and asynchronously sends
 // the result to |callback|.
-void IsNonNativeLocalPathDirectory(
-    Profile* profile,
-    const base::FilePath& path,
-    const base::Callback<void(bool)>& callback);
+void IsNonNativeLocalPathDirectory(Profile* profile,
+                                   const base::FilePath& path,
+                                   base::OnceCallback<void(bool)> callback);
 
 // Ensures a file exists at |path|, i.e., it does nothing if a file is already
 // present, or creates a file there if it isn't, and asynchronously sends to
@@ -51,7 +51,7 @@ void IsNonNativeLocalPathDirectory(
 void PrepareNonNativeLocalFileForWritableApp(
     Profile* profile,
     const base::FilePath& path,
-    const base::Callback<void(bool)>& callback);
+    base::OnceCallback<void(bool)> callback);
 
 }  // namespace util
 }  // namespace file_manager

chrome/browser/extensions/api/file_handlers/non_native_file_system_delegate_chromeos.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/extensions/api/file_handlers/non_native_file_system_delegate_chromeos.h"
 
+#include <string>
+#include <utility>
+
 #include "chrome/browser/chromeos/file_manager/filesystem_api_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_context.h"
@@ -24,9 +27,9 @@ bool NonNativeFileSystemDelegateChromeOS::IsUnderNonNativeLocalPath(
 void NonNativeFileSystemDelegateChromeOS::GetNonNativeLocalPathMimeType(
     content::BrowserContext* context,
     const base::FilePath& path,
-    const base::Callback<void(bool, const std::string&)>& callback) {
+    base::OnceCallback<void(const base::Optional<std::string>&)> callback) {
   return file_manager::util::GetNonNativeLocalPathMimeType(
-      Profile::FromBrowserContext(context), path, callback);
+      Profile::FromBrowserContext(context), path, std::move(callback));
 }
 
 // Checks whether |path| points to a non-local filesystem directory and calls

chrome/browser/extensions/api/file_handlers/non_native_file_system_delegate_chromeos.h

@@ -5,8 +5,11 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_API_FILE_HANDLERS_NON_NATIVE_FILE_SYSTEM_DELEGATE_CHROMEOS_H_
 #define CHROME_BROWSER_EXTENSIONS_API_FILE_HANDLERS_NON_NATIVE_FILE_SYSTEM_DELEGATE_CHROMEOS_H_
 
+#include <string>
+
 #include "base/callback.h"
 #include "base/files/file_path.h"
+#include "base/optional.h"
 #include "extensions/browser/api/file_handlers/non_native_file_system_delegate.h"
 
 namespace content {
@@ -27,7 +30,8 @@ class NonNativeFileSystemDelegateChromeOS
   void GetNonNativeLocalPathMimeType(
       content::BrowserContext* context,
       const base::FilePath& path,
-      const base::Callback<void(bool, const std::string&)>& callback) override;
+      base::OnceCallback<void(const base::Optional<std::string>&)> callback)
+      override;
   void IsNonNativeLocalPathDirectory(
       content::BrowserContext* context,
       const base::FilePath& path,

extensions/browser/api/file_handlers/mime_util.cc

@@ -61,10 +61,9 @@ void OnGetMimeTypeFromFileForNonNativeLocalPathCompleted(
 void OnGetMimeTypeFromMetadataForNonNativeLocalPathCompleted(
     const base::FilePath& local_path,
     const base::Callback<void(const std::string&)>& callback,
-    bool success,
-    const std::string& mime_type) {
-  if (success) {
-    callback.Run(mime_type);
+    const base::Optional<std::string>& mime_type) {
+  if (mime_type) {
+    callback.Run(mime_type.value());
     return;
   }
 
@@ -136,8 +135,8 @@ void GetMimeTypeForLocalPath(
     // it can be very slow).
     delegate->GetNonNativeLocalPathMimeType(
         context, local_path,
-        base::Bind(&OnGetMimeTypeFromMetadataForNonNativeLocalPathCompleted,
-                   local_path, callback));
+        base::BindOnce(&OnGetMimeTypeFromMetadataForNonNativeLocalPathCompleted,
+                       local_path, callback));
     return;
   }
 #endif

extensions/browser/api/file_handlers/non_native_file_system_delegate.h

@@ -5,8 +5,11 @@
 #ifndef EXTENSIONS_BROWSER_API_FILE_HANDLERS_NON_NATIVE_FILE_SYSTEM_DELEGATE_H_
 #define EXTENSIONS_BROWSER_API_FILE_HANDLERS_NON_NATIVE_FILE_SYSTEM_DELEGATE_H_
 
+#include <string>
+
 #include "base/callback.h"
 #include "base/files/file_path.h"
+#include "base/optional.h"
 
 namespace content {
 class BrowserContext;
@@ -30,7 +33,8 @@ class NonNativeFileSystemDelegate {
   virtual void GetNonNativeLocalPathMimeType(
       content::BrowserContext* context,
       const base::FilePath& path,
-      const base::Callback<void(bool, const std::string&)>& callback) = 0;
+      base::OnceCallback<void(const base::Optional<std::string>&)>
+          callback) = 0;
 
   // Checks whether |path| points to a non-local filesystem directory and calls
   // |callback| with the result asynchronously.