Skip to content

Commit

Permalink
Clean up SbFile usages (#3243)
Browse files Browse the repository at this point in the history
b/302715109

(cherry picked from commit fd0c5ff)
  • Loading branch information
haozheng-cobalt authored and anonymous1-me committed Jun 24, 2024
1 parent 0d42143 commit bbc9d17
Show file tree
Hide file tree
Showing 47 changed files with 525 additions and 410 deletions.
11 changes: 9 additions & 2 deletions base/files/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ struct stat;

namespace base {

#if defined(STARBOARD)
using stat_wrapper_t = struct ::stat;
#else
using stat_wrapper_t = struct stat;
#endif

// Thin wrapper around an OS-level file.
// Note that this class does not provide any support for asynchronous IO, other
Expand Down Expand Up @@ -118,7 +122,7 @@ class BASE_EXPORT File {
struct BASE_EXPORT Info {
Info();
~Info();
#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) || defined(STARBOARD)
// Fills this struct with values from |stat_info|.
void FromStat(const stat_wrapper_t& stat_info);
#endif
Expand Down Expand Up @@ -370,11 +374,14 @@ class BASE_EXPORT File {
// Converts an error value to a human-readable form. Used for logging.
static std::string ErrorToString(Error error);

#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) || defined(STARBOARD)
// Wrapper for stat() or stat64().
static int Stat(const char* path, stat_wrapper_t* sb);
static int Fstat(int fd, stat_wrapper_t* sb);
# if !defined(STARBOARD)
// Starboard does not support lstat yet.
static int Lstat(const char* path, stat_wrapper_t* sb);
#endif
#endif

// This function can be used to augment `flags` with the correct flags
Expand Down
14 changes: 6 additions & 8 deletions base/files/file_enumerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#include "build/build_config.h"

#if defined(STARBOARD)
#include <sys/stat.h>
#include <unistd.h>
#include "starboard/file.h"
#elif BUILDFLAG(IS_WIN)
#include "base/win/windows_types.h"
Expand Down Expand Up @@ -60,29 +62,25 @@ class BASE_EXPORT FileEnumerator {
// On POSIX systems, this is rounded down to the second.
Time GetLastModifiedTime() const;

#if defined(STARBOARD)
#if defined(STARBOARD) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
const stat_wrapper_t& stat() const { return stat_; }
#elif BUILDFLAG(IS_WIN)
// Note that the cAlternateFileName (used to hold the "short" 8.3 name)
// of the WIN32_FIND_DATA will be empty. Since we don't use short file
// names, we tell Windows to omit it which speeds up the query slightly.
const WIN32_FIND_DATA& find_data() const {
return *ChromeToWindowsType(&find_data_);
}
#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
const stat_wrapper_t& stat() const { return stat_; }
#endif

private:
friend class FileEnumerator;

#if defined(STARBOARD)
#if defined(STARBOARD) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
stat_wrapper_t stat_;
FilePath filename_;
SbFileInfo sb_info_;
#elif BUILDFLAG(IS_WIN)
CHROME_WIN32_FIND_DATA find_data_;
#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
stat_wrapper_t stat_;
FilePath filename_;
#endif
};

Expand Down
19 changes: 9 additions & 10 deletions base/files/file_enumerator_starboard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,23 @@ namespace base {
// FileEnumerator::FileInfo ----------------------------------------------------

FileEnumerator::FileInfo::FileInfo() {
memset(&sb_info_, 0, sizeof(sb_info_));
memset(&stat_, 0, sizeof(stat_));
}

bool FileEnumerator::FileInfo::IsDirectory() const {
return sb_info_.is_directory;
return S_ISDIR(stat_.st_mode);
}

FilePath FileEnumerator::FileInfo::GetName() const {
return filename_;
}

int64_t FileEnumerator::FileInfo::GetSize() const {
return sb_info_.size;
return stat_.st_size;
}

base::Time FileEnumerator::FileInfo::GetLastModifiedTime() const {
return base::Time::FromDeltaSinceWindowsEpoch(
base::TimeDelta::FromMicroseconds(sb_info_.last_modified));
return base::Time::FromTimeT(stat_.st_mtime);
}

// FileEnumerator --------------------------------------------------------------
Expand Down Expand Up @@ -124,9 +123,9 @@ std::vector<FileEnumerator::FileInfo> FileEnumerator::ReadDirectory(

FilePath full_name = source.Append(filename);
// TODO: Make sure this follows symlinks on relevant platforms.
if (!SbFileGetPathInfo(full_name.value().c_str(), &info.sb_info_)) {
if (stat(full_name.value().c_str(), &info.stat_) != 0) {
DPLOG(ERROR) << "Couldn't SbFileGetInfo on " << full_name.value();
memset(&info.sb_info_, 0, sizeof(info.sb_info_));
memset(&info.stat_, 0, sizeof(info.stat_));
}
return info;
};
Expand Down Expand Up @@ -186,12 +185,12 @@ FilePath FileEnumerator::Next() {
continue;
}

if (recursive_ && file_info.sb_info_.is_directory) {
if (recursive_ && file_info.IsDirectory()) {
pending_paths_.push(full_path);
}

if ((file_info.sb_info_.is_directory && (file_type_ & DIRECTORIES)) ||
(!file_info.sb_info_.is_directory && (file_type_ & FILES)) ||
if ((file_info.IsDirectory() && (file_type_ & DIRECTORIES)) ||
(!file_info.IsDirectory() && (file_type_ & FILES)) ||
(file_type_ & NAMES_ONLY)) {
directory_entries_.push_back(file_info);
}
Expand Down
33 changes: 31 additions & 2 deletions base/files/file_enumerator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ bool CreateDummyFile(const FilePath& path) {
bool GetFileInfo(const FilePath& file_path, File::Info& info) {
// FLAG_WIN_BACKUP_SEMANTICS: Needed to open directories on Windows.
File f(file_path,
File::FLAG_OPEN | File::FLAG_READ | File::FLAG_WIN_BACKUP_SEMANTICS);
File::FLAG_OPEN | File::FLAG_READ | File::FLAG_WIN_BACKUP_SEMANTICS);
if (!f.IsValid()) {
LOG(ERROR) << "Could not open " << file_path.value() << ": "
<< File::ErrorToString(f.error_details());
Expand Down Expand Up @@ -520,7 +520,15 @@ TEST(FileEnumerator, GetInfoRecursive) {
// all the files.
for (TestDirectory& dir : directories) {
const FilePath dir_path = temp_dir.GetPath().Append(dir.name);
#if defined(STARBOARD)
#ifdef _WIN32
// TODO: Reable this test when support directory open in base::File for Windows.
// Below tests would fail because we are now using _open from <io.h> instead of
// CreateFile from <fileapi.h>.
#else
ASSERT_TRUE(GetFileInfo(dir_path, dir.info));
#endif
#endif
}

FileEnumerator file_enumerator(
Expand All @@ -530,13 +538,19 @@ TEST(FileEnumerator, GetInfoRecursive) {
auto info = file_enumerator.GetInfo();
bool found = false;
if (info.IsDirectory()) {
#if defined(STARBOARD)
#ifdef _WIN32
// Reable this test when support directory open in base::File for Windows.
#else
for (TestDirectory& dir : directories) {
if (info.GetName() == dir.name) {
CheckDirectoryAgainstInfo(info, dir);
found = true;
break;
}
}
#endif
#endif
} else {
for (TestFile& file : files) {
if (info.GetName() == file.path.BaseName()) {
Expand All @@ -546,14 +560,25 @@ TEST(FileEnumerator, GetInfoRecursive) {
}
}
}

#if defined(STARBOARD)
#ifdef _WIN32
// Reable this test when support directory open in base::File for Windows.
#else
EXPECT_TRUE(found) << "Got unexpected result " << info.GetName().value();
#endif
#endif
}

#if defined(STARBOARD)
#ifdef _WIN32
// Reable this test when support directory open in base::File for Windows.
#else
for (const TestDirectory& dir : directories) {
EXPECT_TRUE(dir.found) << "Directory " << dir.name.value()
<< " was not returned";
}
#endif
#endif
for (const TestFile& file : files) {
EXPECT_TRUE(file.found)
<< "File " << file.path.value() << " was not returned";
Expand All @@ -569,6 +594,10 @@ TEST(FileEnumerator, GetInfoRecursive) {
// a bug in Windows, not us -- you can see it with the "dir" command (notice
// that the time of . and .. always match). Skip this test.
// https://crbug.com/1119546
#elif defined(STARBOARD)
#ifdef _WIN32
// Reable this test when support directory open in base::File for Windows.
#endif
#else
// Tests that FileEnumerator::GetInfo() returns the correct info for the ..
// directory.
Expand Down
Loading

0 comments on commit bbc9d17

Please sign in to comment.