Skip to content

Commit

Permalink
Add base::GetDeleteFileCallback().
Browse files Browse the repository at this point in the history
There exists many base::DeleteFile() callers that want to use PostTask()
APIs to delete a file on another task runner. To do so, they have to
write:

base::BindOnce(base::IgnoreResult(&base::DeleteFile), path, false)

To simply the callers, and help transition base::DeleteFile() to remove
its |recursive| boolean parameter, add base::GetDeleteFileCallback().
Now, callers can just write:

base::BindOnce(base::GetDeleteFileCallback(), path)

Use base::GetDeleteFileCallback() in chrome/, content/, and ui/.

Bug: 1009837
Change-Id: I06fc6c9585dcb04034b5b25f18f3e96ed94d32c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2203412
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777293}
  • Loading branch information
leizleiz authored and Commit Bot committed Jun 11, 2020
1 parent cdbca9b commit e3aa0b9
Show file tree
Hide file tree
Showing 22 changed files with 50 additions and 55 deletions.
12 changes: 12 additions & 0 deletions base/files/file_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@
namespace base {

#if !defined(OS_NACL_NONSFI)
namespace {

void DeleteFileHelper(const FilePath& path) {
DeleteFile(path, /*recursive=*/false);
}

} // namespace

OnceCallback<void(const FilePath&)> GetDeleteFileCallback() {
return BindOnce(&DeleteFileHelper);
}

int64_t ComputeDirectorySize(const FilePath& root_path) {
int64_t running_size = 0;
FileEnumerator file_iter(root_path, true, FileEnumerator::FILES);
Expand Down
5 changes: 5 additions & 0 deletions base/files/file_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#endif

#include "base/base_export.h"
#include "base/callback_forward.h"
#include "base/containers/span.h"
#include "base/files/file.h"
#include "base/files/file_path.h"
Expand Down Expand Up @@ -87,6 +88,10 @@ BASE_EXPORT bool DeleteFile(const FilePath& path, bool recursive);
// WARNING: USING THIS EQUIVALENT TO "rm -rf", SO USE WITH CAUTION.
BASE_EXPORT bool DeleteFileRecursively(const FilePath& path);

// Simplified way to get a callback to do DeleteFile(path, false) and ignore the
// DeleteFile() result.
BASE_EXPORT OnceCallback<void(const FilePath&)> GetDeleteFileCallback();

#if defined(OS_WIN)
// Schedules to delete the given path, whether it's a file or a directory, until
// the operating system is restarted.
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/apps/app_shim/app_shim_listener.mm
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@
FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), version_path,
false));
base::BindOnce(base::GetDeleteFileCallback(), version_path));
}
}

Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/chromeos/app_mode/kiosk_app_data_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ void KioskAppDataBase::ClearCache() {
if (!icon_path_.empty()) {
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), icon_path_,
false));
base::BindOnce(base::GetDeleteFileCallback(), icon_path_));
}
}

Expand Down
14 changes: 5 additions & 9 deletions chrome/browser/chromeos/crostini/crostini_export_import.cc
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,8 @@ void CrostiniExportImport::Start(
kCrostiniDefaultVmName, path, false,
base::BindOnce(&CrostiniExportImport::ExportAfterSharing,
weak_ptr_factory_.GetWeakPtr(),
operation_data->container_id, std::move(callback))

));
operation_data->container_id,
std::move(callback))));
break;
case ExportImportType::IMPORT:
guest_os::GuestOsSharePath::GetForProfile(profile_)->SharePath(
Expand Down Expand Up @@ -356,8 +355,7 @@ void CrostiniExportImport::OnExportComplete(
// file is functionally the same as a successful cancel.
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile),
it->second->path(), false));
base::BindOnce(base::GetDeleteFileCallback(), it->second->path()));
RemoveTracker(it)->SetStatusCancelled();
break;
}
Expand Down Expand Up @@ -389,8 +387,7 @@ void CrostiniExportImport::OnExportComplete(
// file needs to be cleaned up.
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile),
it->second->path(), false));
base::BindOnce(base::GetDeleteFileCallback(), it->second->path()));
RemoveTracker(it)->SetStatusCancelled();
break;
}
Expand All @@ -401,8 +398,7 @@ void CrostiniExportImport::OnExportComplete(
LOG(ERROR) << "Error exporting " << int(result);
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile),
it->second->path(), false));
base::BindOnce(base::GetDeleteFileCallback(), it->second->path()));
switch (result) {
case CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_VM_STOPPED:
enum_hist_result = ExportContainerResult::kFailedVmStopped;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -955,8 +955,8 @@ void UserImageManagerImpl::DeleteUserImageAndLocalStateEntry(
image_properties->GetString(kImagePathNodeName, &image_path);
if (!image_path.empty()) {
background_task_runner_->PostTask(
FROM_HERE, base::BindOnce(base::IgnoreResult(&base::DeleteFile),
base::FilePath(image_path), false));
FROM_HERE, base::BindOnce(base::GetDeleteFileCallback(),
base::FilePath(image_path)));
}
update->RemoveWithoutPathExpansion(user_id(), nullptr);
}
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/chromeos/system_logs/debug_log_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ void WriteDebugLogToFileCompleted(const base::FilePath& file_path,
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!succeeded) {
bool posted = g_sequenced_task_runner.Get()->PostTaskAndReply(
FROM_HERE,
base::BindOnce(base::IgnoreResult(&base::DeleteFile), file_path, false),
FROM_HERE, base::BindOnce(base::GetDeleteFileCallback(), file_path),
base::BindOnce(std::move(callback), file_path, false));
DCHECK(posted);
return;
Expand Down
12 changes: 4 additions & 8 deletions chrome/browser/media/webrtc/webrtc_rtp_dump_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,13 @@ WebRtcRtpDumpHandler::~WebRtcRtpDumpHandler() {
if (incoming_state_ != STATE_NONE && !incoming_dump_path_.empty()) {
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile),
incoming_dump_path_, false));
base::BindOnce(base::GetDeleteFileCallback(), incoming_dump_path_));
}

if (outgoing_state_ != STATE_NONE && !outgoing_dump_path_.empty()) {
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile),
outgoing_dump_path_, false));
base::BindOnce(base::GetDeleteFileCallback(), outgoing_dump_path_));
}
}

Expand Down Expand Up @@ -295,8 +293,7 @@ void WebRtcRtpDumpHandler::OnDumpEnded(const base::Closure& callback,
if (!incoming_success) {
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile),
incoming_dump_path_, false));
base::BindOnce(base::GetDeleteFileCallback(), incoming_dump_path_));

DVLOG(2) << "Deleted invalid incoming dump "
<< incoming_dump_path_.value();
Expand All @@ -311,8 +308,7 @@ void WebRtcRtpDumpHandler::OnDumpEnded(const base::Closure& callback,
if (!outgoing_success) {
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile),
outgoing_dump_path_, false));
base::BindOnce(base::GetDeleteFileCallback(), outgoing_dump_path_));

DVLOG(2) << "Deleted invalid outgoing dump "
<< outgoing_dump_path_.value();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,7 @@ void CloseFileDescriptor(const int file_descriptor) {
void DeleteTemporaryFile(const base::FilePath& file_path) {
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(base::DeleteFile), file_path,
false /* not recursive*/));
base::BindOnce(base::GetDeleteFileCallback(), file_path));
}

// A fake callback to be passed as CopyFileProgressCallback.
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/metrics/chrome_metrics_service_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ void RegisterOrRemovePreviousRunMetricsFile(
FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), metrics_file,
/*recursive=*/false));
base::BindOnce(base::GetDeleteFileCallback(), metrics_file));
}
}

Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/offline_pages/offline_page_mhtml_archiver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ void DeleteFileOnFileThread(const base::FilePath& file_path,
const base::Closure& callback) {
base::ThreadPool::PostTaskAndReply(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), file_path,
false /* recursive */),
callback);
base::BindOnce(base::GetDeleteFileCallback(), file_path), callback);
}

// Compute a SHA256 digest using a background thread. The computed digest will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ DownloadFeedbackImpl::~DownloadFeedbackImpl() {
}

file_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(base::IgnoreResult(&base::DeleteFile), file_path_, false));
FROM_HERE, base::BindOnce(base::GetDeleteFileCallback(), file_path_));
}

void DownloadFeedbackImpl::Start(const base::Closure& finish_callback) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@ void DownloadFeedbackService::BeginFeedbackOrDeleteFile(
service->BeginFeedback(ping_request, ping_response, path);
} else {
file_task_runner->PostTask(
FROM_HERE,
base::BindOnce(base::IgnoreResult(&base::DeleteFile), path, false));
FROM_HERE, base::BindOnce(base::GetDeleteFileCallback(), path));
}
}

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/search/instant_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ void InstantService::RemoveLocalBackgroundImageCopy() {
chrome::kChromeSearchLocalNtpBackgroundFilename);
base::ThreadPool::PostTask(
FROM_HERE, {base::TaskPriority::BEST_EFFORT, base::MayBlock()},
base::BindOnce(IgnoreResult(&base::DeleteFile), path, false));
base::BindOnce(base::GetDeleteFileCallback(), path));
}

void InstantService::AddValidBackdropUrlForTesting(const GURL& url) const {
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/ui/webui/chromeos/drive_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,7 @@ class LogsZipper : public download::AllDownloadItemNotifier::Observer {
void CleanUp() {
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), zip_path_,
false));
base::BindOnce(base::GetDeleteFileCallback(), zip_path_));
download_notifier_.reset();
if (drive_internals_) {
drive_internals_->OnZipDone();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ void UnregisterFileHandlersWithOs(const AppId& app_id, Profile* profile) {
base::ThreadPool::PostTask(
FROM_HERE,
{base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
base::BindOnce(IgnoreResult(&base::DeleteFile),
app_specific_launcher_path, /*recursively=*/false));
base::BindOnce(base::GetDeleteFileCallback(),
app_specific_launcher_path));
}
base::FilePath only_profile_with_app_installed;
if (IsWebAppLauncherRegisteredWithWindows(app_id, profile,
Expand Down
2 changes: 1 addition & 1 deletion chrome/chrome_cleaner/components/recovery_component.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ void RecoveryComponent::FetchOnIOThread() {
}

base::ScopedClosureRunner delete_file(
base::BindOnce(base::IgnoreResult(&base::DeleteFile), crx_file, false));
base::BindOnce(base::GetDeleteFileCallback(), crx_file));

if (!SaveHttpResponseDataToFile(crx_file, http_response.get())) {
LOG(WARNING) << "Failed to save downloaded recovery component";
Expand Down
2 changes: 1 addition & 1 deletion chrome/chrome_cleaner/logging/pending_logs_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void PendingLogsService::ScheduleLogsUploadTask(

// To get rid of the temporary file if we are not going to use it.
base::ScopedClosureRunner delete_file_closure(
base::BindOnce(IgnoreResult(&base::DeleteFile), temp_file_path, false));
base::BindOnce(base::GetDeleteFileCallback(), temp_file_path));

if (base::WriteFile(temp_file_path, chrome_cleaner_report_string.c_str(),
chrome_cleaner_report_string.size()) <= 0) {
Expand Down
8 changes: 4 additions & 4 deletions chrome/chrome_cleaner/test/generate_test_uws_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ TEST(GenerateTestUwsTest, WriteTestUwS) {
ASSERT_TRUE(base::DeleteFile(uws_file_b, /*recursive=*/false));

// Delete the output files on exit, including on early exit.
base::ScopedClosureRunner delete_uws_file_a(base::BindOnce(
base::IgnoreResult(&base::DeleteFile), uws_file_a, /*recursive=*/false));
base::ScopedClosureRunner delete_uws_file_b(base::BindOnce(
base::IgnoreResult(&base::DeleteFile), uws_file_b, /*recursive=*/false));
base::ScopedClosureRunner delete_uws_file_a(
base::BindOnce(base::GetDeleteFileCallback(), uws_file_a));
base::ScopedClosureRunner delete_uws_file_b(
base::BindOnce(base::GetDeleteFileCallback(), uws_file_b));

// Expect generate_test_uws to finish quickly with exit code 0 (success).
base::Process process(base::LaunchProcess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,7 @@ void NativeFileSystemFileWriterImpl::DoAfterWriteCheck(
// swap file and invoke the callback.
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock()},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), swap_path,
/*recursive=*/false));
base::BindOnce(base::GetDeleteFileCallback(), swap_path));
std::move(callback).Run(native_file_system_error::FromStatus(
NativeFileSystemStatus::kOperationAborted,
"Failed to perform Safe Browsing check."));
Expand Down Expand Up @@ -367,8 +366,7 @@ void NativeFileSystemFileWriterImpl::DidAfterWriteCheck(
// failed.
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock()},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), swap_path,
/*recursive=*/false));
base::BindOnce(base::GetDeleteFileCallback(), swap_path));
std::move(callback).Run(native_file_system_error::FromStatus(
NativeFileSystemStatus::kOperationAborted,
"Write operation blocked by Safe Browsing."));
Expand Down
3 changes: 1 addition & 2 deletions content/browser/web_contents/web_contents_view_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -522,8 +522,7 @@ void WebContentsViewAura::AsyncDropTempFileDeleter::DeleteFileAsync(
FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), std::move(path),
false));
base::BindOnce(base::GetDeleteFileCallback(), std::move(path)));
}
#endif

Expand Down
3 changes: 1 addition & 2 deletions ui/gtk/print_dialog_gtk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,7 @@ void PrintDialogGtk::OnJobCompleted(GtkPrintJob* print_job,
FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), path_to_pdf_,
false));
base::BindOnce(base::GetDeleteFileCallback(), path_to_pdf_));
// Printing finished. Matches AddRef() in PrintDocument();
Release();
}
Expand Down

0 comments on commit e3aa0b9

Please sign in to comment.