Skip to content

Commit

Permalink
Switch some callers to use simpler base::WriteFile() variant.
Browse files Browse the repository at this point in the history
Change printing, media galleries, and some base/ callers to
base::WriteFile() to use the variants that returns a bool. Delete
redundant wrapper functions along the way. Also mark
base::RefCountedMemory::data() as const.

Bug: 418837
Change-Id: I1a6cba3c60261104c6e21aa919a004efb7de1f1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2191572
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#767453}
  • Loading branch information
leizleiz authored and Commit Bot committed May 11, 2020
1 parent d501d76 commit 0fc772d
Show file tree
Hide file tree
Showing 22 changed files with 47 additions and 86 deletions.
6 changes: 0 additions & 6 deletions base/files/file_path_watcher_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,6 @@ class FilePathWatcherTest : public testing::Test {
return temp_dir_.GetPath().AppendASCII("FilePathWatcherTest.lnk");
}

// Write |content| to |file|. Returns true on success.
bool WriteFile(const FilePath& file, const std::string& content) {
int write_size = ::base::WriteFile(file, content.c_str(), content.length());
return write_size == static_cast<int>(content.length());
}

bool SetupWatch(const FilePath& target,
FilePathWatcher* watcher,
TestDelegateBase* delegate,
Expand Down
4 changes: 2 additions & 2 deletions base/fuchsia/file_utils_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ TEST_F(OpenDirectoryTest, OpenNonExistent) {
// OpenDirectory() should open only directories.
TEST_F(OpenDirectoryTest, OpenFile) {
auto file_path = temp_dir.GetPath().AppendASCII("test_file");
ASSERT_TRUE(WriteFile(file_path, "foo", 3));
ASSERT_TRUE(WriteFile(file_path, "foo"));
auto dir = OpenDirectory(file_path);
ASSERT_FALSE(dir);
}

} // namespace fuchsia
} // namespace base
} // namespace base
7 changes: 2 additions & 5 deletions base/json/json_value_serializer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ TEST(JSONValueDeserializerTest, ReadProperJSONFromFile) {
ASSERT_TRUE(tempdir.CreateUniqueTempDir());
// Write it down in the file.
FilePath temp_file(tempdir.GetPath().AppendASCII("test.json"));
ASSERT_EQ(static_cast<int>(strlen(kProperJSON)),
WriteFile(temp_file, kProperJSON, strlen(kProperJSON)));
ASSERT_TRUE(WriteFile(temp_file, kProperJSON));

// Try to deserialize it through the serializer.
JSONFileValueDeserializer file_deserializer(temp_file);
Expand All @@ -173,9 +172,7 @@ TEST(JSONValueDeserializerTest, ReadJSONWithCommasFromFile) {
ASSERT_TRUE(tempdir.CreateUniqueTempDir());
// Write it down in the file.
FilePath temp_file(tempdir.GetPath().AppendASCII("test.json"));
ASSERT_EQ(static_cast<int>(strlen(kProperJSONWithCommas)),
WriteFile(temp_file, kProperJSONWithCommas,
strlen(kProperJSONWithCommas)));
ASSERT_TRUE(WriteFile(temp_file, kProperJSONWithCommas));

// Try to deserialize it through the serializer.
JSONFileValueDeserializer file_deserializer(temp_file);
Expand Down
2 changes: 1 addition & 1 deletion base/memory/ref_counted_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class BASE_EXPORT RefCountedMemory

// Alias for front() to make it possible for RefCountedMemory to implicitly
// convert to span.
const unsigned char* data() { return front(); }
const unsigned char* data() const { return front(); }

protected:
friend class RefCountedThreadSafe<RefCountedMemory>;
Expand Down
4 changes: 2 additions & 2 deletions base/path_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,12 @@ TEST_F(PathServiceTest, OverrideMultiple) {
FilePath fake_cache_dir1(temp_dir.GetPath().AppendASCII("1"));
EXPECT_TRUE(PathService::Override(my_special_key, fake_cache_dir1));
EXPECT_TRUE(PathExists(fake_cache_dir1));
ASSERT_EQ(1, WriteFile(fake_cache_dir1.AppendASCII("t1"), ".", 1));
ASSERT_TRUE(WriteFile(fake_cache_dir1.AppendASCII("t1"), "."));

FilePath fake_cache_dir2(temp_dir.GetPath().AppendASCII("2"));
EXPECT_TRUE(PathService::Override(my_special_key + 1, fake_cache_dir2));
EXPECT_TRUE(PathExists(fake_cache_dir2));
ASSERT_EQ(1, WriteFile(fake_cache_dir2.AppendASCII("t2"), ".", 1));
ASSERT_TRUE(WriteFile(fake_cache_dir2.AppendASCII("t2"), "."));

FilePath result;
EXPECT_TRUE(PathService::Get(my_special_key, &result));
Expand Down
2 changes: 1 addition & 1 deletion base/process/process_metrics_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ TEST(ProcessMetricsTest, GetDiskUsageBytesPerSecond) {
// Write a megabyte on disk.
const int kMegabyte = 1024 * 1014;
std::string data(kMegabyte, 'x');
ASSERT_EQ(kMegabyte, base::WriteFile(temp_path, data.c_str(), data.size()));
ASSERT_TRUE(base::WriteFile(temp_path, data));

// Validate that the counters move up.
EXPECT_GT(metrics->GetDiskUsageBytesPerSecond(), 0U);
Expand Down
2 changes: 1 addition & 1 deletion base/test/generate_fontconfig_caches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ int main() {
base::FilePath uuid_file_path =
dir_module.Append("test_fonts").Append(".uuid");
const char uuid[] = "fb5c91b2895aa445d23aebf7f9e2189c";
WriteFile(uuid_file_path, uuid, strlen(uuid));
WriteFile(uuid_file_path, uuid);

// fontconfig writes the mtime of the test_fonts directory into the cache. It
// presumably checks this later to ensure that the cache is still up to date.
Expand Down
4 changes: 1 addition & 3 deletions base/test/launcher/unit_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,7 @@ CommandLine DefaultUnitTestPlatformDelegate::GetCommandLineForChildGTestProcess(

std::string long_flags(
StrCat({"--", kGTestFilterFlag, "=", JoinString(test_names, ":")}));
CHECK_EQ(static_cast<int>(long_flags.size()),
WriteFile(flag_file, long_flags.data(),
static_cast<int>(long_flags.size())));
CHECK(WriteFile(flag_file, long_flags));

new_cmd_line.AppendSwitchPath(switches::kTestLauncherOutput, output_file);
new_cmd_line.AppendSwitchPath(kGTestFlagfileFlag, flag_file);
Expand Down
3 changes: 1 addition & 2 deletions base/test/trace_to_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ void TraceToFile::BeginTracing(const FilePath& path,
}

void TraceToFile::WriteFileHeader() {
const char str[] = "{\"traceEvents\": [";
WriteFile(path_, str, static_cast<int>(strlen(str)));
WriteFile(path_, "{\"traceEvents\": [");
}

void TraceToFile::AppendFileFooter() {
Expand Down
6 changes: 2 additions & 4 deletions base/trace_event/cpufreq_monitor_android_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,17 +151,15 @@ class CPUFreqMonitorTest : public testing::Test {
std::string file_path =
delegate_->GetScalingCurFreqPathString(pair.first);
std::string str_freq = base::StringPrintf("%d\n", pair.second);
base::WriteFile(base::FilePath(file_path), str_freq.c_str(),
str_freq.length());
base::WriteFile(base::FilePath(file_path), str_freq);
}
}

void CreateRelatedCPUFiles(const std::vector<unsigned int>& clusters,
const std::vector<std::string>& related_cpus) {
for (unsigned int i = 0; i < clusters.size(); i++) {
base::WriteFile(base::FilePath(delegate_->GetRelatedCPUsPathString(i)),
related_cpus[clusters[i]].c_str(),
related_cpus[clusters[i]].length());
related_cpus[clusters[i]]);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ namespace util {
namespace chromeos {

namespace {
bool SetFileContents(const base::FilePath& path, const std::string& contents) {
return static_cast<std::string::size_type>(base::WriteFile(
path, contents.c_str(), contents.size())) == contents.size();
}

// Since it would be very hard to mock sysfs instead we will send in our own
// implementation of WaitForKernelNotification which instead will block on a
Expand Down Expand Up @@ -68,6 +64,7 @@ void RunLoopRunWithTimeout(base::TimeDelta timeout) {
timeout);
run_loop.Run();
}

} // namespace

class TestSystemMemoryPressureEvaluator : public SystemMemoryPressureEvaluator {
Expand Down Expand Up @@ -113,14 +110,14 @@ TEST(ChromeOSSystemMemoryPressureEvaluatorTest, ParseMarginFileGood) {

base::FilePath margin_file = tmp_dir.GetPath().Append("margin");

ASSERT_TRUE(SetFileContents(margin_file, "123"));
ASSERT_TRUE(base::WriteFile(margin_file, "123"));
const std::vector<int> parts1 =
TestSystemMemoryPressureEvaluator::GetMarginFileParts(
margin_file.value());
ASSERT_EQ(1u, parts1.size());
ASSERT_EQ(123, parts1[0]);

ASSERT_TRUE(SetFileContents(margin_file, "123 456"));
ASSERT_TRUE(base::WriteFile(margin_file, "123 456"));
const std::vector<int> parts2 =
TestSystemMemoryPressureEvaluator::GetMarginFileParts(
margin_file.value());
Expand All @@ -135,19 +132,19 @@ TEST(ChromeOSSystemMemoryPressureEvaluatorTest, ParseMarginFileBad) {
base::FilePath margin_file = tmp_dir.GetPath().Append("margin");

// An empty margin file is bad.
ASSERT_TRUE(SetFileContents(margin_file, ""));
ASSERT_TRUE(base::WriteFile(margin_file, ""));
ASSERT_TRUE(
TestSystemMemoryPressureEvaluator::GetMarginFileParts(margin_file.value())
.empty());

// The numbers will be in base10, so 4a6 would be invalid.
ASSERT_TRUE(SetFileContents(margin_file, "123 4a6"));
ASSERT_TRUE(base::WriteFile(margin_file, "123 4a6"));
ASSERT_TRUE(
TestSystemMemoryPressureEvaluator::GetMarginFileParts(margin_file.value())
.empty());

// The numbers must be integers.
ASSERT_TRUE(SetFileContents(margin_file, "123.2 412.3"));
ASSERT_TRUE(base::WriteFile(margin_file, "123.2 412.3"));
ASSERT_TRUE(
TestSystemMemoryPressureEvaluator::GetMarginFileParts(margin_file.value())
.empty());
Expand Down Expand Up @@ -238,11 +235,11 @@ TEST(ChromeOSSystemMemoryPressureEvaluatorTest, CheckMemoryPressure) {

// Set the margin values to 500 (critical) and 1000 (moderate).
const std::string kMarginContents = "500 1000";
ASSERT_TRUE(SetFileContents(margin_file, kMarginContents));
ASSERT_TRUE(base::WriteFile(margin_file, kMarginContents));

// Write the initial available contents.
const std::string kInitialAvailableContents = "1500";
ASSERT_TRUE(SetFileContents(available_file, kInitialAvailableContents));
ASSERT_TRUE(base::WriteFile(available_file, kInitialAvailableContents));

base::test::TaskEnvironment task_environment(
base::test::TaskEnvironment::MainThreadType::UI);
Expand Down Expand Up @@ -286,7 +283,7 @@ TEST(ChromeOSSystemMemoryPressureEvaluatorTest, CheckMemoryPressure) {
evaluator->current_vote());

// Moderate Pressure.
ASSERT_TRUE(SetFileContents(available_file, "900"));
ASSERT_TRUE(base::WriteFile(available_file, "900"));
TriggerKernelNotification(write_end.get());
// TODO(bgeffon): Use RunLoop::QuitClosure() instead of relying on "spin for
// 1 second".
Expand All @@ -295,28 +292,28 @@ TEST(ChromeOSSystemMemoryPressureEvaluatorTest, CheckMemoryPressure) {
evaluator->current_vote());

// Critical Pressure.
ASSERT_TRUE(SetFileContents(available_file, "450"));
ASSERT_TRUE(base::WriteFile(available_file, "450"));
TriggerKernelNotification(write_end.get());
RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL,
evaluator->current_vote());

// Moderate Pressure.
ASSERT_TRUE(SetFileContents(available_file, "550"));
ASSERT_TRUE(base::WriteFile(available_file, "550"));
TriggerKernelNotification(write_end.get());
RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE,
evaluator->current_vote());

// No pressure, note: this will not cause any event.
ASSERT_TRUE(SetFileContents(available_file, "1150"));
ASSERT_TRUE(base::WriteFile(available_file, "1150"));
TriggerKernelNotification(write_end.get());
RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE,
evaluator->current_vote());

// Back into moderate.
ASSERT_TRUE(SetFileContents(available_file, "950"));
ASSERT_TRUE(base::WriteFile(available_file, "950"));
TriggerKernelNotification(write_end.get());
RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,7 @@ class MediaGalleriesGalleryWatchApiTest : public extensions::ExtensionApiTest {
base::ScopedAllowBlockingForTesting allow_blocking;
base::FilePath gallery_file =
test_gallery_.GetPath().Append(FILE_PATH_LITERAL("test1.txt"));
std::string content("new content");
int write_size =
base::WriteFile(gallery_file, content.c_str(), content.length());
return (write_size == static_cast<int>(content.length()));
return base::WriteFile(gallery_file, "new content");
}

void SetupGalleryWatches() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ class MediaFileValidatorTest : public InProcessBrowserTest {

test_file_size_ = content.size();
base::FilePath test_file = src_path.AppendASCII(filename);
ASSERT_EQ(test_file_size_,
base::WriteFile(test_file, content.data(), test_file_size_));
ASSERT_TRUE(base::WriteFile(test_file, content));

base::FilePath dest_path = base.AppendASCII("dest_fs");
ASSERT_TRUE(base::CreateDirectory(dest_path));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,8 @@ void PopulateDirectoryWithTestCases(const base::FilePath& dir,
if (test_cases[i].is_directory) {
ASSERT_TRUE(base::CreateDirectory(path));
} else {
ASSERT_TRUE(test_cases[i].content != nullptr);
int len = strlen(test_cases[i].content);
ASSERT_EQ(len, base::WriteFile(path, test_cases[i].content, len));
ASSERT_TRUE(test_cases[i].content);
ASSERT_TRUE(base::WriteFile(path, test_cases[i].content));
}
}
}
Expand Down Expand Up @@ -322,8 +321,7 @@ TEST_F(NativeMediaFileUtilTest, CopyDestFiltering) {
base::FilePath src_path = root_path().AppendASCII("foo.jpg");
FileSystemURL src_url = CreateURL(FPL("foo.jpg"));
static const char kDummyData[] = "dummy";
ASSERT_EQ(static_cast<int>(strlen(kDummyData)),
base::WriteFile(src_path, kDummyData, strlen(kDummyData)));
ASSERT_TRUE(base::WriteFile(src_path, kDummyData));

for (size_t i = 0; i < base::size(kFilteringTestCases); ++i) {
if (loop_count == 0 && kFilteringTestCases[i].is_directory) {
Expand Down Expand Up @@ -430,8 +428,7 @@ TEST_F(NativeMediaFileUtilTest, MoveDestFiltering) {
base::FilePath src_path = root_path().AppendASCII("foo.jpg");
FileSystemURL src_url = CreateURL(FPL("foo.jpg"));
static const char kDummyData[] = "dummy";
ASSERT_EQ(static_cast<int>(strlen(kDummyData)),
base::WriteFile(src_path, kDummyData, strlen(kDummyData)));
ASSERT_TRUE(base::WriteFile(src_path, kDummyData));

FileSystemURL root_url = CreateURL(FPL(""));
FileSystemURL url = CreateURL(kFilteringTestCases[i].path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,8 @@ TEST_F(GalleryWatchManagerTest, TestWatchOperation) {

base::RunLoop success_loop;
ExpectGalleryChanged(&success_loop);
ASSERT_EQ(4, base::WriteFile(temp_dir.GetPath().AppendASCII("fake file"),
"blah", 4));
ASSERT_TRUE(
base::WriteFile(temp_dir.GetPath().AppendASCII("fake file"), "blah"));
success_loop.Run();
}

Expand All @@ -400,8 +400,8 @@ TEST_F(GalleryWatchManagerTest, TestWatchOperationAfterProfileShutdown) {
// Trigger a watch that should have been removed when the profile was
// destroyed to catch regressions. crbug.com/467627
base::RunLoop run_loop;
ASSERT_EQ(4, base::WriteFile(temp_dir.GetPath().AppendASCII("fake file"),
"blah", 4));
ASSERT_TRUE(
base::WriteFile(temp_dir.GetPath().AppendASCII("fake file"), "blah"));
run_loop.RunUntilIdle();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ - (void)requestDownloadFile:(ICCameraFile*)file
// filename. Do that here to require a rename.
saveAsFilename += ".jpg";
base::FilePath toBeSaved = saveDir.Append(saveAsFilename);
ASSERT_EQ(static_cast<int>(strlen(kTestFileContents)),
base::WriteFile(toBeSaved, kTestFileContents,
strlen(kTestFileContents)));
ASSERT_TRUE(base::WriteFile(toBeSaved, kTestFileContents));

NSMutableDictionary* returnOptions =
[NSMutableDictionary dictionaryWithDictionary:options];
Expand Down
2 changes: 1 addition & 1 deletion chrome/service/cloud_print/connector_settings_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class ConnectorSettingsTest : public testing::Test {
if (json) {
std::string content = json;
std::replace(content.begin(), content.end(), '\'', '"');
base::WriteFile(file_name, content.c_str(), content.size());
base::WriteFile(file_name, content);
}
ServiceProcessPrefs* prefs =
new ServiceProcessPrefs(file_name, task_runner_.get());
Expand Down
5 changes: 2 additions & 3 deletions chrome/service/cloud_print/printer_job_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -386,11 +386,10 @@ PrinterJobHandler::HandlePrintDataResponse(const net::URLFetcher* source,
if (base::CreateTemporaryFile(&job_details_.print_data_file_path_)) {
UMA_HISTOGRAM_ENUMERATION("CloudPrint.JobHandlerEvent", JOB_HANDLER_DATA,
JOB_HANDLER_MAX);
int ret = base::WriteFile(job_details_.print_data_file_path_,
data.c_str(), data.length());
bool ret = base::WriteFile(job_details_.print_data_file_path_, data);
source->GetResponseHeaders()->GetMimeType(
&job_details_.print_data_mime_type_);
if (ret == static_cast<int>(data.length())) {
if (ret) {
UpdateJobStatus(PRINT_JOB_STATUS_IN_PROGRESS, JOB_SUCCESS);
return CloudPrintURLFetcher::STOP_PROCESSING;
}
Expand Down
12 changes: 3 additions & 9 deletions chromeos/printing/ppd_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -642,9 +642,7 @@ TEST_F(PpdProviderTest, ResolveUserSuppliedUrlPpdFromFile) {

std::string user_ppd_contents = "Woohoo";

ASSERT_EQ(base::WriteFile(filename, user_ppd_contents.data(),
user_ppd_contents.size()),
static_cast<int>(user_ppd_contents.size()));
ASSERT_TRUE(base::WriteFile(filename, user_ppd_contents));

Printer::PpdReference ref;
ref.user_supplied_ppd_url =
Expand All @@ -669,9 +667,7 @@ TEST_F(PpdProviderTest, ResolvedPpdsGetCached) {
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
base::FilePath filename = temp_dir.GetPath().Append("my_spiffy.ppd");

ASSERT_EQ(base::WriteFile(filename, user_ppd_contents.data(),
user_ppd_contents.size()),
static_cast<int>(user_ppd_contents.size()));
ASSERT_TRUE(base::WriteFile(filename, user_ppd_contents));

ref.user_supplied_ppd_url =
base::StringPrintf("file://%s", filename.MaybeAsASCII().c_str());
Expand Down Expand Up @@ -970,9 +966,7 @@ TEST_F(PpdProviderTest, UserPpdAlwaysRefreshedIfAvailable) {

// Write different contents to disk, so that the cached contents are
// now stale.
ASSERT_EQ(base::WriteFile(filename, disk_ppd_contents.data(),
disk_ppd_contents.size()),
static_cast<int>(disk_ppd_contents.size()));
ASSERT_TRUE(base::WriteFile(filename, disk_ppd_contents));

provider->ResolvePpd(ref, base::BindOnce(&PpdProviderTest::CaptureResolvePpd,
base::Unretained(this)));
Expand Down
Loading

0 comments on commit 0fc772d

Please sign in to comment.