Commit ce1b9342 authored by Marijn Kruisselbrink's avatar Marijn Kruisselbrink Committed by Commit Bot

Revert "Run ObfuscatedFileUtilMemoryDelegate entirely on TaskRunner."

This reverts commit 88d45f78.

Reason for revert: Unfortunately this seems to be causing frequent flaky crashes in ECKIncognitoEncryptedMediaTest.FileIO

https://ci.chromium.org/p/chromium/builders/ci/Linux%20Ozone%20Tester%20%28X11%29/16596

BrowserTestBase received signal: Segmentation fault. Backtrace:
#0 0x5556b3a3edb9 base::debug::CollectStackTrace()
#1 0x5556b39b4743 base::debug::StackTrace::StackTrace()
#2 0x5556b3f82016 content::(anonymous namespace)::DumpStackTraceSignalHandler()
#3 0x7f4df599a4c0 (/lib/x86_64-linux-gnu/libc-2.23.so+0x354bf)
#4 0x7f4df5ab3095 (/lib/x86_64-linux-gnu/libc-2.23.so+0x14e094)
#5 0x5556b130ff73 _ZNSt3__16vectorIhNS_9allocatorIhEEE6insertIPKcEENS_9enable_ifIXaasr21__is_forward_iteratorIT_EE5valuesr16is_constructibleIhNS_15iterator_traitsIS8_E9referenceEEE5valueENS_11__wrap_iterIPhEEE4typeENSC_IPKhEES8_S8_
#6 0x5556b5ed32e7 storage::ObfuscatedFileUtilMemoryDelegate::WriteFile()
#7 0x5556b5ee0cdf base::internal::Invoker<>::RunOnce()
...


Original change's description:
> Run ObfuscatedFileUtilMemoryDelegate entirely on TaskRunner.
> 
> MemoryFileStreamWriter called some ObfuscatedFileUtilMemoryDelegate
> functions through IO thread while other functions in OFUMD are called
> on a threadpool sequence. This could result in races in updating
> directory structure.
> 
> To fix the issue, MemoryFileStreamWriter and MemoryFileStreamReader are
> updated to call all OFUMD on the default task runner of the file system
> context.
> 
> Bug: 1100136
> Change-Id: I59146ca690eee810c52f807bd1fb4ef2b1f2c929
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2308721
> Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
> Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#802584}

TBR=mek@chromium.org,rhalavati@chromium.org

Change-Id: Ib856bac5a978b8da33e74f8646ddd5be2a285865
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1100136
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382051Reviewed-by: default avatarMarijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802687}
parent 4ce9b063
...@@ -60,7 +60,6 @@ class FileStreamReader { ...@@ -60,7 +60,6 @@ class FileStreamReader {
// ERR_UPLOAD_FILE_CHANGED error. // ERR_UPLOAD_FILE_CHANGED error.
COMPONENT_EXPORT(STORAGE_BROWSER) COMPONENT_EXPORT(STORAGE_BROWSER)
static std::unique_ptr<FileStreamReader> CreateForMemoryFile( static std::unique_ptr<FileStreamReader> CreateForMemoryFile(
scoped_refptr<base::TaskRunner> task_runner,
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util, base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util,
const base::FilePath& file_path, const base::FilePath& file_path,
int64_t initial_offset, int64_t initial_offset,
......
...@@ -40,14 +40,6 @@ void ReadFromReader(FileStreamReader* reader, ...@@ -40,14 +40,6 @@ void ReadFromReader(FileStreamReader* reader,
} }
} }
int64_t GetLengthFromReader(FileStreamReader* reader) {
EXPECT_NE(nullptr, reader);
net::TestInt64CompletionCallback callback;
int rv = reader->GetLength(callback.callback());
return callback.GetResult(rv);
}
int WriteStringToWriter(FileStreamWriter* writer, const std::string& data) { int WriteStringToWriter(FileStreamWriter* writer, const std::string& data) {
scoped_refptr<net::StringIOBuffer> buffer = scoped_refptr<net::StringIOBuffer> buffer =
base::MakeRefCounted<net::StringIOBuffer>(data); base::MakeRefCounted<net::StringIOBuffer>(data);
......
...@@ -20,12 +20,8 @@ void ReadFromReader(FileStreamReader* reader, ...@@ -20,12 +20,8 @@ void ReadFromReader(FileStreamReader* reader,
size_t size, size_t size,
int* result); int* result);
// Returns the length of the file if it could be successfully retrieved, // Writes |data| to |writer|, an intialized FileStreamWriter. Returns net::OK if
// otherwise a net error. // successful, otherwise a net error.
int64_t GetLengthFromReader(FileStreamReader* reader);
// Writes |data| to |writer|, an initialized FileStreamWriter. Returns net::OK
// if successful, otherwise a net error.
int WriteStringToWriter(FileStreamWriter* writer, const std::string& data); int WriteStringToWriter(FileStreamWriter* writer, const std::string& data);
} // namespace storage } // namespace storage
......
...@@ -48,9 +48,10 @@ class FileStreamWriter { ...@@ -48,9 +48,10 @@ class FileStreamWriter {
// Creates a writer for the existing memory file in the path |file_path| // Creates a writer for the existing memory file in the path |file_path|
// starting from |initial_offset|. // starting from |initial_offset|.
// TODO(mek): Remove or use |open_or_create| field here, as it is not
// currently used. https://crbug.com/1041048
COMPONENT_EXPORT(STORAGE_BROWSER) COMPONENT_EXPORT(STORAGE_BROWSER)
static std::unique_ptr<FileStreamWriter> CreateForMemoryFile( static std::unique_ptr<FileStreamWriter> CreateForMemoryFile(
scoped_refptr<base::TaskRunner> task_runner,
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util, base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util,
const base::FilePath& file_path, const base::FilePath& file_path,
int64_t initial_offset); int64_t initial_offset);
......
...@@ -112,7 +112,6 @@ void FileSystemFileStreamReader::DidCreateSnapshot( ...@@ -112,7 +112,6 @@ void FileSystemFileStreamReader::DidCreateSnapshot(
file_system_context_->sandbox_delegate()->memory_file_util_delegate(); file_system_context_->sandbox_delegate()->memory_file_util_delegate();
} }
file_reader_ = FileStreamReader::CreateForMemoryFile( file_reader_ = FileStreamReader::CreateForMemoryFile(
file_system_context_->default_file_task_runner(),
memory_file_util_delegate, platform_path, initial_offset_, memory_file_util_delegate, platform_path, initial_offset_,
expected_modification_time_); expected_modification_time_);
} else { } else {
......
...@@ -8,112 +8,68 @@ ...@@ -8,112 +8,68 @@
#include <utility> #include <utility>
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/task_runner_util.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
namespace storage { namespace storage {
std::unique_ptr<FileStreamReader> FileStreamReader::CreateForMemoryFile( std::unique_ptr<FileStreamReader> FileStreamReader::CreateForMemoryFile(
scoped_refptr<base::TaskRunner> task_runner,
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util, base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util,
const base::FilePath& file_path, const base::FilePath& file_path,
int64_t initial_offset, int64_t initial_offset,
const base::Time& expected_modification_time) { const base::Time& expected_modification_time) {
return base::WrapUnique(new MemoryFileStreamReader( return base::WrapUnique(
std::move(task_runner), std::move(memory_file_util), file_path, new MemoryFileStreamReader(std::move(memory_file_util), file_path,
initial_offset, expected_modification_time)); initial_offset, expected_modification_time));
} }
MemoryFileStreamReader::MemoryFileStreamReader( MemoryFileStreamReader::MemoryFileStreamReader(
scoped_refptr<base::TaskRunner> task_runner,
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util, base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util,
const base::FilePath& file_path, const base::FilePath& file_path,
int64_t initial_offset, int64_t initial_offset,
const base::Time& expected_modification_time) const base::Time& expected_modification_time)
: memory_file_util_(std::move(memory_file_util)), : memory_file_util_(std::move(memory_file_util)),
task_runner_(std::move(task_runner)),
file_path_(file_path), file_path_(file_path),
expected_modification_time_(expected_modification_time), expected_modification_time_(expected_modification_time),
offset_(initial_offset) { offset_(initial_offset) {
DCHECK(memory_file_util_.MaybeValid()); DCHECK(memory_file_util_);
} }
MemoryFileStreamReader::~MemoryFileStreamReader() = default; MemoryFileStreamReader::~MemoryFileStreamReader() = default;
int MemoryFileStreamReader::Read(net::IOBuffer* buf, int MemoryFileStreamReader::Read(net::IOBuffer* buf,
int buf_len, int buf_len,
net::CompletionOnceCallback callback) { net::CompletionOnceCallback /*callback*/) {
task_runner_->PostTaskAndReplyWithResult( base::File::Info file_info;
FROM_HERE, if (memory_file_util_->GetFileInfo(file_path_, &file_info) !=
base::BindOnce( base::File::FILE_OK) {
[](base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> util, return net::ERR_FILE_NOT_FOUND;
const base::FilePath& path, base::Time expected_modification_time, }
int64_t offset, net::IOBuffer* buf, int buf_len) -> int {
if (!util) if (!FileStreamReader::VerifySnapshotTime(expected_modification_time_,
return net::ERR_FILE_NOT_FOUND; file_info)) {
base::File::Info file_info; return net::ERR_UPLOAD_FILE_CHANGED;
if (util->GetFileInfo(path, &file_info) != base::File::FILE_OK) }
return net::ERR_FILE_NOT_FOUND;
int result = memory_file_util_->ReadFile(file_path_, offset_, buf, buf_len);
if (!FileStreamReader::VerifySnapshotTime(
expected_modification_time, file_info)) {
return net::ERR_UPLOAD_FILE_CHANGED;
}
return util->ReadFile(path, offset, buf, buf_len);
},
memory_file_util_, file_path_, expected_modification_time_, offset_,
buf, buf_len),
base::BindOnce(&MemoryFileStreamReader::OnReadCompleted,
weak_factory_.GetWeakPtr(), std::move(callback)));
return net::ERR_IO_PENDING;
}
void MemoryFileStreamReader::OnReadCompleted(
net::CompletionOnceCallback callback,
int result) {
if (result > 0) if (result > 0)
offset_ += result; offset_ += result;
return result;
std::move(callback).Run(result);
} }
int64_t MemoryFileStreamReader::GetLength( int64_t MemoryFileStreamReader::GetLength(
net::Int64CompletionOnceCallback callback) { net::Int64CompletionOnceCallback /*callback*/) {
task_runner_->PostTaskAndReplyWithResult( base::File::Info file_info;
FROM_HERE, if (memory_file_util_->GetFileInfo(file_path_, &file_info) !=
base::BindOnce( base::File::FILE_OK) {
[](base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> util, return net::ERR_FILE_NOT_FOUND;
const base::FilePath& path, }
base::Time expected_modification_time) -> int64_t {
if (!util) if (!FileStreamReader::VerifySnapshotTime(expected_modification_time_,
return net::ERR_FILE_NOT_FOUND; file_info)) {
base::File::Info file_info; return net::ERR_UPLOAD_FILE_CHANGED;
if (util->GetFileInfo(path, &file_info) != base::File::FILE_OK) { }
return net::ERR_FILE_NOT_FOUND;
} return file_info.size;
if (!FileStreamReader::VerifySnapshotTime(
expected_modification_time, file_info)) {
return net::ERR_UPLOAD_FILE_CHANGED;
}
return file_info.size;
},
memory_file_util_, file_path_, expected_modification_time_),
// |callback| is not directly used to make sure that it is not called if
// stream is deleted while this function is in flight.
base::BindOnce(&MemoryFileStreamReader::OnGetLengthCompleted,
weak_factory_.GetWeakPtr(), std::move(callback)));
return net::ERR_IO_PENDING;
}
void MemoryFileStreamReader::OnGetLengthCompleted(
net::Int64CompletionOnceCallback callback,
int64_t result) {
std::move(callback).Run(result);
} }
} // namespace storage } // namespace storage
...@@ -32,25 +32,17 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) MemoryFileStreamReader ...@@ -32,25 +32,17 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) MemoryFileStreamReader
friend class FileStreamReader; friend class FileStreamReader;
MemoryFileStreamReader( MemoryFileStreamReader(
scoped_refptr<base::TaskRunner> task_runner,
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util, base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util,
const base::FilePath& file_path, const base::FilePath& file_path,
int64_t initial_offset, int64_t initial_offset,
const base::Time& expected_modification_time); const base::Time& expected_modification_time);
void OnReadCompleted(net::CompletionOnceCallback callback, int result);
void OnGetLengthCompleted(net::Int64CompletionOnceCallback callback,
int64_t result);
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util_; base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util_;
const scoped_refptr<base::TaskRunner> task_runner_;
const base::FilePath file_path_; const base::FilePath file_path_;
const base::Time expected_modification_time_; const base::Time expected_modification_time_;
int64_t offset_; int64_t offset_;
base::WeakPtrFactory<MemoryFileStreamReader> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(MemoryFileStreamReader); DISALLOW_COPY_AND_ASSIGN(MemoryFileStreamReader);
}; };
......
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/test/task_environment.h"
#include "net/base/io_buffer.h" #include "net/base/io_buffer.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "storage/browser/file_system/file_stream_reader.h" #include "storage/browser/file_system/file_stream_reader.h"
...@@ -63,9 +62,9 @@ class MemoryFileStreamReaderTest : public testing::Test { ...@@ -63,9 +62,9 @@ class MemoryFileStreamReaderTest : public testing::Test {
const base::FilePath& path, const base::FilePath& path,
int64_t initial_offset, int64_t initial_offset,
const base::Time& expected_modification_time) { const base::Time& expected_modification_time) {
return FileStreamReader::CreateForMemoryFile( return FileStreamReader::CreateForMemoryFile(file_util_->GetWeakPtr(), path,
base::ThreadTaskRunnerHandle::Get(), file_util_->GetWeakPtr(), path, initial_offset,
initial_offset, expected_modification_time); expected_modification_time);
} }
void TouchTestFile(base::TimeDelta delta) { void TouchTestFile(base::TimeDelta delta) {
...@@ -84,7 +83,6 @@ class MemoryFileStreamReaderTest : public testing::Test { ...@@ -84,7 +83,6 @@ class MemoryFileStreamReaderTest : public testing::Test {
} }
private: private:
base::test::TaskEnvironment task_environment_;
base::ScopedTempDir file_system_directory_; base::ScopedTempDir file_system_directory_;
std::unique_ptr<ObfuscatedFileUtilMemoryDelegate> file_util_; std::unique_ptr<ObfuscatedFileUtilMemoryDelegate> file_util_;
base::Time test_file_modification_time_; base::Time test_file_modification_time_;
...@@ -115,14 +113,14 @@ TEST_F(MemoryFileStreamReaderTest, Empty) { ...@@ -115,14 +113,14 @@ TEST_F(MemoryFileStreamReaderTest, Empty) {
ASSERT_EQ(net::OK, result); ASSERT_EQ(net::OK, result);
ASSERT_EQ(0U, data.size()); ASSERT_EQ(0U, data.size());
int64_t length_result = GetLengthFromReader(reader.get()); int64_t length_result = reader->GetLength(base::DoNothing());
ASSERT_EQ(0, length_result); ASSERT_EQ(0, length_result);
} }
TEST_F(MemoryFileStreamReaderTest, GetLengthNormal) { TEST_F(MemoryFileStreamReaderTest, GetLengthNormal) {
std::unique_ptr<FileStreamReader> reader( std::unique_ptr<FileStreamReader> reader(
CreateFileReader(test_path(), 0, test_file_modification_time())); CreateFileReader(test_path(), 0, test_file_modification_time()));
int64_t result = GetLengthFromReader(reader.get()); int64_t result = reader->GetLength(base::DoNothing());
ASSERT_EQ(kTestDataSize, result); ASSERT_EQ(kTestDataSize, result);
} }
...@@ -133,7 +131,7 @@ TEST_F(MemoryFileStreamReaderTest, GetLengthAfterModified) { ...@@ -133,7 +131,7 @@ TEST_F(MemoryFileStreamReaderTest, GetLengthAfterModified) {
std::unique_ptr<FileStreamReader> reader( std::unique_ptr<FileStreamReader> reader(
CreateFileReader(test_path(), 0, test_file_modification_time())); CreateFileReader(test_path(), 0, test_file_modification_time()));
int64_t result = GetLengthFromReader(reader.get()); int64_t result = reader->GetLength(base::DoNothing());
ASSERT_EQ(net::ERR_UPLOAD_FILE_CHANGED, result); ASSERT_EQ(net::ERR_UPLOAD_FILE_CHANGED, result);
} }
...@@ -144,14 +142,14 @@ TEST_F(MemoryFileStreamReaderTest, GetLengthAfterModifiedWithNoExpectedTime) { ...@@ -144,14 +142,14 @@ TEST_F(MemoryFileStreamReaderTest, GetLengthAfterModifiedWithNoExpectedTime) {
std::unique_ptr<FileStreamReader> reader( std::unique_ptr<FileStreamReader> reader(
CreateFileReader(test_path(), 0, base::Time())); CreateFileReader(test_path(), 0, base::Time()));
int64_t result = GetLengthFromReader(reader.get()); int64_t result = reader->GetLength(base::DoNothing());
ASSERT_EQ(kTestDataSize, result); ASSERT_EQ(kTestDataSize, result);
} }
TEST_F(MemoryFileStreamReaderTest, GetLengthWithOffset) { TEST_F(MemoryFileStreamReaderTest, GetLengthWithOffset) {
std::unique_ptr<FileStreamReader> reader( std::unique_ptr<FileStreamReader> reader(
CreateFileReader(test_path(), 3, base::Time())); CreateFileReader(test_path(), 3, base::Time()));
int64_t result = GetLengthFromReader(reader.get()); int64_t result = reader->GetLength(base::DoNothing());
// Initial offset does not affect the result of GetLength. // Initial offset does not affect the result of GetLength.
ASSERT_EQ(kTestDataSize, result); ASSERT_EQ(kTestDataSize, result);
} }
......
...@@ -8,66 +8,43 @@ ...@@ -8,66 +8,43 @@
#include <utility> #include <utility>
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/task_runner_util.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
namespace storage { namespace storage {
std::unique_ptr<FileStreamWriter> FileStreamWriter::CreateForMemoryFile( std::unique_ptr<FileStreamWriter> FileStreamWriter::CreateForMemoryFile(
scoped_refptr<base::TaskRunner> task_runner,
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util, base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util,
const base::FilePath& file_path, const base::FilePath& file_path,
int64_t initial_offset) { int64_t initial_offset) {
return base::WrapUnique(new MemoryFileStreamWriter( return base::WrapUnique(new MemoryFileStreamWriter(
std::move(task_runner), std::move(memory_file_util), file_path, std::move(memory_file_util), file_path, initial_offset));
initial_offset));
} }
MemoryFileStreamWriter::MemoryFileStreamWriter( MemoryFileStreamWriter::MemoryFileStreamWriter(
scoped_refptr<base::TaskRunner> task_runner,
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util, base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util,
const base::FilePath& file_path, const base::FilePath& file_path,
int64_t initial_offset) int64_t initial_offset)
: memory_file_util_(std::move(memory_file_util)), : memory_file_util_(std::move(memory_file_util)),
task_runner_(std::move(task_runner)),
file_path_(file_path), file_path_(file_path),
offset_(initial_offset) { offset_(initial_offset) {
DCHECK(memory_file_util_.MaybeValid()); DCHECK(memory_file_util_);
} }
MemoryFileStreamWriter::~MemoryFileStreamWriter() = default; MemoryFileStreamWriter::~MemoryFileStreamWriter() = default;
int MemoryFileStreamWriter::Write(net::IOBuffer* buf, int MemoryFileStreamWriter::Write(net::IOBuffer* buf,
int buf_len, int buf_len,
net::CompletionOnceCallback callback) { net::CompletionOnceCallback /*callback*/) {
task_runner_->PostTaskAndReplyWithResult( base::File::Info file_info;
FROM_HERE, if (memory_file_util_->GetFileInfo(file_path_, &file_info) !=
base::BindOnce( base::File::FILE_OK) {
[](base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> util, return net::ERR_FILE_NOT_FOUND;
const base::FilePath& path, int64_t offset, net::IOBuffer* buf, }
int buf_len) -> int {
if (!util) int result = memory_file_util_->WriteFile(file_path_, offset_, buf, buf_len);
return net::ERR_FILE_NOT_FOUND;
base::File::Info file_info;
if (util->GetFileInfo(path, &file_info) != base::File::FILE_OK)
return net::ERR_FILE_NOT_FOUND;
return util->WriteFile(path, offset, buf, buf_len);
},
memory_file_util_, file_path_, offset_, buf, buf_len),
base::BindOnce(&MemoryFileStreamWriter::OnWriteCompleted,
weak_factory_.GetWeakPtr(), std::move(callback)));
return net::ERR_IO_PENDING;
}
void MemoryFileStreamWriter::OnWriteCompleted(
net::CompletionOnceCallback callback,
int result) {
if (result > 0) if (result > 0)
offset_ += result; offset_ += result;
return result;
std::move(callback).Run(result);
} }
int MemoryFileStreamWriter::Cancel(net::CompletionOnceCallback /*callback*/) { int MemoryFileStreamWriter::Cancel(net::CompletionOnceCallback /*callback*/) {
......
...@@ -30,21 +30,15 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) MemoryFileStreamWriter ...@@ -30,21 +30,15 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) MemoryFileStreamWriter
private: private:
friend class FileStreamWriter; friend class FileStreamWriter;
MemoryFileStreamWriter( MemoryFileStreamWriter(
scoped_refptr<base::TaskRunner> task_runner,
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util, base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util,
const base::FilePath& file_path, const base::FilePath& file_path,
int64_t initial_offset); int64_t initial_offset);
void OnWriteCompleted(net::CompletionOnceCallback callback, int result);
base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util_; base::WeakPtr<ObfuscatedFileUtilMemoryDelegate> memory_file_util_;
const scoped_refptr<base::TaskRunner> task_runner_;
const base::FilePath file_path_; const base::FilePath file_path_;
int64_t offset_; int64_t offset_;
base::WeakPtrFactory<MemoryFileStreamWriter> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(MemoryFileStreamWriter); DISALLOW_COPY_AND_ASSIGN(MemoryFileStreamWriter);
}; };
......
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/test/task_environment.h"
#include "net/base/io_buffer.h" #include "net/base/io_buffer.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "storage/browser/file_system/file_stream_test_utils.h" #include "storage/browser/file_system/file_stream_test_utils.h"
...@@ -60,13 +59,11 @@ class MemoryFileStreamWriterTest : public testing::Test { ...@@ -60,13 +59,11 @@ class MemoryFileStreamWriterTest : public testing::Test {
std::unique_ptr<FileStreamWriter> CreateWriter(const base::FilePath& path, std::unique_ptr<FileStreamWriter> CreateWriter(const base::FilePath& path,
int64_t offset) { int64_t offset) {
return FileStreamWriter::CreateForMemoryFile( return FileStreamWriter::CreateForMemoryFile(file_util_->GetWeakPtr(), path,
base::ThreadTaskRunnerHandle::Get(), file_util_->GetWeakPtr(), path, offset);
offset);
} }
private: private:
base::test::TaskEnvironment task_environment_;
base::ScopedTempDir file_system_directory_; base::ScopedTempDir file_system_directory_;
std::unique_ptr<ObfuscatedFileUtilMemoryDelegate> file_util_; std::unique_ptr<ObfuscatedFileUtilMemoryDelegate> file_util_;
}; };
......
...@@ -56,17 +56,13 @@ struct ObfuscatedFileUtilMemoryDelegate::DecomposedPath { ...@@ -56,17 +56,13 @@ struct ObfuscatedFileUtilMemoryDelegate::DecomposedPath {
ObfuscatedFileUtilMemoryDelegate::ObfuscatedFileUtilMemoryDelegate( ObfuscatedFileUtilMemoryDelegate::ObfuscatedFileUtilMemoryDelegate(
const base::FilePath& file_system_directory) const base::FilePath& file_system_directory)
: root_(std::make_unique<Entry>(Entry::kDirectory)) { : root_(std::make_unique<Entry>(Entry::kDirectory)) {
DETACH_FROM_SEQUENCE(sequence_checker_);
file_system_directory.GetComponents(&root_path_components_); file_system_directory.GetComponents(&root_path_components_);
} }
ObfuscatedFileUtilMemoryDelegate::~ObfuscatedFileUtilMemoryDelegate() { ObfuscatedFileUtilMemoryDelegate::~ObfuscatedFileUtilMemoryDelegate() = default;
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
}
base::Optional<ObfuscatedFileUtilMemoryDelegate::DecomposedPath> base::Optional<ObfuscatedFileUtilMemoryDelegate::DecomposedPath>
ObfuscatedFileUtilMemoryDelegate::ParsePath(const base::FilePath& path) { ObfuscatedFileUtilMemoryDelegate::ParsePath(const base::FilePath& path) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DecomposedPath dp; DecomposedPath dp;
path.GetComponents(&dp.components); path.GetComponents(&dp.components);
...@@ -122,7 +118,6 @@ ObfuscatedFileUtilMemoryDelegate::ParsePath(const base::FilePath& path) { ...@@ -122,7 +118,6 @@ ObfuscatedFileUtilMemoryDelegate::ParsePath(const base::FilePath& path) {
bool ObfuscatedFileUtilMemoryDelegate::DirectoryExists( bool ObfuscatedFileUtilMemoryDelegate::DirectoryExists(
const base::FilePath& path) { const base::FilePath& path) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
return dp && dp->entry && dp->entry->type == Entry::kDirectory; return dp && dp->entry && dp->entry->type == Entry::kDirectory;
} }
...@@ -131,7 +126,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CreateDirectory( ...@@ -131,7 +126,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CreateDirectory(
const base::FilePath& path, const base::FilePath& path,
bool exclusive, bool exclusive,
bool recursive) { bool recursive) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp) if (!dp)
return base::File::FILE_ERROR_NOT_FOUND; return base::File::FILE_ERROR_NOT_FOUND;
...@@ -175,7 +169,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CreateDirectory( ...@@ -175,7 +169,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CreateDirectory(
bool ObfuscatedFileUtilMemoryDelegate::DeleteFileOrDirectory( bool ObfuscatedFileUtilMemoryDelegate::DeleteFileOrDirectory(
const base::FilePath& path, const base::FilePath& path,
bool recursive) { bool recursive) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp) if (!dp)
return false; return false;
...@@ -192,13 +185,11 @@ bool ObfuscatedFileUtilMemoryDelegate::DeleteFileOrDirectory( ...@@ -192,13 +185,11 @@ bool ObfuscatedFileUtilMemoryDelegate::DeleteFileOrDirectory(
} }
bool ObfuscatedFileUtilMemoryDelegate::IsLink(const base::FilePath& file_path) { bool ObfuscatedFileUtilMemoryDelegate::IsLink(const base::FilePath& file_path) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// In-memory file system does not support links. // In-memory file system does not support links.
return false; return false;
} }
bool ObfuscatedFileUtilMemoryDelegate::PathExists(const base::FilePath& path) { bool ObfuscatedFileUtilMemoryDelegate::PathExists(const base::FilePath& path) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
return dp && dp->entry; return dp && dp->entry;
} }
...@@ -206,7 +197,6 @@ bool ObfuscatedFileUtilMemoryDelegate::PathExists(const base::FilePath& path) { ...@@ -206,7 +197,6 @@ bool ObfuscatedFileUtilMemoryDelegate::PathExists(const base::FilePath& path) {
base::File ObfuscatedFileUtilMemoryDelegate::CreateOrOpen( base::File ObfuscatedFileUtilMemoryDelegate::CreateOrOpen(
const base::FilePath& path, const base::FilePath& path,
int file_flags) { int file_flags) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// TODO:(https://crbug.com/936722): Once the output of this function is // TODO:(https://crbug.com/936722): Once the output of this function is
// changed to base::File::Error, it can use CreateOrOpenInternal to perform // changed to base::File::Error, it can use CreateOrOpenInternal to perform
// the task and return the result. // the task and return the result.
...@@ -216,7 +206,6 @@ base::File ObfuscatedFileUtilMemoryDelegate::CreateOrOpen( ...@@ -216,7 +206,6 @@ base::File ObfuscatedFileUtilMemoryDelegate::CreateOrOpen(
void ObfuscatedFileUtilMemoryDelegate::CreateOrOpenInternal( void ObfuscatedFileUtilMemoryDelegate::CreateOrOpenInternal(
const DecomposedPath& dp, const DecomposedPath& dp,
int file_flags) { int file_flags) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!dp.entry) { if (!dp.entry) {
dp.parent->directory_content.emplace(dp.components.back(), Entry::kFile); dp.parent->directory_content.emplace(dp.components.back(), Entry::kFile);
return; return;
...@@ -232,7 +221,6 @@ void ObfuscatedFileUtilMemoryDelegate::CreateOrOpenInternal( ...@@ -232,7 +221,6 @@ void ObfuscatedFileUtilMemoryDelegate::CreateOrOpenInternal(
base::File::Error ObfuscatedFileUtilMemoryDelegate::DeleteFile( base::File::Error ObfuscatedFileUtilMemoryDelegate::DeleteFile(
const base::FilePath& path) { const base::FilePath& path) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp || !dp->entry) if (!dp || !dp->entry)
return base::File::FILE_ERROR_NOT_FOUND; return base::File::FILE_ERROR_NOT_FOUND;
...@@ -247,7 +235,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::DeleteFile( ...@@ -247,7 +235,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::DeleteFile(
base::File::Error ObfuscatedFileUtilMemoryDelegate::EnsureFileExists( base::File::Error ObfuscatedFileUtilMemoryDelegate::EnsureFileExists(
const base::FilePath& path, const base::FilePath& path,
bool* created) { bool* created) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
*created = false; *created = false;
if (!dp || !dp->parent) if (!dp || !dp->parent)
...@@ -266,7 +253,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::EnsureFileExists( ...@@ -266,7 +253,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::EnsureFileExists(
base::File::Error ObfuscatedFileUtilMemoryDelegate::GetFileInfo( base::File::Error ObfuscatedFileUtilMemoryDelegate::GetFileInfo(
const base::FilePath& path, const base::FilePath& path,
base::File::Info* file_info) { base::File::Info* file_info) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp || !dp->entry) if (!dp || !dp->entry)
return base::File::FILE_ERROR_NOT_FOUND; return base::File::FILE_ERROR_NOT_FOUND;
...@@ -286,7 +272,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::Touch( ...@@ -286,7 +272,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::Touch(
const base::FilePath& path, const base::FilePath& path,
const base::Time& last_access_time, const base::Time& last_access_time,
const base::Time& last_modified_time) { const base::Time& last_modified_time) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp || !dp->entry) if (!dp || !dp->entry)
return base::File::FILE_ERROR_FAILED; return base::File::FILE_ERROR_FAILED;
...@@ -300,7 +285,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::Touch( ...@@ -300,7 +285,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::Touch(
base::File::Error ObfuscatedFileUtilMemoryDelegate::Truncate( base::File::Error ObfuscatedFileUtilMemoryDelegate::Truncate(
const base::FilePath& path, const base::FilePath& path,
int64_t length) { int64_t length) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp || !dp->entry || dp->entry->type != Entry::kFile) if (!dp || !dp->entry || dp->entry->type != Entry::kFile)
return base::File::FILE_ERROR_NOT_FOUND; return base::File::FILE_ERROR_NOT_FOUND;
...@@ -313,7 +297,6 @@ NativeFileUtil::CopyOrMoveMode ...@@ -313,7 +297,6 @@ NativeFileUtil::CopyOrMoveMode
ObfuscatedFileUtilMemoryDelegate::CopyOrMoveModeForDestination( ObfuscatedFileUtilMemoryDelegate::CopyOrMoveModeForDestination(
const FileSystemURL& /*dest_url*/, const FileSystemURL& /*dest_url*/,
bool copy) { bool copy) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return copy ? NativeFileUtil::CopyOrMoveMode::COPY_SYNC return copy ? NativeFileUtil::CopyOrMoveMode::COPY_SYNC
: NativeFileUtil::CopyOrMoveMode::MOVE; : NativeFileUtil::CopyOrMoveMode::MOVE;
} }
...@@ -323,7 +306,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CopyOrMoveFile( ...@@ -323,7 +306,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CopyOrMoveFile(
const base::FilePath& dest_path, const base::FilePath& dest_path,
FileSystemOperation::CopyOrMoveOption option, FileSystemOperation::CopyOrMoveOption option,
NativeFileUtil::CopyOrMoveMode mode) { NativeFileUtil::CopyOrMoveMode mode) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> src_dp = ParsePath(src_path); base::Optional<DecomposedPath> src_dp = ParsePath(src_path);
base::Optional<DecomposedPath> dest_dp = ParsePath(dest_path); base::Optional<DecomposedPath> dest_dp = ParsePath(dest_path);
...@@ -379,7 +361,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CopyOrMoveFile( ...@@ -379,7 +361,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CopyOrMoveFile(
bool ObfuscatedFileUtilMemoryDelegate::MoveDirectoryInternal( bool ObfuscatedFileUtilMemoryDelegate::MoveDirectoryInternal(
const DecomposedPath& src_dp, const DecomposedPath& src_dp,
const DecomposedPath& dest_dp) { const DecomposedPath& dest_dp) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(src_dp.entry->type == Entry::kDirectory); DCHECK(src_dp.entry->type == Entry::kDirectory);
if (!dest_dp.entry) { if (!dest_dp.entry) {
dest_dp.parent->directory_content.insert( dest_dp.parent->directory_content.insert(
...@@ -398,7 +379,6 @@ bool ObfuscatedFileUtilMemoryDelegate::CopyOrMoveFileInternal( ...@@ -398,7 +379,6 @@ bool ObfuscatedFileUtilMemoryDelegate::CopyOrMoveFileInternal(
const DecomposedPath& src_dp, const DecomposedPath& src_dp,
const DecomposedPath& dest_dp, const DecomposedPath& dest_dp,
bool move) { bool move) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(src_dp.entry->type == Entry::kFile); DCHECK(src_dp.entry->type == Entry::kFile);
if (dest_dp.entry) if (dest_dp.entry)
dest_dp.parent->directory_content.erase(dest_dp.components.back()); dest_dp.parent->directory_content.erase(dest_dp.components.back());
...@@ -424,7 +404,6 @@ bool ObfuscatedFileUtilMemoryDelegate::CopyOrMoveFileInternal( ...@@ -424,7 +404,6 @@ bool ObfuscatedFileUtilMemoryDelegate::CopyOrMoveFileInternal(
size_t ObfuscatedFileUtilMemoryDelegate::ComputeDirectorySize( size_t ObfuscatedFileUtilMemoryDelegate::ComputeDirectorySize(
const base::FilePath& path) { const base::FilePath& path) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp || !dp->entry || dp->entry->type != Entry::kDirectory) if (!dp || !dp->entry || dp->entry->type != Entry::kDirectory)
return 0; return 0;
...@@ -450,7 +429,6 @@ int ObfuscatedFileUtilMemoryDelegate::ReadFile(const base::FilePath& path, ...@@ -450,7 +429,6 @@ int ObfuscatedFileUtilMemoryDelegate::ReadFile(const base::FilePath& path,
int64_t offset, int64_t offset,
net::IOBuffer* buf, net::IOBuffer* buf,
int buf_len) { int buf_len) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp || dp->entry->type != Entry::kFile) if (!dp || dp->entry->type != Entry::kFile)
return net::ERR_FILE_NOT_FOUND; return net::ERR_FILE_NOT_FOUND;
...@@ -471,10 +449,9 @@ int ObfuscatedFileUtilMemoryDelegate::WriteFile(const base::FilePath& path, ...@@ -471,10 +449,9 @@ int ObfuscatedFileUtilMemoryDelegate::WriteFile(const base::FilePath& path,
int64_t offset, int64_t offset,
net::IOBuffer* buf, net::IOBuffer* buf,
int buf_len) { int buf_len) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dp = ParsePath(path); base::Optional<DecomposedPath> dp = ParsePath(path);
if (!dp || !dp->entry || dp->entry->type != Entry::kFile) if (!dp || dp->entry->type != Entry::kFile)
return net::ERR_FILE_NOT_FOUND; return net::ERR_FILE_NOT_FOUND;
size_t offset_u = static_cast<size_t>(offset); size_t offset_u = static_cast<size_t>(offset);
...@@ -502,7 +479,6 @@ int ObfuscatedFileUtilMemoryDelegate::WriteFile(const base::FilePath& path, ...@@ -502,7 +479,6 @@ int ObfuscatedFileUtilMemoryDelegate::WriteFile(const base::FilePath& path,
base::File::Error ObfuscatedFileUtilMemoryDelegate::CreateFileForTesting( base::File::Error ObfuscatedFileUtilMemoryDelegate::CreateFileForTesting(
const base::FilePath& path, const base::FilePath& path,
base::span<const char> content) { base::span<const char> content) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
bool created; bool created;
base::File::Error result = EnsureFileExists(path, &created); base::File::Error result = EnsureFileExists(path, &created);
if (result != base::File::FILE_OK) if (result != base::File::FILE_OK)
...@@ -522,7 +498,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CopyInForeignFile( ...@@ -522,7 +498,6 @@ base::File::Error ObfuscatedFileUtilMemoryDelegate::CopyInForeignFile(
const base::FilePath& dest_path, const base::FilePath& dest_path,
FileSystemOperation::CopyOrMoveOption /* option */, FileSystemOperation::CopyOrMoveOption /* option */,
NativeFileUtil::CopyOrMoveMode /* mode */) { NativeFileUtil::CopyOrMoveMode /* mode */) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::Optional<DecomposedPath> dest_dp = ParsePath(dest_path); base::Optional<DecomposedPath> dest_dp = ParsePath(dest_path);
if (!dest_dp || !dest_dp->parent) if (!dest_dp || !dest_dp->parent)
......
...@@ -126,8 +126,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) ObfuscatedFileUtilMemoryDelegate ...@@ -126,8 +126,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) ObfuscatedFileUtilMemoryDelegate
const DecomposedPath& dest_dp, const DecomposedPath& dest_dp,
bool move); bool move);
SEQUENCE_CHECKER(sequence_checker_);
// The root of the directory tree. // The root of the directory tree.
std::unique_ptr<Entry> root_; std::unique_ptr<Entry> root_;
......
...@@ -159,7 +159,6 @@ void SandboxFileStreamWriter::DidCreateSnapshotFile( ...@@ -159,7 +159,6 @@ void SandboxFileStreamWriter::DidCreateSnapshotFile(
file_system_context_->sandbox_delegate()->memory_file_util_delegate(); file_system_context_->sandbox_delegate()->memory_file_util_delegate();
} }
file_writer_ = FileStreamWriter::CreateForMemoryFile( file_writer_ = FileStreamWriter::CreateForMemoryFile(
file_system_context_->default_file_task_runner(),
memory_file_util_delegate, platform_path, initial_offset_); memory_file_util_delegate, platform_path, initial_offset_);
} else { } else {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment