diff options
author | rvargas@chromium.org <rvargas@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-03 22:14:15 +0000 |
---|---|---|
committer | rvargas@chromium.org <rvargas@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-03 22:14:15 +0000 |
commit | 58ba3eab5af91f112de6a26856f07cdd46cde35e (patch) | |
tree | b9fa5046900e2003926caee62454dc572ea9508d | |
parent | 4b50cf797e2a1b5026da2b03a6f3c6e6f8065d5d (diff) | |
download | chromium_src-58ba3eab5af91f112de6a26856f07cdd46cde35e.zip chromium_src-58ba3eab5af91f112de6a26856f07cdd46cde35e.tar.gz chromium_src-58ba3eab5af91f112de6a26856f07cdd46cde35e.tar.bz2 |
Convert base::MemoryMappedFile to use File instead of PlatformFile.
This also modifies consumers of MemoryMappedFile and fixes double handle
close on MediaFileChecker, media_file_checker_unittest and data_pack_unittests.
BUG=322664
TEST=unit tests
R=cpu@chromium.org, dalecurtis@chromium.org (media)
TBR (owners):
tony@chromium.org (resource)
jochen@chromium.org (chrome-content)
thakis@chromium.org (base)
Review URL: https://codereview.chromium.org/109273002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@242937 0039d316-1c4b-4281-b951-d872f2087c98
23 files changed, 170 insertions, 169 deletions
diff --git a/base/files/file.cc b/base/files/file.cc index 4902f15..508103a 100644 --- a/base/files/file.cc +++ b/base/files/file.cc @@ -31,14 +31,17 @@ File::File(const FilePath& name, uint32 flags) error_(FILE_OK), created_(false), async_(false) { - if (name.ReferencesParent()) { - error_ = FILE_ERROR_ACCESS_DENIED; - return; - } - CreateBaseFileUnsafe(name, flags); + Initialize(name, flags); } #endif +File::File(PlatformFile platform_file) + : file_(platform_file), + error_(FILE_OK), + created_(false), + async_(false) { +} + File::File(RValue other) : file_(other.object->TakePlatformFile()), error_(other.object->error()), @@ -61,4 +64,14 @@ File& File::operator=(RValue other) { return *this; } +#if !defined(OS_NACL) +void File::Initialize(const FilePath& name, uint32 flags) { + if (name.ReferencesParent()) { + error_ = FILE_ERROR_ACCESS_DENIED; + return; + } + InitializeUnsafe(name, flags); +} +#endif + } // namespace base diff --git a/base/files/file.h b/base/files/file.h index d1e0e8c..e87dd7b 100644 --- a/base/files/file.h +++ b/base/files/file.h @@ -157,9 +157,12 @@ class BASE_EXPORT File { // Move operator= for C++03 move emulation of this type. File& operator=(RValue other); + // Creates or opens the given file. + void Initialize(const FilePath& name, uint32 flags); + // Creates or opens the given file, allowing paths with traversal ('..') // components. Use only with extreme care. - void CreateBaseFileUnsafe(const FilePath& name, uint32 flags); + void InitializeUnsafe(const FilePath& name, uint32 flags); bool IsValid() const; @@ -216,10 +219,13 @@ class BASE_EXPORT File { // platforms. Returns the number of bytes written, or -1 on error. int WriteAtCurrentPosNoBestEffort(const char* data, int size); + // Returns the current size of this file, or a negative number on failure. + int64 GetLength(); + // Truncates the file to the given length. If |length| is greater than the // current size of the file, the file is extended with zeros. If the file // doesn't exist, |false| is returned. - bool Truncate(int64 length); + bool SetLength(int64 length); // Flushes the buffers. bool Flush(); diff --git a/base/files/file_posix.cc b/base/files/file_posix.cc index 9d97c33..e8b4d77 100644 --- a/base/files/file_posix.cc +++ b/base/files/file_posix.cc @@ -122,7 +122,7 @@ static File::Error CallFctnlFlock(PlatformFile file, bool do_lock) { // NaCl doesn't implement system calls to open files directly. #if !defined(OS_NACL) // TODO(erikkay): does it make sense to support FLAG_EXCLUSIVE_* here? -void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { +void File::InitializeUnsafe(const FilePath& name, uint32 flags) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(!IsValid()); DCHECK(!(flags & FLAG_ASYNC)); @@ -341,7 +341,17 @@ int File::WriteAtCurrentPosNoBestEffort(const char* data, int size) { return HANDLE_EINTR(write(file_, data, size)); } -bool File::Truncate(int64 length) { +int64 File::GetLength() { + DCHECK(IsValid()); + + stat_wrapper_t file_info; + if (CallFstat(file_, &file_info)) + return false; + + return file_info.st_size; +} + +bool File::SetLength(int64 length) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); return !CallFtruncate(file_, length); diff --git a/base/files/file_unittest.cc b/base/files/file_unittest.cc index b2e855d..16eece1 100644 --- a/base/files/file_unittest.cc +++ b/base/files/file_unittest.cc @@ -44,6 +44,19 @@ TEST(File, Create) { } { + // Open an existing file through Initialize + File file; + file.Initialize(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ); + EXPECT_TRUE(file.IsValid()); + EXPECT_FALSE(file.created()); + EXPECT_EQ(base::File::FILE_OK, file.error()); + + // This time verify closing the file. + file.Close(); + EXPECT_FALSE(file.IsValid()); + } + + { // Create a file that exists. File file(file_path, base::File::FLAG_CREATE | base::File::FLAG_READ); EXPECT_FALSE(file.IsValid()); @@ -221,7 +234,7 @@ TEST(File, Append) { } -TEST(File, Truncate) { +TEST(File, Length) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); FilePath file_path = temp_dir.path().AppendASCII("truncate_file"); @@ -229,6 +242,7 @@ TEST(File, Truncate) { base::File::FLAG_CREATE | base::File::FLAG_READ | base::File::FLAG_WRITE); ASSERT_TRUE(file.IsValid()); + EXPECT_EQ(0, file.GetLength()); // Write "test" to the file. char data_to_write[] = "test"; @@ -239,7 +253,8 @@ TEST(File, Truncate) { // Extend the file. const int kExtendedFileLength = 10; int64 file_size = 0; - EXPECT_TRUE(file.Truncate(kExtendedFileLength)); + EXPECT_TRUE(file.SetLength(kExtendedFileLength)); + EXPECT_EQ(kExtendedFileLength, file.GetLength()); EXPECT_TRUE(GetFileSize(file_path, &file_size)); EXPECT_EQ(kExtendedFileLength, file_size); @@ -254,7 +269,8 @@ TEST(File, Truncate) { // Truncate the file. const int kTruncatedFileLength = 2; - EXPECT_TRUE(file.Truncate(kTruncatedFileLength)); + EXPECT_TRUE(file.SetLength(kTruncatedFileLength)); + EXPECT_EQ(kTruncatedFileLength, file.GetLength()); EXPECT_TRUE(GetFileSize(file_path, &file_size)); EXPECT_EQ(kTruncatedFileLength, file_size); diff --git a/base/files/file_win.cc b/base/files/file_win.cc index 94f4d7f..c7ce909 100644 --- a/base/files/file_win.cc +++ b/base/files/file_win.cc @@ -13,7 +13,7 @@ namespace base { -void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { +void File::InitializeUnsafe(const FilePath& name, uint32 flags) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(!IsValid()); @@ -197,7 +197,17 @@ int File::WriteAtCurrentPosNoBestEffort(const char* data, int size) { return WriteAtCurrentPos(data, size); } -bool File::Truncate(int64 length) { +int64 File::GetLength() { + base::ThreadRestrictions::AssertIOAllowed(); + DCHECK(IsValid()); + LARGE_INTEGER size; + if (!::GetFileSizeEx(file_.Get(), &size)) + return -1; + + return static_cast<int64>(size.QuadPart); +} + +bool File::SetLength(int64 length) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); @@ -218,6 +228,9 @@ bool File::Truncate(int64 length) { // Set the new file length and move the file pointer to its old position. // This is consistent with ftruncate()'s behavior, even when the file // pointer points to a location beyond the end of the file. + // TODO(rvargas): Emulating ftruncate details seem suspicious and it is not + // promised by the interface (nor was promised by PlatformFile). See if this + // implementation detail can be removed. return ((::SetEndOfFile(file_) != 0) && (::SetFilePointerEx(file_, file_pointer, NULL, FILE_BEGIN) != 0)); } diff --git a/base/files/memory_mapped_file.cc b/base/files/memory_mapped_file.cc index a48ec0c..ace4e11 100644 --- a/base/files/memory_mapped_file.cc +++ b/base/files/memory_mapped_file.cc @@ -17,7 +17,14 @@ bool MemoryMappedFile::Initialize(const FilePath& file_name) { if (IsValid()) return false; - if (!MapFileToMemory(file_name)) { + file_.Initialize(file_name, File::FLAG_OPEN | File::FLAG_READ); + + if (!file_.IsValid()) { + DLOG(ERROR) << "Couldn't open " << file_name.AsUTF8Unsafe(); + return false; + } + + if (!MapFileToMemory()) { CloseHandles(); return false; } @@ -25,13 +32,13 @@ bool MemoryMappedFile::Initialize(const FilePath& file_name) { return true; } -bool MemoryMappedFile::Initialize(PlatformFile file) { +bool MemoryMappedFile::Initialize(File file) { if (IsValid()) return false; - file_ = file; + file_ = file.Pass(); - if (!MapFileToMemoryInternal()) { + if (!MapFileToMemory()) { CloseHandles(); return false; } @@ -43,16 +50,4 @@ bool MemoryMappedFile::IsValid() const { return data_ != NULL; } -bool MemoryMappedFile::MapFileToMemory(const FilePath& file_name) { - file_ = CreatePlatformFile(file_name, PLATFORM_FILE_OPEN | PLATFORM_FILE_READ, - NULL, NULL); - - if (file_ == kInvalidPlatformFileValue) { - DLOG(ERROR) << "Couldn't open " << file_name.AsUTF8Unsafe(); - return false; - } - - return MapFileToMemoryInternal(); -} - } // namespace base diff --git a/base/files/memory_mapped_file.h b/base/files/memory_mapped_file.h index 6df1bad..b02d8cf 100644 --- a/base/files/memory_mapped_file.h +++ b/base/files/memory_mapped_file.h @@ -7,7 +7,7 @@ #include "base/base_export.h" #include "base/basictypes.h" -#include "base/platform_file.h" +#include "base/files/file.h" #include "build/build_config.h" #if defined(OS_WIN) @@ -30,9 +30,10 @@ class BASE_EXPORT MemoryMappedFile { // the file does not exist, or the memory mapping fails, it will return false. // Later we may want to allow the user to specify access. bool Initialize(const FilePath& file_name); - // As above, but works with an already-opened file. MemoryMappedFile will take - // ownership of |file| and close it when done. - bool Initialize(PlatformFile file); + + // As above, but works with an already-opened file. MemoryMappedFile takes + // ownership of |file| and closes it when done. + bool Initialize(File file); #if defined(OS_WIN) // Opens an existing file and maps it as an image section. Please refer to @@ -47,27 +48,22 @@ class BASE_EXPORT MemoryMappedFile { bool IsValid() const; private: - // Open the given file and pass it to MapFileToMemoryInternal(). - bool MapFileToMemory(const FilePath& file_name); - // Map the file to memory, set data_ to that memory address. Return true on // success, false on any kind of failure. This is a helper for Initialize(). - bool MapFileToMemoryInternal(); + bool MapFileToMemory(); - // Closes all open handles. Later we may want to make this public. + // Closes all open handles. void CloseHandles(); -#if defined(OS_WIN) - // MapFileToMemoryInternal calls this function. It provides the ability to - // pass in flags which control the mapped section. - bool MapFileToMemoryInternalEx(int flags); - - HANDLE file_mapping_; -#endif - PlatformFile file_; + File file_; uint8* data_; size_t length_; +#if defined(OS_WIN) + win::ScopedHandle file_mapping_; + bool image_; // Map as an image. +#endif + DISALLOW_COPY_AND_ASSIGN(MemoryMappedFile); }; diff --git a/base/files/memory_mapped_file_posix.cc b/base/files/memory_mapped_file_posix.cc index c4c477a..5d7e007 100644 --- a/base/files/memory_mapped_file_posix.cc +++ b/base/files/memory_mapped_file_posix.cc @@ -13,26 +13,23 @@ namespace base { -MemoryMappedFile::MemoryMappedFile() - : file_(kInvalidPlatformFileValue), - data_(NULL), - length_(0) { +MemoryMappedFile::MemoryMappedFile() : data_(NULL), length_(0) { } -bool MemoryMappedFile::MapFileToMemoryInternal() { +bool MemoryMappedFile::MapFileToMemory() { ThreadRestrictions::AssertIOAllowed(); struct stat file_stat; - if (fstat(file_, &file_stat) == kInvalidPlatformFileValue) { - DPLOG(ERROR) << "fstat " << file_; + if (fstat(file_.GetPlatformFile(), &file_stat) == -1 ) { + DPLOG(ERROR) << "fstat " << file_.GetPlatformFile(); return false; } length_ = file_stat.st_size; data_ = static_cast<uint8*>( - mmap(NULL, length_, PROT_READ, MAP_SHARED, file_, 0)); + mmap(NULL, length_, PROT_READ, MAP_SHARED, file_.GetPlatformFile(), 0)); if (data_ == MAP_FAILED) - DPLOG(ERROR) << "mmap " << file_; + DPLOG(ERROR) << "mmap " << file_.GetPlatformFile(); return data_ != MAP_FAILED; } @@ -42,12 +39,10 @@ void MemoryMappedFile::CloseHandles() { if (data_ != NULL) munmap(data_, length_); - if (file_ != kInvalidPlatformFileValue) - close(file_); + file_.Close(); data_ = NULL; length_ = 0; - file_ = kInvalidPlatformFileValue; } } // namespace base diff --git a/base/files/memory_mapped_file_win.cc b/base/files/memory_mapped_file_win.cc index 6942129..f382287 100644 --- a/base/files/memory_mapped_file_win.cc +++ b/base/files/memory_mapped_file_win.cc @@ -5,83 +5,52 @@ #include "base/files/memory_mapped_file.h" #include "base/files/file_path.h" -#include "base/logging.h" -#include "base/metrics/histogram.h" #include "base/strings/string16.h" #include "base/threading/thread_restrictions.h" namespace base { -MemoryMappedFile::MemoryMappedFile() - : file_(INVALID_HANDLE_VALUE), - file_mapping_(INVALID_HANDLE_VALUE), - data_(NULL), - length_(INVALID_FILE_SIZE) { +MemoryMappedFile::MemoryMappedFile() : data_(NULL), length_(0), image_(false) { } bool MemoryMappedFile::InitializeAsImageSection(const FilePath& file_name) { - if (IsValid()) - return false; - file_ = CreatePlatformFile(file_name, PLATFORM_FILE_OPEN | PLATFORM_FILE_READ, - NULL, NULL); - - if (file_ == kInvalidPlatformFileValue) { - DLOG(ERROR) << "Couldn't open " << file_name.AsUTF8Unsafe(); - return false; - } - - if (!MapFileToMemoryInternalEx(SEC_IMAGE)) { - CloseHandles(); - return false; - } - - return true; -} - -bool MemoryMappedFile::MapFileToMemoryInternal() { - return MapFileToMemoryInternalEx(0); + image_ = true; + return Initialize(file_name); } -bool MemoryMappedFile::MapFileToMemoryInternalEx(int flags) { +bool MemoryMappedFile::MapFileToMemory() { ThreadRestrictions::AssertIOAllowed(); - if (file_ == INVALID_HANDLE_VALUE) + if (!file_.IsValid()) return false; - length_ = ::GetFileSize(file_, NULL); - if (length_ == INVALID_FILE_SIZE) + int64 len = file_.GetLength(); + if (len <= 0 || len > kint32max) return false; + length_ = static_cast<size_t>(len); + + int flags = image_ ? SEC_IMAGE | PAGE_READONLY : PAGE_READONLY; - file_mapping_ = ::CreateFileMapping(file_, NULL, PAGE_READONLY | flags, - 0, 0, NULL); - if (!file_mapping_) { - // According to msdn, system error codes are only reserved up to 15999. - // http://msdn.microsoft.com/en-us/library/ms681381(v=VS.85).aspx. - UMA_HISTOGRAM_ENUMERATION("MemoryMappedFile.CreateFileMapping", - logging::GetLastSystemErrorCode(), 16000); + file_mapping_.Set(::CreateFileMapping(file_.GetPlatformFile(), NULL, + flags, 0, 0, NULL)); + if (!file_mapping_.IsValid()) return false; - } - data_ = static_cast<uint8*>( - ::MapViewOfFile(file_mapping_, FILE_MAP_READ, 0, 0, 0)); - if (!data_) { - UMA_HISTOGRAM_ENUMERATION("MemoryMappedFile.MapViewOfFile", - logging::GetLastSystemErrorCode(), 16000); - } + data_ = static_cast<uint8*>(::MapViewOfFile(file_mapping_.Get(), + FILE_MAP_READ, 0, 0, 0)); return data_ != NULL; } void MemoryMappedFile::CloseHandles() { if (data_) ::UnmapViewOfFile(data_); - if (file_mapping_ != INVALID_HANDLE_VALUE) - ::CloseHandle(file_mapping_); - if (file_ != INVALID_HANDLE_VALUE) - ::CloseHandle(file_); + if (file_mapping_.IsValid()) + file_mapping_.Close(); + if (file_.IsValid()) + file_.Close(); data_ = NULL; - file_mapping_ = file_ = INVALID_HANDLE_VALUE; - length_ = INVALID_FILE_SIZE; + length_ = 0; } } // namespace base diff --git a/chrome/app/chrome_main_delegate.cc b/chrome/app/chrome_main_delegate.cc index f75032e..579874d 100644 --- a/chrome/app/chrome_main_delegate.cc +++ b/chrome/app/chrome_main_delegate.cc @@ -667,7 +667,8 @@ void ChromeMainDelegate::PreSandboxStartup() { int locale_pak_fd = base::GlobalDescriptors::GetInstance()->MaybeGet( kAndroidLocalePakDescriptor); CHECK(locale_pak_fd != -1); - ResourceBundle::InitSharedInstanceWithPakFile(locale_pak_fd, false); + ResourceBundle::InitSharedInstanceWithPakFile(base::File(locale_pak_fd), + false); int extra_pak_keys[] = { kAndroidChrome100PercentPakDescriptor, @@ -678,7 +679,7 @@ void ChromeMainDelegate::PreSandboxStartup() { base::GlobalDescriptors::GetInstance()->MaybeGet(extra_pak_keys[i]); CHECK(pak_fd != -1); ResourceBundle::GetSharedInstance().AddDataPackFromFile( - pak_fd, ui::SCALE_FACTOR_100P); + base::File(pak_fd), ui::SCALE_FACTOR_100P); } base::i18n::SetICUDefaultLocale(locale); diff --git a/chrome/renderer/spellchecker/hunspell_engine.cc b/chrome/renderer/spellchecker/hunspell_engine.cc index 74dd292..c2cdd29f 100644 --- a/chrome/renderer/spellchecker/hunspell_engine.cc +++ b/chrome/renderer/spellchecker/hunspell_engine.cc @@ -61,7 +61,9 @@ void HunspellEngine::InitializeHunspell() { bdict_file_.reset(new base::MemoryMappedFile); - if (bdict_file_->Initialize(file_)) { + // TODO(rvargas): This object should not keep file_ after passing it to + // bdict_file_. + if (bdict_file_->Initialize(base::File(file_))) { TimeTicks debug_start_time = base::Histogram::DebugNow(); hunspell_.reset( diff --git a/chrome/utility/chrome_content_utility_client.cc b/chrome/utility/chrome_content_utility_client.cc index 48acaa9..9f872c5 100644 --- a/chrome/utility/chrome_content_utility_client.cc +++ b/chrome/utility/chrome_content_utility_client.cc @@ -752,8 +752,8 @@ void ChromeContentUtilityClient::OnAnalyzeZipFileForDownloadProtection( void ChromeContentUtilityClient::OnCheckMediaFile( int64 milliseconds_of_decoding, const IPC::PlatformFileForTransit& media_file) { - media::MediaFileChecker - checker(IPC::PlatformFileForTransitToPlatformFile(media_file)); + media::MediaFileChecker checker( + base::File(IPC::PlatformFileForTransitToPlatformFile(media_file))); const bool check_success = checker.Start( base::TimeDelta::FromMilliseconds(milliseconds_of_decoding)); Send(new ChromeUtilityHostMsg_CheckMediaFile_Finished(check_success)); diff --git a/content/shell/app/shell_main_delegate.cc b/content/shell/app/shell_main_delegate.cc index 9966c4a..9867be7 100644 --- a/content/shell/app/shell_main_delegate.cc +++ b/content/shell/app/shell_main_delegate.cc @@ -259,9 +259,10 @@ void ShellMainDelegate::InitializeResourceBundle() { int pak_fd = base::GlobalDescriptors::GetInstance()->MaybeGet(kShellPakDescriptor); if (pak_fd != base::kInvalidPlatformFileValue) { - ui::ResourceBundle::InitSharedInstanceWithPakFile(pak_fd, false); + ui::ResourceBundle::InitSharedInstanceWithPakFile(base::File(pak_fd), + false); ResourceBundle::GetSharedInstance().AddDataPackFromFile( - pak_fd, ui::SCALE_FACTOR_100P); + base::File(pak_fd), ui::SCALE_FACTOR_100P); return; } #endif diff --git a/media/base/media_file_checker.cc b/media/base/media_file_checker.cc index 494657d..74e83b6 100644 --- a/media/base/media_file_checker.cc +++ b/media/base/media_file_checker.cc @@ -21,20 +21,17 @@ static void OnError(bool* called) { *called = false; } -MediaFileChecker::MediaFileChecker(const base::PlatformFile& file) - : file_(file), - file_closer_(&file_) { +MediaFileChecker::MediaFileChecker(base::File file) : file_(file.Pass()) { } MediaFileChecker::~MediaFileChecker() { } bool MediaFileChecker::Start(base::TimeDelta check_time) { - media::FileDataSource source; + media::FileDataSource source(file_.Pass()); bool read_ok = true; media::BlockingUrlProtocol protocol(&source, base::Bind(&OnError, &read_ok)); media::FFmpegGlue glue(&protocol); - source.InitializeFromPlatformFile(file_); AVFormatContext* format_context = glue.format_context(); if (!glue.OpenContext()) diff --git a/media/base/media_file_checker.h b/media/base/media_file_checker.h index 6e8fc9f..8ed191a 100644 --- a/media/base/media_file_checker.h +++ b/media/base/media_file_checker.h @@ -6,8 +6,7 @@ #define MEDIA_BASE_MEDIA_FILE_CHECKER_H_ #include "base/basictypes.h" -#include "base/files/scoped_platform_file_closer.h" -#include "base/platform_file.h" +#include "base/files/file.h" #include "media/base/media_export.h" namespace base { @@ -21,7 +20,7 @@ namespace media { // file safe to use in the browser process. class MEDIA_EXPORT MediaFileChecker { public: - explicit MediaFileChecker(const base::PlatformFile& file); + explicit MediaFileChecker(base::File file); ~MediaFileChecker(); // After opening |file|, up to |check_time| amount of wall-clock time is spent @@ -30,8 +29,7 @@ class MEDIA_EXPORT MediaFileChecker { bool Start(base::TimeDelta check_time); private: - base::PlatformFile file_; - base::ScopedPlatformFileCloser file_closer_; + base::File file_; DISALLOW_COPY_AND_ASSIGN(MediaFileChecker); }; diff --git a/media/base/media_file_checker_unittest.cc b/media/base/media_file_checker_unittest.cc index ec61edf..001fe04 100644 --- a/media/base/media_file_checker_unittest.cc +++ b/media/base/media_file_checker_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/files/file.h" #include "base/logging.h" #include "build/build_config.h" #include "media/base/media_file_checker.h" @@ -11,20 +12,14 @@ namespace media { static void RunMediaFileChecker(const std::string& filename, bool expectation) { - base::PlatformFileError error; - base::PlatformFile file = base::CreatePlatformFile( - GetTestDataFilePath(filename), - base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ, - NULL, - &error); - ASSERT_EQ(base::PLATFORM_FILE_OK, error); - - MediaFileChecker checker(file); + base::File file(GetTestDataFilePath(filename), + base::File::FLAG_OPEN | base::File::FLAG_READ); + ASSERT_TRUE(file.IsValid()); + + MediaFileChecker checker(file.Pass()); const base::TimeDelta check_time = base::TimeDelta::FromMilliseconds(100); bool result = checker.Start(check_time); EXPECT_EQ(expectation, result); - - base::ClosePlatformFile(file); } TEST(MediaFileCheckerTest, InvalidFile) { diff --git a/media/filters/file_data_source.cc b/media/filters/file_data_source.cc index 341347e..8fb3100 100644 --- a/media/filters/file_data_source.cc +++ b/media/filters/file_data_source.cc @@ -15,21 +15,19 @@ FileDataSource::FileDataSource() force_streaming_(false) { } -bool FileDataSource::Initialize(const base::FilePath& file_path) { - DCHECK(!file_.IsValid()); - - if (!file_.Initialize(file_path)) - return false; +FileDataSource::FileDataSource(base::File file) + : force_read_errors_(false), + force_streaming_(false) { + if (!file_.Initialize(file.Pass())) + return; UpdateHostBytes(); - return true; } -bool FileDataSource::InitializeFromPlatformFile( - const base::PlatformFile& file) { +bool FileDataSource::Initialize(const base::FilePath& file_path) { DCHECK(!file_.IsValid()); - if (!file_.Initialize(file)) + if (!file_.Initialize(file_path)) return false; UpdateHostBytes(); diff --git a/media/filters/file_data_source.h b/media/filters/file_data_source.h index c0164dac..817e2b9 100644 --- a/media/filters/file_data_source.h +++ b/media/filters/file_data_source.h @@ -7,9 +7,9 @@ #include <string> +#include "base/files/file.h" #include "base/files/file_path.h" #include "base/files/memory_mapped_file.h" -#include "base/platform_file.h" #include "media/base/data_source.h" namespace media { @@ -19,10 +19,10 @@ namespace media { class MEDIA_EXPORT FileDataSource : public DataSource { public: FileDataSource(); + explicit FileDataSource(base::File file); virtual ~FileDataSource(); bool Initialize(const base::FilePath& file_path); - bool InitializeFromPlatformFile(const base::PlatformFile& file); // Implementation of DataSource. virtual void set_host(DataSourceHost* host) OVERRIDE; diff --git a/ui/base/resource/data_pack.cc b/ui/base/resource/data_pack.cc index 8f8f182..56d74a27 100644 --- a/ui/base/resource/data_pack.cc +++ b/ui/base/resource/data_pack.cc @@ -84,9 +84,9 @@ bool DataPack::LoadFromPath(const base::FilePath& path) { return LoadImpl(); } -bool DataPack::LoadFromFile(base::PlatformFile file) { +bool DataPack::LoadFromFile(base::File file) { mmap_.reset(new base::MemoryMappedFile); - if (!mmap_->Initialize(file)) { + if (!mmap_->Initialize(file.Pass())) { DLOG(ERROR) << "Failed to mmap datapack"; UMA_HISTOGRAM_ENUMERATION("DataPack.Load", INIT_FAILED_FROM_FILE, LOAD_ERRORS_COUNT); diff --git a/ui/base/resource/data_pack.h b/ui/base/resource/data_pack.h index 2ea938f..a1a92c5 100644 --- a/ui/base/resource/data_pack.h +++ b/ui/base/resource/data_pack.h @@ -12,8 +12,8 @@ #include <map> #include "base/basictypes.h" +#include "base/files/file.h" #include "base/memory/scoped_ptr.h" -#include "base/platform_file.h" #include "base/strings/string_piece.h" #include "ui/base/layout.h" #include "ui/base/resource/resource_handle.h" @@ -36,7 +36,7 @@ class UI_EXPORT DataPack : public ResourceHandle { bool LoadFromPath(const base::FilePath& path); // Loads a pack file from |file|, returning false on error. - bool LoadFromFile(base::PlatformFile file); + bool LoadFromFile(base::File file); // Writes a pack file containing |resources| to |path|. If there are any // text resources to be written, their encoding must already agree to the diff --git a/ui/base/resource/data_pack_unittest.cc b/ui/base/resource/data_pack_unittest.cc index 0ac7819d..e3cd2fb 100644 --- a/ui/base/resource/data_pack_unittest.cc +++ b/ui/base/resource/data_pack_unittest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "base/file_util.h" +#include "base/files/file.h" #include "base/files/file_path.h" #include "base/files/scoped_temp_dir.h" #include "base/path_service.h" @@ -62,15 +63,12 @@ TEST(DataPackTest, LoadFromFile) { ASSERT_EQ(file_util::WriteFile(data_path, kSamplePakContents, kSamplePakSize), static_cast<int>(kSamplePakSize)); - bool created = false; - base::PlatformFileError error_code = base::PLATFORM_FILE_OK; - base::PlatformFile file = base::CreatePlatformFile( - data_path, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ, - &created, &error_code); + base::File file(data_path, base::File::FLAG_OPEN | base::File::FLAG_READ); + ASSERT_TRUE(file.IsValid()); // Load the file through the data pack API. DataPack pack(SCALE_FACTOR_100P); - ASSERT_TRUE(pack.LoadFromFile(file)); + ASSERT_TRUE(pack.LoadFromFile(file.Pass())); base::StringPiece data; ASSERT_TRUE(pack.HasResource(4)); @@ -89,8 +87,6 @@ TEST(DataPackTest, LoadFromFile) { // Try looking up an invalid key. ASSERT_FALSE(pack.HasResource(140)); ASSERT_FALSE(pack.GetStringPiece(140, &data)); - - base::ClosePlatformFile(file); } INSTANTIATE_TEST_CASE_P(WriteBINARY, DataPackTest, ::testing::Values( diff --git a/ui/base/resource/resource_bundle.cc b/ui/base/resource/resource_bundle.cc index 22f191c..4371e4f 100644 --- a/ui/base/resource/resource_bundle.cc +++ b/ui/base/resource/resource_bundle.cc @@ -162,14 +162,14 @@ std::string ResourceBundle::InitSharedInstanceLocaleOnly( // static void ResourceBundle::InitSharedInstanceWithPakFile( - base::PlatformFile pak_file, bool should_load_common_resources) { + base::File pak_file, bool should_load_common_resources) { InitSharedInstance(NULL); if (should_load_common_resources) g_shared_instance_->LoadCommonResources(); scoped_ptr<DataPack> data_pack( new DataPack(SCALE_FACTOR_100P)); - if (!data_pack->LoadFromFile(pak_file)) { + if (!data_pack->LoadFromFile(pak_file.Pass())) { NOTREACHED() << "failed to load pak file"; return; } @@ -219,11 +219,11 @@ void ResourceBundle::AddOptionalDataPackFromPath(const base::FilePath& path, AddDataPackFromPathInternal(path, scale_factor, true); } -void ResourceBundle::AddDataPackFromFile(base::PlatformFile file, +void ResourceBundle::AddDataPackFromFile(base::File file, ScaleFactor scale_factor) { scoped_ptr<DataPack> data_pack( new DataPack(scale_factor)); - if (data_pack->LoadFromFile(file)) { + if (data_pack->LoadFromFile(file.Pass())) { AddDataPack(data_pack.release()); } else { LOG(ERROR) << "Failed to load data pack from file." diff --git a/ui/base/resource/resource_bundle.h b/ui/base/resource/resource_bundle.h index 272c5b3..9df8705 100644 --- a/ui/base/resource/resource_bundle.h +++ b/ui/base/resource/resource_bundle.h @@ -11,11 +11,11 @@ #include <string> #include "base/basictypes.h" +#include "base/files/file.h" #include "base/files/file_path.h" #include "base/gtest_prod_util.h" #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" -#include "base/platform_file.h" #include "base/strings/string16.h" #include "base/strings/string_piece.h" #include "ui/base/layout.h" @@ -135,8 +135,8 @@ class UI_EXPORT ResourceBundle { // controls whether or not ResourceBundle::LoadCommonResources is called. // This allows the use of this function in a sandbox without local file // access (as on Android). - static void InitSharedInstanceWithPakFile( - base::PlatformFile file, bool should_load_common_resources); + static void InitSharedInstanceWithPakFile(base::File file, + bool should_load_common_resources); // Initialize the ResourceBundle using given data pack path for testing. static void InitSharedInstanceWithPakPath(const base::FilePath& path); @@ -164,7 +164,7 @@ class UI_EXPORT ResourceBundle { ScaleFactor scale_factor); // Same as above but using an already open file. - void AddDataPackFromFile(base::PlatformFile file, ScaleFactor scale_factor); + void AddDataPackFromFile(base::File file, ScaleFactor scale_factor); // Same as AddDataPackFromPath but does not log an error if the pack fails to // load. |