Convert SyncFileSystemService.h to OnceCallback

This CL converts SyncFileSystemService::DumpFilesCallback and
SyncFileSystemService::ExtensionStatusMapCallback to base::OnceCallbacks.

Bug: 1152272
Change-Id: Idf277ff8057df61849f5f4ec701f94b8ed0c8a05
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2607651
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841216}

chrome/browser/sync_file_system/sync_file_system_service.cc

@@ -114,7 +114,7 @@ std::string SyncFileStatusToString(SyncFileStatus sync_file_status) {
 void DidGetFileSyncStatusForDump(
     base::ListValue* files,
     size_t* num_results,
-    const SyncFileSystemService::DumpFilesCallback& callback,
+    base::RepeatingCallback<void(const base::ListValue&)> callback,
     base::DictionaryValue* file,
     SyncStatusCode sync_status_code,
     SyncFileStatus sync_file_status) {
@@ -129,7 +129,10 @@ void DidGetFileSyncStatusForDump(
   if (++*num_results < files->GetSize())
     return;
 
-  callback.Run(*files);
+  // |callback| is backed by a DumpFilesCallback, which should only be called
+  // once. Move |callback| here to force repeated calls to crash instead of
+  // silently failing.
+  std::move(callback).Run(*files);
 }
 
 // We need this indirection because WeakPtr can only be bound to methods
@@ -300,13 +303,13 @@ void SyncFileSystemService::InitializeForApp(
 
 void SyncFileSystemService::GetExtensionStatusMap(
     ExtensionStatusMapCallback callback) {
-  remote_service_->GetOriginStatusMap(base::AdaptCallbackForRepeating(
+  remote_service_->GetOriginStatusMap(
       base::BindOnce(&SyncFileSystemService::DidGetExtensionStatusMap,
-                     AsWeakPtr(), std::move(callback))));
+                     AsWeakPtr(), std::move(callback)));
 }
 
 void SyncFileSystemService::DumpFiles(const GURL& origin,
-                                      const DumpFilesCallback& callback) {
+                                      DumpFilesCallback callback) {
   DCHECK(!origin.is_empty());
 
   content::StoragePartition* storage_partition =
@@ -315,14 +318,14 @@ void SyncFileSystemService::DumpFiles(const GURL& origin,
       storage_partition->GetFileSystemContext();
   local_service_->MaybeInitializeFileSystemContext(
       origin, file_system_context,
-      base::Bind(&SyncFileSystemService::DidInitializeFileSystemForDump,
-                 AsWeakPtr(), origin, callback));
+      base::BindOnce(&SyncFileSystemService::DidInitializeFileSystemForDump,
+                     AsWeakPtr(), origin, std::move(callback)));
 }
 
-void SyncFileSystemService::DumpDatabase(const DumpFilesCallback& callback) {
+void SyncFileSystemService::DumpDatabase(DumpFilesCallback callback) {
   remote_service_->DumpDatabase(
-      base::Bind(&SyncFileSystemService::DidDumpDatabase,
-                 AsWeakPtr(), callback));
+      base::BindOnce(&SyncFileSystemService::DidDumpDatabase, AsWeakPtr(),
+                     std::move(callback)));
 }
 
 void SyncFileSystemService::GetFileSyncStatus(const FileSystemURL& url,
@@ -538,47 +541,40 @@ void SyncFileSystemService::DidRegisterOrigin(const GURL& app_origin,
 
 void SyncFileSystemService::DidInitializeFileSystemForDump(
     const GURL& origin,
-    const DumpFilesCallback& callback,
+    DumpFilesCallback callback,
     SyncStatusCode status) {
   DCHECK(!origin.is_empty());
 
-  if (status != SYNC_STATUS_OK) {
-    callback.Run(base::ListValue());
-    return;
-  }
-
-  if (!remote_service_) {
-    callback.Run(base::ListValue());
+  if (status != SYNC_STATUS_OK || !remote_service_) {
+    std::move(callback).Run(base::ListValue());
     return;
   }
 
   remote_service_->DumpFiles(
-      origin,
-      base::Bind(
-          &SyncFileSystemService::DidDumpFiles,
-          AsWeakPtr(),
-          origin,
-          callback));
+      origin, base::BindOnce(&SyncFileSystemService::DidDumpFiles, AsWeakPtr(),
+                             origin, std::move(callback)));
 }
 
 void SyncFileSystemService::DidDumpFiles(
     const GURL& origin,
-    const DumpFilesCallback& callback,
+    DumpFilesCallback callback,
     std::unique_ptr<base::ListValue> dump_files) {
   if (!dump_files || !dump_files->GetSize() ||
       !local_service_ || !remote_service_) {
-    callback.Run(base::ListValue());
+    std::move(callback).Run(base::ListValue());
     return;
   }
 
   base::ListValue* files = dump_files.get();
-  base::Callback<void(base::DictionaryValue*,
-                      SyncStatusCode,
-                      SyncFileStatus)> completion_callback =
-      base::Bind(&DidGetFileSyncStatusForDump,
-                 base::Owned(dump_files.release()),
-                 base::Owned(new size_t(0)),
-                 callback);
+
+  using AccumulateFileSyncStatusCallback = base::RepeatingCallback<void(
+      base::DictionaryValue*, SyncStatusCode, SyncFileStatus)>;
+
+  // |accumulate_callback| should only call |callback| once.
+  AccumulateFileSyncStatusCallback accumulate_callback = base::BindRepeating(
+      &DidGetFileSyncStatusForDump, base::Owned(dump_files.release()),
+      base::Owned(new size_t(0)),
+      base::AdaptCallbackForRepeating(std::move(callback)));
 
   // After all metadata loaded, sync status can be added to each entry.
   for (size_t i = 0; i < files->GetSize(); ++i) {
@@ -587,30 +583,30 @@ void SyncFileSystemService::DidDumpFiles(
     if (!files->GetDictionary(i, &file) ||
         !file->GetString("path", &path_string)) {
       NOTREACHED();
-      completion_callback.Run(
-          nullptr, SYNC_FILE_ERROR_FAILED, SYNC_FILE_STATUS_UNKNOWN);
+      accumulate_callback.Run(nullptr, SYNC_FILE_ERROR_FAILED,
+                              SYNC_FILE_STATUS_UNKNOWN);
       continue;
     }
 
     base::FilePath file_path = base::FilePath::FromUTF8Unsafe(path_string);
     FileSystemURL url = CreateSyncableFileSystemURL(origin, file_path);
-    GetFileSyncStatus(url, base::Bind(completion_callback, file));
+    GetFileSyncStatus(url, base::BindOnce(accumulate_callback, file));
   }
 }
 
 void SyncFileSystemService::DidDumpDatabase(
-    const DumpFilesCallback& callback,
+    DumpFilesCallback callback,
     std::unique_ptr<base::ListValue> list) {
   if (!list)
-    list = base::WrapUnique(new base::ListValue);
-  callback.Run(*list);
+    list = std::make_unique<base::ListValue>();
+  std::move(callback).Run(*list);
 }
 
 void SyncFileSystemService::DidGetExtensionStatusMap(
     ExtensionStatusMapCallback callback,
     std::unique_ptr<RemoteFileSyncService::OriginStatusMap> status_map) {
   if (!status_map)
-    status_map = base::WrapUnique(new RemoteFileSyncService::OriginStatusMap);
+    status_map = std::make_unique<RemoteFileSyncService::OriginStatusMap>();
   std::move(callback).Run(*status_map);
 }
 

chrome/browser/sync_file_system/sync_file_system_service.h

@@ -51,10 +51,9 @@ class SyncFileSystemService
       public extensions::ExtensionRegistryObserver,
       public base::SupportsWeakPtr<SyncFileSystemService> {
  public:
-  typedef base::Callback<void(const base::ListValue&)> DumpFilesCallback;
-  typedef base::OnceCallback<void(
-      const RemoteFileSyncService::OriginStatusMap&)>
-      ExtensionStatusMapCallback;
+  using DumpFilesCallback = base::OnceCallback<void(const base::ListValue&)>;
+  using ExtensionStatusMapCallback =
+      base::OnceCallback<void(const RemoteFileSyncService::OriginStatusMap&)>;
 
   // KeyedService implementation.
   void Shutdown() override;
@@ -64,8 +63,8 @@ class SyncFileSystemService
                         SyncStatusCallback callback);
 
   void GetExtensionStatusMap(ExtensionStatusMapCallback callback);
-  void DumpFiles(const GURL& origin, const DumpFilesCallback& callback);
-  void DumpDatabase(const DumpFilesCallback& callback);
+  void DumpFiles(const GURL& origin, DumpFilesCallback callback);
+  void DumpDatabase(DumpFilesCallback callback);
 
   // Returns the file |url|'s sync status.
   void GetFileSyncStatus(const storage::FileSystemURL& url,
@@ -111,13 +110,13 @@ class SyncFileSystemService
                          SyncStatusCode status);
 
   void DidInitializeFileSystemForDump(const GURL& app_origin,
-                                      const DumpFilesCallback& callback,
+                                      DumpFilesCallback callback,
                                       SyncStatusCode status);
   void DidDumpFiles(const GURL& app_origin,
-                    const DumpFilesCallback& callback,
+                    DumpFilesCallback callback,
                     std::unique_ptr<base::ListValue> files);
 
-  void DidDumpDatabase(const DumpFilesCallback& callback,
+  void DidDumpDatabase(DumpFilesCallback callback,
                        std::unique_ptr<base::ListValue> list);
 
   void DidGetExtensionStatusMap(

chrome/browser/sync_file_system/sync_file_system_test_util.cc

@@ -29,9 +29,10 @@ void AssignAndQuit(base::RunLoop* run_loop, R* result_out, R result) {
   run_loop->Quit();
 }
 
-template <typename R> base::Callback<void(R)>
-AssignAndQuitCallback(base::RunLoop* run_loop, R* result) {
-  return base::Bind(&AssignAndQuit<R>, run_loop, base::Unretained(result));
+template <typename R>
+base::OnceCallback<void(R)> AssignAndQuitCallback(base::RunLoop* run_loop,
+                                                  R* result) {
+  return base::BindOnce(&AssignAndQuit<R>, run_loop, base::Unretained(result));
 }
 
 template <typename Arg, typename Param>
@@ -42,19 +43,20 @@ void ReceiveResult1(bool* done, Arg* arg_out, Param arg) {
 }
 
 template <typename Arg>
-base::Callback<void(typename TypeTraits<Arg>::ParamType)>
+base::OnceCallback<void(typename TypeTraits<Arg>::ParamType)>
 CreateResultReceiver(Arg* arg_out) {
-  typedef typename TypeTraits<Arg>::ParamType Param;
-  return base::Bind(&ReceiveResult1<Arg, Param>,
-                    base::Owned(new bool(false)), arg_out);
+  using Param = typename TypeTraits<Arg>::ParamType;
+  return base::BindOnce(&ReceiveResult1<Arg, Param>,
+                        base::Owned(new bool(false)), arg_out);
 }
 
 // Instantiate versions we know callers will need.
-template base::Callback<void(SyncStatusCode)>
-AssignAndQuitCallback(base::RunLoop*, SyncStatusCode*);
+template base::OnceCallback<void(SyncStatusCode)> AssignAndQuitCallback(
+    base::RunLoop*,
+    SyncStatusCode*);
 
 #define INSTANTIATE_RECEIVER(type) \
-  template base::Callback<void(type)> CreateResultReceiver(type*)
+  template base::OnceCallback<void(type)> CreateResultReceiver(type*)
 INSTANTIATE_RECEIVER(SyncStatusCode);
 INSTANTIATE_RECEIVER(google_apis::DriveApiErrorCode);
 INSTANTIATE_RECEIVER(std::unique_ptr<RemoteFileSyncService::OriginStatusMap>);

chrome/browser/sync_file_system/sync_file_system_test_util.h

@@ -52,23 +52,22 @@ void ReceiveResult2(bool* done,
 template <typename R>
 void AssignAndQuit(base::RunLoop* run_loop, R* result_out, R result);
 
-template <typename R> base::Callback<void(R)>
-AssignAndQuitCallback(base::RunLoop* run_loop, R* result);
+template <typename R>
+base::OnceCallback<void(R)> AssignAndQuitCallback(base::RunLoop* run_loop,
+                                                  R* result);
 
 template <typename Arg>
-base::Callback<void(typename TypeTraits<Arg>::ParamType)>
+base::OnceCallback<void(typename TypeTraits<Arg>::ParamType)>
 CreateResultReceiver(Arg* arg_out);
 
 template <typename Arg1, typename Arg2>
-base::Callback<void(typename TypeTraits<Arg1>::ParamType,
-                    typename TypeTraits<Arg2>::ParamType)>
-CreateResultReceiver(Arg1* arg1_out,
-                     Arg2* arg2_out) {
-  typedef typename TypeTraits<Arg1>::ParamType Param1;
-  typedef typename TypeTraits<Arg2>::ParamType Param2;
-  return base::Bind(&ReceiveResult2<Arg1, Arg2, Param1, Param2>,
-                    base::Owned(new bool(false)),
-                    arg1_out, arg2_out);
+base::OnceCallback<void(typename TypeTraits<Arg1>::ParamType,
+                        typename TypeTraits<Arg2>::ParamType)>
+CreateResultReceiver(Arg1* arg1_out, Arg2* arg2_out) {
+  using Param1 = typename TypeTraits<Arg1>::ParamType;
+  using Param2 = typename TypeTraits<Arg2>::ParamType;
+  return base::BindOnce(&ReceiveResult2<Arg1, Arg2, Param1, Param2>,
+                        base::Owned(new bool(false)), arg1_out, arg2_out);
 }
 
 }  // namespace sync_file_system