Skip to content

Commit

Permalink
Introduce net::CompletionOnceCallback.
Browse files Browse the repository at this point in the history
Also convert net::FileStreamContext to use it, as a sample use case with
few consumers.

This CL does not add a OnceCallback version of TestCompletionCallback,
as the repeating version works for use with methods that take
OnceCallbacks.  I think not creating a new test class will make updating
interfaces take a lot less effort.

Also, I expect that all net::CompletionCallbacks are effectively used
as OnceCallbacks, so we can make TestCompletionCallback return
OnceCallbacks when/if we ever finish converting all of net to use
CompletionOnceCallbacks.

Bug: 714018
Change-Id: Iec76deae075b32fe0c65eb18596859f5e6413647
Reviewed-on: https://chromium-review.googlesource.com/853092
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533061}
  • Loading branch information
Matt Menke authored and Commit Bot committed Jan 30, 2018
1 parent 006fd61 commit dadd6c7
Show file tree
Hide file tree
Showing 12 changed files with 199 additions and 182 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "content/browser/loader/test_resource_handler.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "net/base/completion_callback.h"
#include "net/base/completion_once_callback.h"
#include "net/base/file_stream.h"
#include "net/base/io_buffer.h"
#include "net/base/mime_sniffer.h"
Expand Down Expand Up @@ -91,16 +92,16 @@ class MockFileStream : public net::FileStream {

int Open(const base::FilePath& path,
int open_flags,
const net::CompletionCallback& callback) override {
return ReturnResult(open_result_, callback);
net::CompletionOnceCallback callback) override {
return ReturnResult(open_result_, std::move(callback));
}

int Close(const net::CompletionCallback& callback) override {
int Close(net::CompletionOnceCallback callback) override {
EXPECT_FALSE(closed_);
int result = ReturnResult(
close_result_,
base::BindRepeating(&MockFileStream::SetClosedAndRunCallback,
base::Unretained(this), callback));
base::BindOnce(&MockFileStream::SetClosedAndRunCallback,
base::Unretained(this), std::move(callback)));
if (result != net::ERR_IO_PENDING)
closed_ = true;
return result;
Expand All @@ -111,22 +112,21 @@ class MockFileStream : public net::FileStream {
return false;
}

int Seek(int64_t offset,
const net::Int64CompletionCallback& callback) override {
int Seek(int64_t offset, net::Int64CompletionOnceCallback callback) override {
NOTREACHED();
return net::ERR_UNEXPECTED;
}

int Read(net::IOBuffer* buf,
int buf_len,
const net::CompletionCallback& callback) override {
net::CompletionOnceCallback callback) override {
NOTREACHED();
return net::ERR_UNEXPECTED;
}

int Write(net::IOBuffer* buf,
int buf_len,
const net::CompletionCallback& callback) override {
net::CompletionOnceCallback callback) override {
// 0-byte writes aren't allowed.
EXPECT_GT(buf_len, 0);

Expand All @@ -137,10 +137,10 @@ class MockFileStream : public net::FileStream {
if (write_result.result > 0)
written_data_ += std::string(buf->data(), write_result.result);

return ReturnResult(write_result, callback);
return ReturnResult(write_result, std::move(callback));
}

int Flush(const net::CompletionCallback& callback) override {
int Flush(net::CompletionOnceCallback callback) override {
NOTREACHED();
return net::ERR_UNEXPECTED;
}
Expand Down Expand Up @@ -172,19 +172,19 @@ class MockFileStream : public net::FileStream {
void set_expect_closed(bool expect_closed) { expect_closed_ = expect_closed; }

private:
void SetClosedAndRunCallback(const net::CompletionCallback& callback,
void SetClosedAndRunCallback(net::CompletionOnceCallback callback,
int result) {
EXPECT_FALSE(closed_);
closed_ = true;
callback.Run(result);
std::move(callback).Run(result);
}

int ReturnResult(OperationResult result,
const net::CompletionCallback& callback) {
net::CompletionOnceCallback callback) {
if (result.completion_mode == CompletionMode::SYNC)
return result.result;
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(callback, result.result));
FROM_HERE, base::BindOnce(std::move(callback), result.result));
return net::ERR_IO_PENDING;
}

Expand Down
1 change: 1 addition & 0 deletions net/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ component("net") {
"base/auth.cc",
"base/auth.h",
"base/completion_callback.h",
"base/completion_once_callback.h",
"base/escape.cc",
"base/escape.h",
"base/hash_value.cc",
Expand Down
6 changes: 3 additions & 3 deletions net/base/completion_callback.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef NET_BASE_COMPLETION_CALLBACK_H__
#define NET_BASE_COMPLETION_CALLBACK_H__
#ifndef NET_BASE_COMPLETION_CALLBACK_H_
#define NET_BASE_COMPLETION_CALLBACK_H_

#include <stdint.h>

Expand All @@ -25,4 +25,4 @@ typedef base::CancelableCallback<void(int)> CancelableCompletionCallback;

} // namespace net

#endif // NET_BASE_COMPLETION_CALLBACK_H__
#endif // NET_BASE_COMPLETION_CALLBACK_H_
29 changes: 29 additions & 0 deletions net/base/completion_once_callback.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// 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 NET_BASE_COMPLETION_ONCE_CALLBACK_H_
#define NET_BASE_COMPLETION_ONCE_CALLBACK_H_

#include <stdint.h>

#include "base/callback.h"
#include "base/cancelable_callback.h"

namespace net {

// A OnceCallback specialization that takes a single int parameter. Usually this
// is used to report a byte count or network error code.
typedef base::OnceCallback<void(int)> CompletionOnceCallback;

// 64bit version of the OnceCallback specialization that takes a single int64_t
// parameter. Usually this is used to report a file offset, size or network
// error code.
typedef base::OnceCallback<void(int64_t)> Int64CompletionOnceCallback;

typedef base::CancelableOnceCallback<void(int)>
CancelableCompletionOnceCallback;

} // namespace net

#endif // NET_BASE_COMPLETION_ONCE_CALLBACK_H_
31 changes: 16 additions & 15 deletions net/base/file_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,71 +22,72 @@ FileStream::~FileStream() {
context_.release()->Orphan();
}

int FileStream::Open(const base::FilePath& path, int open_flags,
const CompletionCallback& callback) {
int FileStream::Open(const base::FilePath& path,
int open_flags,
CompletionOnceCallback callback) {
if (IsOpen()) {
DLOG(FATAL) << "File is already open!";
return ERR_UNEXPECTED;
}

DCHECK(open_flags & base::File::FLAG_ASYNC);
context_->Open(path, open_flags, callback);
context_->Open(path, open_flags, std::move(callback));
return ERR_IO_PENDING;
}

int FileStream::Close(const CompletionCallback& callback) {
context_->Close(callback);
int FileStream::Close(CompletionOnceCallback callback) {
context_->Close(std::move(callback));
return ERR_IO_PENDING;
}

bool FileStream::IsOpen() const {
return context_->IsOpen();
}

int FileStream::Seek(int64_t offset, const Int64CompletionCallback& callback) {
int FileStream::Seek(int64_t offset, Int64CompletionOnceCallback callback) {
if (!IsOpen())
return ERR_UNEXPECTED;

context_->Seek(offset, callback);
context_->Seek(offset, std::move(callback));
return ERR_IO_PENDING;
}

int FileStream::Read(IOBuffer* buf,
int buf_len,
const CompletionCallback& callback) {
CompletionOnceCallback callback) {
if (!IsOpen())
return ERR_UNEXPECTED;

// read(..., 0) will return 0, which indicates end-of-file.
DCHECK_GT(buf_len, 0);

return context_->Read(buf, buf_len, callback);
return context_->Read(buf, buf_len, std::move(callback));
}

int FileStream::Write(IOBuffer* buf,
int buf_len,
const CompletionCallback& callback) {
CompletionOnceCallback callback) {
if (!IsOpen())
return ERR_UNEXPECTED;

DCHECK_GE(buf_len, 0);
return context_->Write(buf, buf_len, callback);
return context_->Write(buf, buf_len, std::move(callback));
}

int FileStream::GetFileInfo(base::File::Info* file_info,
const CompletionCallback& callback) {
CompletionOnceCallback callback) {
if (!IsOpen())
return ERR_UNEXPECTED;

context_->GetFileInfo(file_info, callback);
context_->GetFileInfo(file_info, std::move(callback));
return ERR_IO_PENDING;
}

int FileStream::Flush(const CompletionCallback& callback) {
int FileStream::Flush(CompletionOnceCallback callback) {
if (!IsOpen())
return ERR_UNEXPECTED;

context_->Flush(callback);
context_->Flush(std::move(callback));
return ERR_IO_PENDING;
}

Expand Down
23 changes: 12 additions & 11 deletions net/base/file_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

#include "base/files/file.h"
#include "base/macros.h"
#include "net/base/completion_callback.h"
#include "net/base/completion_once_callback.h"
#include "net/base/net_export.h"

namespace base {
Expand Down Expand Up @@ -56,14 +56,15 @@ class NET_EXPORT FileStream {
// automatically closed when FileStream is destructed in an asynchronous
// manner (i.e. the file stream is closed in the background but you don't
// know when).
virtual int Open(const base::FilePath& path, int open_flags,
const CompletionCallback& callback);
virtual int Open(const base::FilePath& path,
int open_flags,
CompletionOnceCallback callback);

// Returns ERR_IO_PENDING and closes the file asynchronously, calling
// |callback| when done.
// It is invalid to request any asynchronous operations while there is an
// in-flight asynchronous operation.
virtual int Close(const CompletionCallback& callback);
virtual int Close(CompletionOnceCallback callback);

// Returns true if Open succeeded and Close has not been called.
virtual bool IsOpen() const;
Expand All @@ -74,7 +75,7 @@ class NET_EXPORT FileStream {
// position relative to the start of the file. Otherwise, an error code is
// returned. It is invalid to request any asynchronous operations while there
// is an in-flight asynchronous operation.
virtual int Seek(int64_t offset, const Int64CompletionCallback& callback);
virtual int Seek(int64_t offset, Int64CompletionOnceCallback callback);

// Call this method to read data from the current stream position
// asynchronously. Up to buf_len bytes will be copied into buf. (In
Expand All @@ -96,8 +97,7 @@ class NET_EXPORT FileStream {
// in-flight asynchronous operation.
//
// This method must not be called if the stream was opened WRITE_ONLY.
virtual int Read(IOBuffer* buf, int buf_len,
const CompletionCallback& callback);
virtual int Read(IOBuffer* buf, int buf_len, CompletionOnceCallback callback);

// Call this method to write data at the current stream position
// asynchronously. Up to buf_len bytes will be written from buf. (In
Expand All @@ -121,8 +121,9 @@ class NET_EXPORT FileStream {
// This method must not be called if the stream was opened READ_ONLY.
//
// Zero byte writes are not allowed.
virtual int Write(IOBuffer* buf, int buf_len,
const CompletionCallback& callback);
virtual int Write(IOBuffer* buf,
int buf_len,
CompletionOnceCallback callback);

// Gets status information about File. May fail synchronously, but never
// succeeds synchronously.
Expand All @@ -132,7 +133,7 @@ class NET_EXPORT FileStream {
//
// |file_info| must remain valid until |callback| is invoked.
virtual int GetFileInfo(base::File::Info* file_info,
const CompletionCallback& callback);
CompletionOnceCallback callback);

// Forces out a filesystem sync on this file to make sure that the file was
// written out to disk and is not currently sitting in the buffer. This does
Expand All @@ -153,7 +154,7 @@ class NET_EXPORT FileStream {
// in-flight asynchronous operation.
//
// This method should not be called if the stream was opened READ_ONLY.
virtual int Flush(const CompletionCallback& callback);
virtual int Flush(CompletionOnceCallback callback);

private:
class Context;
Expand Down
Loading

0 comments on commit dadd6c7

Please sign in to comment.