Skip to content

Commit

Permalink
Remove some PlatformFile instances from ChromeOS Drive.
Browse files Browse the repository at this point in the history
BUG=322664

Review URL: https://codereview.chromium.org/211483004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260326 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rvargas@chromium.org committed Mar 29, 2014
1 parent faf59c4 commit b7617c7
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 170 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/drive/drive_file_stream_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ int LocalReaderProxy::Read(net::IOBuffer* buffer, int buffer_length,
buffer_length = static_cast<int>(remaining_length_);
}

if (!buffer_length)
return 0;

file_reader_->Read(buffer, buffer_length,
base::Bind(&LocalReaderProxy::OnReadCompleted,
weak_ptr_factory_.GetWeakPtr(), callback));
Expand Down
18 changes: 5 additions & 13 deletions chrome/browser/chromeos/drive/file_system/truncate_operation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@

#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/files/file.h"
#include "base/files/file_path.h"
#include "base/files/scoped_platform_file_closer.h"
#include "base/logging.h"
#include "base/message_loop/message_loop_proxy.h"
#include "base/platform_file.h"
#include "base/sequenced_task_runner.h"
#include "base/task_runner_util.h"
#include "chrome/browser/chromeos/drive/drive.pb.h"
Expand Down Expand Up @@ -42,19 +41,12 @@ FileError TruncateOnBlockingPool(internal::ResourceMetadata* metadata,
if (error != FILE_ERROR_OK)
return error;

base::PlatformFileError result = base::PLATFORM_FILE_ERROR_FAILED;
base::PlatformFile file = base::CreatePlatformFile(
local_cache_path,
base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_WRITE,
NULL,
&result);
if (result != base::PLATFORM_FILE_OK)
base::File file(local_cache_path,
base::File::FLAG_OPEN | base::File::FLAG_WRITE);
if (!file.IsValid())
return FILE_ERROR_FAILED;

DCHECK_NE(base::kInvalidPlatformFileValue, file);
base::ScopedPlatformFileCloser platform_file_closer(&file);

if (!base::TruncatePlatformFile(file, length))
if (!file.SetLength(length))
return FILE_ERROR_FAILED;

return FILE_ERROR_OK;
Expand Down
164 changes: 28 additions & 136 deletions chrome/browser/chromeos/drive/local_file_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,173 +4,65 @@

#include "chrome/browser/chromeos/drive/local_file_reader.h"

#include <errno.h>

#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/files/file_path.h"
#include "base/location.h"
#include "base/platform_file.h"
#include "base/sequenced_task_runner.h"
#include "base/task_runner_util.h"
#include "net/base/completion_callback.h"
#include "net/base/io_buffer.h"
#include "net/base/net_errors.h"

namespace drive {
namespace util {

namespace {

// Opens the file at |file_path| and seeks to the |offset| from begin.
// Returns the net::Error code. If succeeded, |platform_file| is set to point
// the opened file.
// This function should run on the blocking pool.
int OpenAndSeekOnBlockingPool(const base::FilePath& file_path,
int64 offset,
base::PlatformFile* platform_file) {
DCHECK(platform_file);
DCHECK_EQ(base::kInvalidPlatformFileValue, *platform_file);

// First of all, open the file.
const int open_flags = base::PLATFORM_FILE_OPEN |
base::PLATFORM_FILE_READ |
base::PLATFORM_FILE_ASYNC;
base::PlatformFileError error = base::PLATFORM_FILE_ERROR_FAILED;
base::PlatformFile file =
base::CreatePlatformFile(file_path, open_flags, NULL, &error);
if (file == base::kInvalidPlatformFileValue) {
DCHECK_NE(base::PLATFORM_FILE_OK, error);
return net::FileErrorToNetError(static_cast<base::File::Error>(error));
}

// If succeeded, seek to the |offset| from begin.
int64 position = base::SeekPlatformFile(
file, base::PLATFORM_FILE_FROM_BEGIN, offset);
if (position < 0) {
// If failed, close the file and return an error.
base::ClosePlatformFile(file);
return net::ERR_FAILED;
}

*platform_file = file;
return net::OK;
}

// Reads the data from the |platform_file| and copies it to the |buffer| at
// most |buffer_length| size. Returns the number of copied bytes if succeeded,
// or the net::Error code.
// This function should run on the blocking pool.
int ReadOnBlockingPool(base::PlatformFile platform_file,
scoped_refptr<net::IOBuffer> buffer,
int buffer_length) {
DCHECK_NE(base::kInvalidPlatformFileValue, platform_file);
int result = base::ReadPlatformFileCurPosNoBestEffort(
platform_file, buffer->data(), buffer_length);
return result < 0 ? net::MapSystemError(errno) : result;
}

// Posts a task to close the |platform_file| into |task_runner|.
// Or, if |platform_file| is kInvalidPlatformFileValue, does nothing.
void PostCloseIfNeeded(base::TaskRunner* task_runner,
base::PlatformFile platform_file) {
DCHECK(task_runner);
if (platform_file != base::kInvalidPlatformFileValue) {
task_runner->PostTask(
FROM_HERE,
base::Bind(
base::IgnoreResult(&base::ClosePlatformFile), platform_file));
}
}

} // namespace

class LocalFileReader::ScopedPlatformFile {
public:
explicit ScopedPlatformFile(base::TaskRunner* task_runner)
: task_runner_(task_runner),
platform_file_(base::kInvalidPlatformFileValue) {
DCHECK(task_runner);
}

~ScopedPlatformFile() {
PostCloseIfNeeded(task_runner_.get(), platform_file_);
}

base::PlatformFile* ptr() { return &platform_file_; }

base::PlatformFile release() {
base::PlatformFile result = platform_file_;
platform_file_ = base::kInvalidPlatformFileValue;
return result;
}

private:
scoped_refptr<base::TaskRunner> task_runner_;
base::PlatformFile platform_file_;

DISALLOW_COPY_AND_ASSIGN(ScopedPlatformFile);
};

LocalFileReader::LocalFileReader(
base::SequencedTaskRunner* sequenced_task_runner)
: sequenced_task_runner_(sequenced_task_runner),
platform_file_(base::kInvalidPlatformFileValue),
: file_stream_(NULL, sequenced_task_runner),
weak_ptr_factory_(this) {
DCHECK(sequenced_task_runner_.get());
}

LocalFileReader::~LocalFileReader() {
PostCloseIfNeeded(sequenced_task_runner_.get(), platform_file_);
}

void LocalFileReader::Open(const base::FilePath& file_path,
int64 offset,
const net::CompletionCallback& callback) {
DCHECK(!callback.is_null());
DCHECK_EQ(base::kInvalidPlatformFileValue, platform_file_);

ScopedPlatformFile* platform_file =
new ScopedPlatformFile(sequenced_task_runner_.get());
base::PostTaskAndReplyWithResult(
sequenced_task_runner_.get(),
FROM_HERE,
base::Bind(
&OpenAndSeekOnBlockingPool, file_path, offset, platform_file->ptr()),
base::Bind(&LocalFileReader::OpenAfterBlockingPoolTask,
weak_ptr_factory_.GetWeakPtr(),
callback,
base::Owned(platform_file)));
int flags = base::File::FLAG_OPEN | base::File::FLAG_READ |
base::PLATFORM_FILE_ASYNC;

int rv = file_stream_.Open(file_path, flags,
Bind(&LocalFileReader::DidOpen,
weak_ptr_factory_.GetWeakPtr(),
callback, offset));
DCHECK_EQ(rv, net::ERR_IO_PENDING);
}

void LocalFileReader::Read(net::IOBuffer* in_buffer,
int buffer_length,
const net::CompletionCallback& callback) {
DCHECK(!callback.is_null());
DCHECK_NE(base::kInvalidPlatformFileValue, platform_file_);

scoped_refptr<net::IOBuffer> buffer(in_buffer);
base::PostTaskAndReplyWithResult(
sequenced_task_runner_.get(),
FROM_HERE,
base::Bind(&ReadOnBlockingPool, platform_file_, buffer, buffer_length),
callback);
DCHECK(file_stream_.IsOpen());
int rv = file_stream_.Read(in_buffer, buffer_length, callback);
DCHECK_EQ(rv, net::ERR_IO_PENDING);
}

void LocalFileReader::OpenAfterBlockingPoolTask(
const net::CompletionCallback& callback,
ScopedPlatformFile* platform_file,
int open_result) {
DCHECK(!callback.is_null());
DCHECK(platform_file);
DCHECK_EQ(base::kInvalidPlatformFileValue, platform_file_);

if (open_result == net::OK) {
DCHECK_NE(base::kInvalidPlatformFileValue, *platform_file->ptr());
platform_file_ = platform_file->release();
}
void LocalFileReader::DidOpen(const net::CompletionCallback& callback,
int64 offset,
int error) {
if (error != net::OK)
return callback.Run(error);

int rv = file_stream_.Seek(net::FROM_BEGIN, offset,
Bind(&LocalFileReader::DidSeek,
weak_ptr_factory_.GetWeakPtr(),
callback, offset));
DCHECK_EQ(rv, net::ERR_IO_PENDING);
}

callback.Run(open_result);
void LocalFileReader::DidSeek(const net::CompletionCallback& callback,
int64 offset,
int64 error) {
callback.Run(error == offset ? net::OK : net::ERR_FAILED);
}

} // namespace util
Expand Down
22 changes: 9 additions & 13 deletions chrome/browser/chromeos/drive/local_file_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
#include "base/basictypes.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/platform_file.h"
#include "net/base/completion_callback.h"
#include "net/base/file_stream.h"

namespace base {
class FilePath;
Expand All @@ -18,7 +18,7 @@ class SequencedTaskRunner;

namespace net {
class IOBuffer;
} // namespace
} // namespace net

namespace drive {
namespace util {
Expand Down Expand Up @@ -49,18 +49,14 @@ class LocalFileReader {
const net::CompletionCallback& callback);

private:
// The thin wrapper for the platform file to handle closing correctly.
class ScopedPlatformFile;
void DidOpen(const net::CompletionCallback& callback,
int64 offset,
int error);
void DidSeek(const net::CompletionCallback& callback,
int64 offset,
int64 error);

// Part of Open(). Called after the open() operation task running
// on blocking pool.
void OpenAfterBlockingPoolTask(
const net::CompletionCallback& callback,
ScopedPlatformFile* result_platform_file,
int open_result);

scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_;
base::PlatformFile platform_file_;
net::FileStream file_stream_;

// Note: This should remain the last member so it'll be destroyed and
// invalidate the weak pointers before any other members are destroyed.
Expand Down
9 changes: 2 additions & 7 deletions net/base/file_stream_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -274,21 +274,16 @@ void FileStream::Context::CloseAndDelete() {
DCHECK(!async_in_progress_);

if (file_.IsValid()) {
bool posted = task_runner_.get()->PostTaskAndReply(
bool posted = task_runner_.get()->PostTask(
FROM_HERE,
base::Bind(base::IgnoreResult(&Context::CloseFileImpl),
base::Unretained(this)),
base::Bind(&Context::OnCloseCompleted, base::Unretained(this)));
base::Owned(this)));
DCHECK(posted);
} else {
delete this;
}
}

void FileStream::Context::OnCloseCompleted() {
delete this;
}

Int64CompletionCallback FileStream::Context::IntToInt64(
const CompletionCallback& callback) {
return base::Bind(&CallInt64ToInt, callback);
Expand Down
1 change: 0 additions & 1 deletion net/base/file_stream_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ class FileStream::Context {
OpenResult open_result);

void CloseAndDelete();
void OnCloseCompleted();

Int64CompletionCallback IntToInt64(const CompletionCallback& callback);

Expand Down

0 comments on commit b7617c7

Please sign in to comment.