Skip to content

Commit

Permalink
File API: Upgrade base::Bind to modern alternatives.
Browse files Browse the repository at this point in the history
Change-Id: I7eba57be3c04b8c8d7963e2043bc3992e7193484
Reviewed-on: https://chromium-review.googlesource.com/c/1293031
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601697}
  • Loading branch information
pwnall authored and Commit Bot committed Oct 22, 2018
1 parent edbe095 commit 3bdb686
Show file tree
Hide file tree
Showing 37 changed files with 192 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ class FakeBackend : public QuotaReservationManager::QuotaBackend {
const url::Origin& origin,
storage::FileSystemType type,
int64_t delta,
const QuotaReservationManager::ReserveQuotaCallback& callback) override {
QuotaReservationManager::ReserveQuotaCallback callback) override {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(base::IgnoreResult(callback),
FROM_HERE, base::BindOnce(base::IgnoreResult(std::move(callback)),
base::File::FILE_OK, delta));
}

Expand Down Expand Up @@ -81,9 +81,8 @@ class QuotaReservationTest : public testing::Test {
void SetUp() override {
ASSERT_TRUE(work_dir_.CreateUniqueTempDir());

reservation_manager_.reset(new QuotaReservationManager(
std::unique_ptr<QuotaReservationManager::QuotaBackend>(
new FakeBackend)));
reservation_manager_ = std::make_unique<QuotaReservationManager>(
std::make_unique<FakeBackend>());
}

void TearDown() override {
Expand Down
22 changes: 11 additions & 11 deletions storage/browser/fileapi/copy_or_move_operation_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,13 @@ class CopyOrMoveOnSameFileSystemImpl
const FileSystemURL& src_url,
const FileSystemURL& dest_url,
CopyOrMoveOperationDelegate::CopyOrMoveOption option,
const FileSystemOperation::CopyFileProgressCallback&
file_progress_callback)
FileSystemOperation::CopyFileProgressCallback file_progress_callback)
: operation_runner_(operation_runner),
operation_type_(operation_type),
src_url_(src_url),
dest_url_(dest_url),
option_(option),
file_progress_callback_(file_progress_callback) {}
file_progress_callback_(std::move(file_progress_callback)) {}

void Run(CopyOrMoveOperationDelegate::StatusCallback callback) override {
if (operation_type_ == CopyOrMoveOperationDelegate::OPERATION_MOVE) {
Expand Down Expand Up @@ -596,12 +595,12 @@ CopyOrMoveOperationDelegate::StreamCopyHelper::StreamCopyHelper(
std::unique_ptr<FileStreamWriter> writer,
storage::FlushPolicy flush_policy,
int buffer_size,
const FileSystemOperation::CopyFileProgressCallback& file_progress_callback,
FileSystemOperation::CopyFileProgressCallback file_progress_callback,
const base::TimeDelta& min_progress_callback_invocation_span)
: reader_(std::move(reader)),
writer_(std::move(writer)),
flush_policy_(flush_policy),
file_progress_callback_(file_progress_callback),
file_progress_callback_(std::move(file_progress_callback)),
io_buffer_(base::MakeRefCounted<net::IOBufferWithSize>(buffer_size)),
num_copied_bytes_(0),
previous_flush_offset_(0),
Expand Down Expand Up @@ -814,8 +813,8 @@ void CopyOrMoveOperationDelegate::ProcessFile(const FileSystemURL& src_url,
operation_type_ == OPERATION_MOVE)) {
impl = std::make_unique<CopyOrMoveOnSameFileSystemImpl>(
operation_runner(), operation_type_, src_url, dest_url, option_,
base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
weak_factory_.GetWeakPtr(), src_url));
base::BindRepeating(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
weak_factory_.GetWeakPtr(), src_url));
} else {
// Cross filesystem case.
base::File::Error error = base::File::FILE_ERROR_FAILED;
Expand All @@ -841,17 +840,18 @@ void CopyOrMoveOperationDelegate::ProcessFile(const FileSystemURL& src_url,
impl = std::make_unique<StreamCopyOrMoveImpl>(
operation_runner(), file_system_context(), operation_type_, src_url,
dest_url, option_, std::move(reader), std::move(writer),
base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
weak_factory_.GetWeakPtr(), src_url));
base::BindRepeating(
&CopyOrMoveOperationDelegate::OnCopyFileProgress,
weak_factory_.GetWeakPtr(), src_url));
}
}

if (!impl) {
impl = std::make_unique<SnapshotCopyOrMoveImpl>(
operation_runner(), operation_type_, src_url, dest_url, option_,
validator_factory,
base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
weak_factory_.GetWeakPtr(), src_url));
base::BindRepeating(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
weak_factory_.GetWeakPtr(), src_url));
}
}

Expand Down
3 changes: 1 addition & 2 deletions storage/browser/fileapi/copy_or_move_operation_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ class CopyOrMoveOperationDelegate
std::unique_ptr<FileStreamWriter> writer,
FlushPolicy flush_policy,
int buffer_size,
const FileSystemOperation::CopyFileProgressCallback&
file_progress_callback,
FileSystemOperation::CopyFileProgressCallback file_progress_callback,
const base::TimeDelta& min_progress_callback_invocation_span);
~StreamCopyHelper();

Expand Down
24 changes: 14 additions & 10 deletions storage/browser/fileapi/copy_or_move_operation_delegate_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -674,10 +674,11 @@ TEST(LocalFileSystemCopyOrMoveOperationTest, ProgressCallback) {
kRegularFileSystemTestCaseSize));

std::vector<ProgressRecord> records;
ASSERT_EQ(base::File::FILE_OK,
helper.CopyWithProgress(src, dest,
base::Bind(&RecordProgressCallback,
base::Unretained(&records))));
ASSERT_EQ(
base::File::FILE_OK,
helper.CopyWithProgress(src, dest,
base::BindRepeating(&RecordProgressCallback,
base::Unretained(&records))));

// Verify progress callback.
for (size_t i = 0; i < kRegularFileSystemTestCaseSize; ++i) {
Expand Down Expand Up @@ -758,12 +759,13 @@ TEST(LocalFileSystemCopyOrMoveOperationTest, StreamCopyHelper) {
std::move(reader), std::move(writer),
storage::FlushPolicy::NO_FLUSH_ON_COMPLETION,
10, // buffer size
base::Bind(&RecordFileProgressCallback, base::Unretained(&progress)),
base::BindRepeating(&RecordFileProgressCallback,
base::Unretained(&progress)),
base::TimeDelta()); // For testing, we need all the progress.

base::File::Error error = base::File::FILE_ERROR_FAILED;
base::RunLoop run_loop;
helper.Run(base::Bind(&AssignAndQuit, &run_loop, &error));
helper.Run(base::BindOnce(&AssignAndQuit, &run_loop, &error));
run_loop.Run();

EXPECT_EQ(base::File::FILE_OK, error);
Expand Down Expand Up @@ -814,12 +816,13 @@ TEST(LocalFileSystemCopyOrMoveOperationTest, StreamCopyHelperWithFlush) {
std::move(reader), std::move(writer),
storage::FlushPolicy::NO_FLUSH_ON_COMPLETION,
10, // buffer size
base::Bind(&RecordFileProgressCallback, base::Unretained(&progress)),
base::BindRepeating(&RecordFileProgressCallback,
base::Unretained(&progress)),
base::TimeDelta()); // For testing, we need all the progress.

base::File::Error error = base::File::FILE_ERROR_FAILED;
base::RunLoop run_loop;
helper.Run(base::Bind(&AssignAndQuit, &run_loop, &error));
helper.Run(base::BindOnce(&AssignAndQuit, &run_loop, &error));
run_loop.Run();

EXPECT_EQ(base::File::FILE_OK, error);
Expand Down Expand Up @@ -865,7 +868,8 @@ TEST(LocalFileSystemCopyOrMoveOperationTest, StreamCopyHelper_Cancel) {
std::move(reader), std::move(writer),
storage::FlushPolicy::NO_FLUSH_ON_COMPLETION,
10, // buffer size
base::Bind(&RecordFileProgressCallback, base::Unretained(&progress)),
base::BindRepeating(&RecordFileProgressCallback,
base::Unretained(&progress)),
base::TimeDelta()); // For testing, we need all the progress.

// Call Cancel() later.
Expand All @@ -876,7 +880,7 @@ TEST(LocalFileSystemCopyOrMoveOperationTest, StreamCopyHelper_Cancel) {

base::File::Error error = base::File::FILE_ERROR_FAILED;
base::RunLoop run_loop;
helper.Run(base::Bind(&AssignAndQuit, &run_loop, &error));
helper.Run(base::BindOnce(&AssignAndQuit, &run_loop, &error));
run_loop.Run();

EXPECT_EQ(base::File::FILE_ERROR_ABORT, error);
Expand Down
2 changes: 1 addition & 1 deletion storage/browser/fileapi/file_system_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class STORAGE_EXPORT FileSystemBackend {
base::OnceCallback<void(const GURL& root_url,
const std::string& name,
base::File::Error error)>;
virtual ~FileSystemBackend() {}
virtual ~FileSystemBackend() = default;

// Returns true if this filesystem backend can handle |type|.
// One filesystem backend may be able to handle multiple filesystem types.
Expand Down
4 changes: 2 additions & 2 deletions storage/browser/fileapi/file_system_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,8 @@ void FileSystemContext::DidOpenFileSystemForResolveURL(
FileSystemOperation::GET_METADATA_FIELD_IS_DIRECTORY |
FileSystemOperation::GET_METADATA_FIELD_SIZE |
FileSystemOperation::GET_METADATA_FIELD_LAST_MODIFIED,
base::Bind(&DidGetMetadataForResolveURL, path, base::Passed(&callback),
info));
base::BindOnce(&DidGetMetadataForResolveURL, path, std::move(callback),
info));
}

} // namespace storage
9 changes: 5 additions & 4 deletions storage/browser/fileapi/file_system_dir_url_request_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,11 @@ void FileSystemDirURLRequestJob::GetMetadata(size_t index) {
url_.path().Append(base::FilePath(entry.name)));
DCHECK(url.is_valid());
file_system_context_->operation_runner()->GetMetadata(
url, FileSystemOperation::GET_METADATA_FIELD_SIZE |
FileSystemOperation::GET_METADATA_FIELD_LAST_MODIFIED,
base::Bind(&FileSystemDirURLRequestJob::DidGetMetadata,
weak_factory_.GetWeakPtr(), index));
url,
FileSystemOperation::GET_METADATA_FIELD_SIZE |
FileSystemOperation::GET_METADATA_FIELD_LAST_MODIFIED,
base::BindOnce(&FileSystemDirURLRequestJob::DidGetMetadata,
weak_factory_.GetWeakPtr(), index));
}

void FileSystemDirURLRequestJob::DidGetMetadata(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,11 @@ class FileSystemDirURLRequestJobTest : public testing::Test {

std::vector<std::unique_ptr<storage::FileSystemBackend>>
additional_providers;
additional_providers.push_back(std::make_unique<TestFileSystemBackend>(
additional_providers.emplace_back(std::make_unique<TestFileSystemBackend>(
base::ThreadTaskRunnerHandle::Get().get(), *mnt_point));

std::vector<storage::URLRequestAutoMountHandler> handlers;
handlers.push_back(base::Bind(&TestAutoMountForURLRequest));
handlers.emplace_back(base::BindRepeating(&TestAutoMountForURLRequest));

file_system_context_ = CreateFileSystemContextWithAutoMountersForTesting(
nullptr, std::move(additional_providers), handlers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ TEST_F(FileSystemFileStreamReaderTest, DeleteWithUnfinishedRead) {
net::TestCompletionCallback callback;
scoped_refptr<net::IOBufferWithSize> buf =
base::MakeRefCounted<net::IOBufferWithSize>(kTestDataSize);
int rv = reader->Read(buf.get(), buf->size(), base::Bind(&NeverCalled));
int rv = reader->Read(buf.get(), buf->size(), base::BindOnce(&NeverCalled));
ASSERT_TRUE(rv == net::ERR_IO_PENDING || rv >= 0);

// Delete immediately.
Expand Down
4 changes: 2 additions & 2 deletions storage/browser/fileapi/file_system_file_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class STORAGE_EXPORT FileSystemFileUtil {
// It will be implemented by each subclass such as FileSystemFileEnumerator.
class STORAGE_EXPORT AbstractFileEnumerator {
public:
virtual ~AbstractFileEnumerator() {}
virtual ~AbstractFileEnumerator() = default;

// Returns an empty string if there are no more results.
virtual base::FilePath Next() = 0;
Expand All @@ -59,7 +59,7 @@ class STORAGE_EXPORT FileSystemFileUtil {
bool IsDirectory() override;
};

virtual ~FileSystemFileUtil() {}
virtual ~FileSystemFileUtil() = default;

// Creates or opens a file with the given flags.
// See header comments for AsyncFileUtil::CreateOrOpen() for more details.
Expand Down
4 changes: 2 additions & 2 deletions storage/browser/fileapi/file_system_operation_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ void FileSystemOperationImpl::WriteBlob(
file_writer_delegate_ = std::move(writer_delegate);
file_writer_delegate_->Start(
std::move(blob_reader),
base::Bind(&FileSystemOperationImpl::DidWrite, weak_factory_.GetWeakPtr(),
url, callback));
base::BindRepeating(&FileSystemOperationImpl::DidWrite,
weak_factory_.GetWeakPtr(), url, callback));
}

void FileSystemOperationImpl::Write(
Expand Down
12 changes: 4 additions & 8 deletions storage/browser/fileapi/file_system_operation_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,8 @@ class FileSystemOperationImplTest
FileSystemOperation::StatusCallback RecordStatusCallback(
const base::Closure& closure,
base::File::Error* status) {
return base::Bind(&FileSystemOperationImplTest::DidFinish,
weak_factory_.GetWeakPtr(),
closure,
status);
return base::BindOnce(&FileSystemOperationImplTest::DidFinish,
weak_factory_.GetWeakPtr(), closure, status);
}

FileSystemOperation::ReadDirectoryCallback RecordReadDirectoryCallback(
Expand All @@ -194,10 +192,8 @@ class FileSystemOperationImplTest
FileSystemOperation::GetMetadataCallback RecordMetadataCallback(
const base::Closure& closure,
base::File::Error* status) {
return base::Bind(&FileSystemOperationImplTest::DidGetMetadata,
weak_factory_.GetWeakPtr(),
closure,
status);
return base::BindOnce(&FileSystemOperationImplTest::DidGetMetadata,
weak_factory_.GetWeakPtr(), closure, status);
}

FileSystemOperation::SnapshotFileCallback RecordSnapshotFileCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class FileSystemOperationImplWriteTest : public testing::Test {

file_system_context_->operation_runner()->CreateFile(
URLForPath(virtual_path_), true /* exclusive */,
base::Bind(&AssertStatusEq, base::File::FILE_OK));
base::BindOnce(&AssertStatusEq, base::File::FILE_OK));

static_cast<TestFileSystemBackend*>(
file_system_context_->GetFileSystemBackend(kFileSystemType))
Expand Down Expand Up @@ -121,13 +121,13 @@ class FileSystemOperationImplWriteTest : public testing::Test {

// Callback function for recording test results.
FileSystemOperation::WriteCallback RecordWriteCallback() {
return base::Bind(&FileSystemOperationImplWriteTest::DidWrite,
weak_factory_.GetWeakPtr());
return base::BindRepeating(&FileSystemOperationImplWriteTest::DidWrite,
weak_factory_.GetWeakPtr());
}

FileSystemOperation::StatusCallback RecordCancelCallback() {
return base::Bind(&FileSystemOperationImplWriteTest::DidCancel,
weak_factory_.GetWeakPtr());
return base::BindOnce(&FileSystemOperationImplWriteTest::DidCancel,
weak_factory_.GetWeakPtr());
}

void DidWrite(base::File::Error status, int64_t bytes, bool complete) {
Expand Down Expand Up @@ -237,7 +237,7 @@ TEST_F(FileSystemOperationImplWriteTest, TestWriteDir) {
base::FilePath virtual_dir_path(FILE_PATH_LITERAL("d"));
file_system_context_->operation_runner()->CreateDirectory(
URLForPath(virtual_dir_path), true /* exclusive */, false /* recursive */,
base::Bind(&AssertStatusEq, base::File::FILE_OK));
base::BindOnce(&AssertStatusEq, base::File::FILE_OK));

ScopedTextBlob blob(url_request_context(), "blob:writedir",
"It\'ll not be written, too.");
Expand Down
4 changes: 2 additions & 2 deletions storage/browser/fileapi/file_system_operation_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ OperationID FileSystemOperationRunner::Copy(
src_url, dest_url, option, error_behavior,
progress_callback.is_null()
? CopyProgressCallback()
: base::Bind(&FileSystemOperationRunner::OnCopyProgress, weak_ptr_,
id, progress_callback),
: base::BindRepeating(&FileSystemOperationRunner::OnCopyProgress,
weak_ptr_, id, progress_callback),
base::BindOnce(&FileSystemOperationRunner::DidFinish, weak_ptr_, id,
std::move(callback)));
return id;
Expand Down
13 changes: 7 additions & 6 deletions storage/browser/fileapi/file_system_url_request_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ int FileSystemURLRequestJob::ReadRawData(net::IOBuffer* dest, int dest_size) {
return 0;

const int rv = reader_->Read(dest, dest_size,
base::Bind(&FileSystemURLRequestJob::DidRead,
weak_factory_.GetWeakPtr()));
base::BindOnce(&FileSystemURLRequestJob::DidRead,
weak_factory_.GetWeakPtr()));
if (rv >= 0) {
remaining_bytes_ -= rv;
DCHECK_GE(remaining_bytes_, 0);
Expand Down Expand Up @@ -162,10 +162,11 @@ void FileSystemURLRequestJob::StartAsync() {
return;
}
file_system_context_->operation_runner()->GetMetadata(
url_, FileSystemOperation::GET_METADATA_FIELD_IS_DIRECTORY |
FileSystemOperation::GET_METADATA_FIELD_SIZE,
base::Bind(&FileSystemURLRequestJob::DidGetMetadata,
weak_factory_.GetWeakPtr()));
url_,
FileSystemOperation::GET_METADATA_FIELD_IS_DIRECTORY |
FileSystemOperation::GET_METADATA_FIELD_SIZE,
base::BindOnce(&FileSystemURLRequestJob::DidGetMetadata,
weak_factory_.GetWeakPtr()));
}

void FileSystemURLRequestJob::DidAttemptAutoMount(base::File::Error result) {
Expand Down
4 changes: 2 additions & 2 deletions storage/browser/fileapi/file_system_usage_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ void FileSystemUsageCache::ScheduleCloseTimer() {
}

timer_.Start(FROM_HERE, kCloseDelay,
base::Bind(&FileSystemUsageCache::CloseCacheFiles,
weak_factory_.GetWeakPtr()));
base::BindOnce(&FileSystemUsageCache::CloseCacheFiles,
weak_factory_.GetWeakPtr()));
}

bool FileSystemUsageCache::HasCacheFileHandle(const base::FilePath& file_path) {
Expand Down
Loading

0 comments on commit 3bdb686

Please sign in to comment.