use std::optional base::GetFileSize() in misc. files

Bug: 371234479
Change-Id: If81a7b96fc7081f0fffb274622055e493abb52c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5938237
Commit-Queue: Helmut Januschka <helmut@januschka.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Owners-Override: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1371862}

base/files/file_unittest.cc

@@ -11,6 +11,7 @@
 
 #include <stdint.h>
 
+#include <optional>
 #include <utility>
 
 #include "base/files/file_util.h"
@@ -244,13 +245,13 @@ TEST(FileTest, ReadWrite) {
   EXPECT_EQ(kPartialWriteLength, bytes_written);
 
   // Make sure the file was extended.
-  int64_t file_size = 0;
-  EXPECT_TRUE(GetFileSize(file_path, &file_size));
-  EXPECT_EQ(kOffsetBeyondEndOfFile + kPartialWriteLength, file_size);
+  std::optional<int64_t> file_size = GetFileSize(file_path);
+  ASSERT_TRUE(file_size.has_value());
+  EXPECT_EQ(kOffsetBeyondEndOfFile + kPartialWriteLength, file_size.value());
 
   // Make sure the file was zero-padded.
   char data_read_2[32];
-  bytes_read = file.Read(0, data_read_2, static_cast<int>(file_size));
+  bytes_read = file.Read(0, data_read_2, static_cast<int>(file_size.value()));
   EXPECT_EQ(file_size, bytes_read);
   for (int i = 0; i < kTestDataSize; i++)
     EXPECT_EQ(data_to_write[i], data_read_2[i]);
@@ -318,10 +319,10 @@ TEST(FileTest, ReadWriteSpans) {
   EXPECT_EQ(kPartialWriteLength, bytes_written.value());
 
   // Make sure the file was extended.
-  int64_t file_size = 0;
-  EXPECT_TRUE(GetFileSize(file_path, &file_size));
+  std::optional<int64_t> file_size = GetFileSize(file_path);
+  ASSERT_TRUE(file_size.has_value());
   EXPECT_EQ(static_cast<int64_t>(kOffsetBeyondEndOfFile + kPartialWriteLength),
-            file_size);
+            file_size.value());
 
   // Make sure the file was zero-padded.
   uint8_t data_read_2[32];
@@ -428,15 +429,15 @@ TEST(FileTest, Length) {
 
   // Extend the file.
   const int kExtendedFileLength = 10;
-  int64_t file_size = 0;
   EXPECT_TRUE(file.SetLength(kExtendedFileLength));
   EXPECT_EQ(kExtendedFileLength, file.GetLength());
-  EXPECT_TRUE(GetFileSize(file_path, &file_size));
-  EXPECT_EQ(kExtendedFileLength, file_size);
+  std::optional<int64_t> file_size = GetFileSize(file_path);
+  ASSERT_TRUE(file_size.has_value());
+  EXPECT_EQ(kExtendedFileLength, file_size.value());
 
   // Make sure the file was zero-padded.
   char data_read[32];
-  int bytes_read = file.Read(0, data_read, static_cast<int>(file_size));
+  int bytes_read = file.Read(0, data_read, static_cast<int>(file_size.value()));
   EXPECT_EQ(file_size, bytes_read);
   for (int i = 0; i < kTestDataSize; i++)
     EXPECT_EQ(data_to_write[i], data_read[i]);
@@ -447,22 +448,26 @@ TEST(FileTest, Length) {
   const int kTruncatedFileLength = 2;
   EXPECT_TRUE(file.SetLength(kTruncatedFileLength));
   EXPECT_EQ(kTruncatedFileLength, file.GetLength());
-  EXPECT_TRUE(GetFileSize(file_path, &file_size));
-  EXPECT_EQ(kTruncatedFileLength, file_size);
+
+  file_size = GetFileSize(file_path);
+  ASSERT_TRUE(file_size.has_value());
+  EXPECT_EQ(kTruncatedFileLength, file_size.value());
 
   // Make sure the file was truncated.
   bytes_read = file.Read(0, data_read, kTestDataSize);
-  EXPECT_EQ(file_size, bytes_read);
-  for (int i = 0; i < file_size; i++)
+  EXPECT_EQ(file_size.value(), bytes_read);
+  for (int i = 0; i < file_size.value(); i++) {
     EXPECT_EQ(data_to_write[i], data_read[i]);
+  }
 
 #if !BUILDFLAG(IS_FUCHSIA)  // Fuchsia doesn't seem to support big files.
   // Expand the file past the 4 GB limit.
   const int64_t kBigFileLength = 5'000'000'000;
   EXPECT_TRUE(file.SetLength(kBigFileLength));
   EXPECT_EQ(kBigFileLength, file.GetLength());
-  EXPECT_TRUE(GetFileSize(file_path, &file_size));
-  EXPECT_EQ(kBigFileLength, file_size);
+  file_size = GetFileSize(file_path);
+  ASSERT_TRUE(file_size.has_value());
+  EXPECT_EQ(kBigFileLength, file_size.value());
 #endif
 
   // Close the file and reopen with base::File::FLAG_CREATE_ALWAYS, and make

base/files/file_util_unittest.cc

@@ -12,6 +12,7 @@
 #include <fstream>
 #include <initializer_list>
 #include <memory>
+#include <optional>
 #include <set>
 #include <utility>
 #include <vector>
@@ -384,9 +385,9 @@ TEST_F(FileUtilTest, FileAndDirectorySize) {
 
   std::optional<int64_t> size_f1 = GetFileSize(file_01);
   ASSERT_THAT(size_f1, testing::Optional(20));
-  int64_t size_f1_out = 0;
-  ASSERT_TRUE(GetFileSize(file_01, &size_f1_out));
-  EXPECT_EQ(size_f1.value(), size_f1_out);
+  std::optional<int64_t> size_f1_out = GetFileSize(file_01);
+  ASSERT_TRUE(size_f1_out.has_value());
+  EXPECT_EQ(size_f1.value(), size_f1_out.value());
 
   FilePath subdir_path = temp_dir_.GetPath().Append(FPL("Level2"));
   CreateDirectory(subdir_path);
@@ -395,9 +396,9 @@ TEST_F(FileUtilTest, FileAndDirectorySize) {
   CreateTextFile(file_02, L"123456789012345678901234567890");
   std::optional<int64_t> size_f2 = GetFileSize(file_02);
   ASSERT_THAT(size_f2, testing::Optional(30));
-  int64_t size_f2_out = 0;
-  ASSERT_TRUE(GetFileSize(file_02, &size_f2_out));
-  EXPECT_EQ(size_f2.value(), size_f2_out);
+  std::optional<int64_t> size_f2_out = GetFileSize(file_02);
+  ASSERT_TRUE(size_f2_out.has_value());
+  EXPECT_EQ(size_f2.value(), size_f2_out.value());
 
   FilePath subsubdir_path = subdir_path.Append(FPL("Level3"));
   CreateDirectory(subsubdir_path);
@@ -4442,9 +4443,9 @@ TEST_F(FileUtilTest, ValidContentUriTest) {
   data_dir = data_dir.AppendASCII("file_util");
   ASSERT_TRUE(PathExists(data_dir));
   FilePath image_file = data_dir.Append(FILE_PATH_LITERAL("red.png"));
-  int64_t image_size;
-  GetFileSize(image_file, &image_size);
-  ASSERT_GT(image_size, 0);
+  std::optional<int64_t> image_size = GetFileSize(image_file);
+  ASSERT_TRUE(image_size.has_value());
+  ASSERT_GT(image_size.value(), 0);
 
   // Insert the image into MediaStore. MediaStore will do some conversions, and
   // return the content URI.
@@ -4453,16 +4454,17 @@ TEST_F(FileUtilTest, ValidContentUriTest) {
   EXPECT_TRUE(PathExists(path));
   // The file size may not equal to the input image as MediaStore may convert
   // the image.
-  int64_t content_uri_size;
-  GetFileSize(path, &content_uri_size);
-  EXPECT_EQ(image_size, content_uri_size);
+  std::optional<int64_t> content_uri_size = GetFileSize(path);
+  ASSERT_TRUE(content_uri_size.has_value());
+  EXPECT_EQ(image_size.value(), content_uri_size.value());
 
   // We should be able to read the file.
   File file(path, File::FLAG_OPEN | File::FLAG_READ);
   EXPECT_TRUE(file.IsValid());
-  auto buffer = std::make_unique<char[]>(image_size);
+  auto buffer = std::make_unique<char[]>(image_size.value());
   // SAFETY: required for test.
-  EXPECT_TRUE(UNSAFE_BUFFERS(file.ReadAtCurrentPos(buffer.get(), image_size)));
+  EXPECT_TRUE(
+      UNSAFE_BUFFERS(file.ReadAtCurrentPos(buffer.get(), image_size.value())));
 }
 
 TEST_F(FileUtilTest, WriteContentUri) {
@@ -4475,23 +4477,22 @@ TEST_F(FileUtilTest, WriteContentUri) {
   // We should be able to open the file as writable which truncates the file.
   File file = File(content_uri, File::FLAG_CREATE_ALWAYS | File::FLAG_WRITE);
   EXPECT_TRUE(file.IsValid());
-  int64_t size;
-  GetFileSize(path, &size);
-  EXPECT_EQ(size, 0);
+  std::optional<int64_t> size = GetFileSize(path);
+  ASSERT_TRUE(size.has_value());
+  EXPECT_EQ(size.value(), 0);
 
   EXPECT_EQ(*file.WriteAtCurrentPos(byte_span_from_cstring("123")), 3u);
   EXPECT_TRUE(file.Flush());
-  GetFileSize(path, &size);
-  EXPECT_EQ(size, 3);
+  size = GetFileSize(path);
+  ASSERT_TRUE(size.has_value());
+  EXPECT_EQ(size.value(), 3);
 }
 
 TEST_F(FileUtilTest, NonExistentContentUriTest) {
   FilePath path("content://foo.bar");
   EXPECT_TRUE(path.IsContentUri());
   EXPECT_FALSE(PathExists(path));
-  // Size should be smaller than 0.
-  int64_t size;
-  EXPECT_FALSE(GetFileSize(path, &size));
+  EXPECT_FALSE(GetFileSize(path).has_value());
 
   // We should not be able to read the file.
   File file(path, File::FLAG_OPEN | File::FLAG_READ);

base/files/memory_mapped_file_unittest.cc

@@ -12,6 +12,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <optional>
 #include <utility>
 
 #include "base/containers/heap_array.h"
@@ -203,9 +204,9 @@ TEST_F(MemoryMappedFileTest, WriteableFile) {
         CheckBufferContents(map.bytes().first(kFileSize - 1).subspan(3), 3));
   }
 
-  int64_t file_size;
-  ASSERT_TRUE(GetFileSize(temp_file_path(), &file_size));
-  EXPECT_EQ(static_cast<int64_t>(kFileSize), file_size);
+  std::optional<int64_t> file_size = GetFileSize(temp_file_path());
+  ASSERT_TRUE(file_size.has_value());
+  EXPECT_EQ(static_cast<int64_t>(kFileSize), file_size.value());
 
   std::string contents;
   ASSERT_TRUE(ReadFileToString(temp_file_path(), &contents));
@@ -236,9 +237,9 @@ TEST_F(MemoryMappedFileTest, CopyOnWrite) {
         CheckBufferContents(map.bytes().first(kFileSize - 1).subspan(3), 3));
   }
 
-  int64_t file_size;
-  ASSERT_TRUE(GetFileSize(temp_file_path(), &file_size));
-  EXPECT_EQ(static_cast<int64_t>(kFileSize), file_size);
+  std::optional<int64_t> file_size = GetFileSize(temp_file_path());
+  ASSERT_TRUE(file_size.has_value());
+  EXPECT_EQ(static_cast<int64_t>(kFileSize), file_size.value());
 
   // Although the buffer has been modified in memory, the file is unchanged.
   std::string contents;
@@ -273,10 +274,10 @@ TEST_F(MemoryMappedFileTest, ExtendableFile) {
     EXPECT_TRUE(CheckBufferContents(map.bytes().first(kFileSize), 0));
   }
 
-  int64_t file_size;
-  ASSERT_TRUE(GetFileSize(temp_file_path(), &file_size));
-  EXPECT_LE(static_cast<int64_t>(kFileSize + 3), file_size);
-  EXPECT_GE(static_cast<int64_t>(kFileSize + kFileExtend), file_size);
+  std::optional<int64_t> file_size = GetFileSize(temp_file_path());
+  ASSERT_TRUE(file_size.has_value());
+  EXPECT_LE(static_cast<int64_t>(kFileSize + 3), file_size.value());
+  EXPECT_GE(static_cast<int64_t>(kFileSize + kFileExtend), file_size.value());
 
   std::string contents;
   ASSERT_TRUE(ReadFileToString(temp_file_path(), &contents));

base/metrics/persistent_memory_allocator_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/metrics/persistent_memory_allocator.h"
 
 #include <memory>
+#include <optional>
 
 #include "base/containers/heap_array.h"
 #include "base/files/file.h"
@@ -48,9 +49,9 @@ void SetFileLength(const base::FilePath& path, size_t length) {
     ASSERT_TRUE(file.SetLength(static_cast<int64_t>(length)));
   }
 
-  int64_t actual_length;
-  DCHECK(GetFileSize(path, &actual_length));
-  DCHECK_EQ(length, static_cast<size_t>(actual_length));
+  std::optional<int64_t> actual_length = GetFileSize(path);
+  DCHECK(actual_length.has_value());
+  DCHECK_EQ(length, static_cast<size_t>(actual_length.value()));
 }
 
 }  // namespace
@@ -880,8 +881,8 @@ TEST(FilePersistentMemoryAllocatorTest, ExtendTest) {
     writer.Write(0, (const char*)local.data(), local.used());
   }
   ASSERT_TRUE(PathExists(file_path));
-  int64_t before_size;
-  ASSERT_TRUE(GetFileSize(file_path, &before_size));
+  std::optional<int64_t> before_size = GetFileSize(file_path);
+  ASSERT_TRUE(before_size.has_value());
 
   // Map it as an extendable read/write file and append to it.
   {
@@ -892,16 +893,16 @@ TEST(FilePersistentMemoryAllocatorTest, ExtendTest) {
     FilePersistentMemoryAllocator allocator(
         std::move(mmfile), region.size, 0, "",
         FilePersistentMemoryAllocator::kReadWrite);
-    EXPECT_EQ(static_cast<size_t>(before_size), allocator.used());
+    EXPECT_EQ(static_cast<size_t>(before_size.value()), allocator.used());
 
     allocator.Allocate(111, 111);
-    EXPECT_LT(static_cast<size_t>(before_size), allocator.used());
+    EXPECT_LT(static_cast<size_t>(before_size.value()), allocator.used());
   }
 
   // Validate that append worked.
-  int64_t after_size;
-  ASSERT_TRUE(GetFileSize(file_path, &after_size));
-  EXPECT_LT(before_size, after_size);
+  std::optional<int64_t> after_size = GetFileSize(file_path);
+  ASSERT_TRUE(after_size.has_value());
+  EXPECT_LT(before_size.value(), after_size.value());
 
   // Verify that it's still an acceptable file.
   {
@@ -1078,9 +1079,9 @@ TEST_F(PersistentMemoryAllocatorTest, TruncateTest) {
     }
 
     // Ensure that file length was not adjusted.
-    int64_t actual_length;
-    ASSERT_TRUE(GetFileSize(file_path, &actual_length));
-    EXPECT_EQ(file_length, static_cast<size_t>(actual_length));
+    std::optional<int64_t> actual_length = GetFileSize(file_path);
+    ASSERT_TRUE(actual_length.has_value());
+    EXPECT_EQ(file_length, static_cast<size_t>(actual_length.value()));
   }
 }
 

content/public/test/resource_load_observer.cc

@@ -4,6 +4,7 @@
 
 #include "content/public/test/resource_load_observer.h"
 
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -62,13 +63,14 @@ void ResourceLoadObserver::CheckResourceLoaded(
     }
 
     resource_load_info_found = true;
-    int64_t file_size = -1;
+    std::optional<int64_t> file_size;
     if (!served_file_name.empty()) {
       base::ScopedAllowBlockingForTesting allow_blocking;
       base::FilePath test_dir;
       ASSERT_TRUE(base::PathService::Get(content::DIR_TEST_DATA, &test_dir));
       base::FilePath served_file = test_dir.Append(served_file_name);
-      ASSERT_TRUE(GetFileSize(served_file, &file_size));
+      file_size = base::GetFileSize(served_file);
+      ASSERT_TRUE(file_size.has_value());
     }
     EXPECT_EQ(referrer, resource_load_info->referrer);
     EXPECT_EQ(load_method, resource_load_info->method);
@@ -97,9 +99,9 @@ void ResourceLoadObserver::CheckResourceLoaded(
       CheckTime(timing.connect_timing.connect_start);
       CheckTime(timing.connect_timing.connect_end);
     }
-    if (file_size != -1) {
-      EXPECT_EQ(file_size, resource_load_info->raw_body_bytes);
-      EXPECT_LT(file_size, resource_load_info->total_received_bytes);
+    if (file_size.has_value()) {
+      EXPECT_EQ(file_size.value(), resource_load_info->raw_body_bytes);
+      EXPECT_LT(file_size.value(), resource_load_info->total_received_bytes);
     }
   }
   EXPECT_TRUE(resource_load_info_found);

google_apis/gcm/engine/gcm_store_impl.cc

@@ -4,6 +4,7 @@
 
 #include "google_apis/gcm/engine/gcm_store_impl.h"
 
+#include <optional>
 #include <string_view>
 #include <utility>
 
@@ -381,10 +382,10 @@ void GCMStoreImpl::Backend::Load(StoreOpenMode open_mode,
 
   // Only record histograms if GCM had already been set up for this device.
   if (result->device_android_id != 0 && result->device_security_token != 0) {
-    int64_t file_size = 0;
-    if (base::GetFileSize(path_, &file_size)) {
+    std::optional<int64_t> file_size = base::GetFileSize(path_);
+    if (file_size.has_value()) {
       UMA_HISTOGRAM_COUNTS_1M("GCM.StoreSizeKB",
-                              static_cast<int>(file_size / 1024));
+                              static_cast<int>(file_size.value() / 1024));
     }
   }
 

ios/chrome/browser/reading_list/model/url_downloader.mm

@@ -96,8 +96,7 @@ std::pair<URLDownloader::SuccessState, int64_t> SavePDFFile(
                                                              path);
 
     if (base::Move(temporary_path, absolute_path)) {
-      int64_t pdf_file_size;
-      base::GetFileSize(absolute_path, &pdf_file_size);
+      int64_t pdf_file_size = base::GetFileSize(absolute_path).value_or(0);
       return {URLDownloader::DOWNLOAD_SUCCESS, pdf_file_size};
     } else {
       return {URLDownloader::ERROR, 0};

ios/web/download/download_native_task_bridge.mm

@@ -4,6 +4,8 @@
 
 #import "ios/web/download/download_native_task_bridge.h"
 
+#import <optional>
+
 #import "base/apple/foundation_util.h"
 #import "base/check.h"
 #import "base/files/file_util.h"
@@ -17,11 +19,7 @@ namespace {
 
 // Helper to get the size of file at `file_path`. Returns -1 in case of error.
 int64_t FileSizeForFileAtPath(base::FilePath file_path) {
-  int64_t file_size = 0;
-  if (!base::GetFileSize(file_path, &file_size))
-    return -1;
-
-  return file_size;
+  return base::GetFileSize(file_path).value_or(-1);
 }
 
 // Helper to invoke the download complete callback after getting the file

media/base/android/media_codec_bridge_impl_unittest.cc

@@ -475,7 +475,7 @@ TEST(MediaCodecBridgeTest, H264VideoEncodeAndValidate) {
   const char kSrcFileName[] = "bali_640x360_P420.yuv";
   base::FilePath src_file = GetTestDataFilePath(kSrcFileName);
   std::optional<int64_t> src_file_size = base::GetFileSize(src_file);
-  ASSERT_TRUE(src_file_size);
+  ASSERT_TRUE(src_file_size.has_value());
 
   const VideoPixelFormat kInputFormat = PIXEL_FORMAT_I420;
   const int frame_size = static_cast<int>(

media/base/test_data_util.cc

@@ -236,7 +236,7 @@ scoped_refptr<DecoderBuffer> ReadTestDataFile(std::string_view name) {
   base::FilePath file_path = GetTestDataFilePath(name);
 
   std::optional<int64_t> tmp = base::GetFileSize(file_path);
-  CHECK(tmp) << "Failed to get file size for '" << name << "'";
+  CHECK(tmp.has_value()) << "Failed to get file size for '" << name << "'";
 
   int file_size = base::checked_cast<int>(tmp.value());
 

media/filters/hls_test_helpers.cc

@@ -55,7 +55,8 @@ FileHlsDataSourceStreamFactory::CreateStream(std::string filename,
                                              bool taint_origin) {
   base::FilePath file_path = GetTestDataFilePath(filename);
   std::optional<int64_t> file_size = base::GetFileSize(file_path);
-  CHECK(file_size) << "Failed to get file size for '" << filename << "'";
+  CHECK(file_size.has_value())
+      << "Failed to get file size for '" << filename << "'";
   HlsDataSourceProvider::SegmentQueue segments;
   auto stream = std::make_unique<HlsDataSourceStream>(
       HlsDataSourceStream::StreamId::FromUnsafeValue(42), std::move(segments),

media/webrtc/audio_processor_test.cc

@@ -72,7 +72,7 @@ void ReadDataFromSpeechFile(char* data, int length) {
              .Append(FILE_PATH_LITERAL("speech_16b_stereo_48kHz.raw"));
   DCHECK(base::PathExists(file));
   std::optional<int64_t> data_file_size64 = base::GetFileSize(file);
-  DCHECK(data_file_size64);
+  DCHECK(data_file_size64.has_value());
   EXPECT_EQ(length, base::ReadFile(file, data, length));
   DCHECK(data_file_size64.value() > length);
 }

printing/emf_win_unittest.cc

@@ -10,6 +10,7 @@
 #include <winspool.h>
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
 
@@ -199,9 +200,9 @@ TEST(EmfTest, FileBackedEmf) {
     EXPECT_TRUE(emf.GetDataAsVector(&data));
     EXPECT_EQ(data.size(), size);
   }
-  int64_t file_size = 0;
-  base::GetFileSize(metafile_path, &file_size);
-  EXPECT_EQ(size, file_size);
+  std::optional<int64_t> file_size = base::GetFileSize(metafile_path);
+  ASSERT_TRUE(file_size.has_value());
+  EXPECT_EQ(size, file_size.value());
 
   // Playback the data.
   HDC hdc = CreateCompatibleDC(nullptr);

services/network/url_loader_unittest.cc

@@ -7607,9 +7607,9 @@ TEST_F(URLLoaderTest, CookieSettingOverridesCopiedToURLRequest) {
 TEST_F(URLLoaderTest, ReadAndDiscardBody) {
   const std::string file = "simple_page.html";
   const GURL url = test_server()->GetURL("/" + file);
-  int64_t actual_size = 0;
-  bool got_file_size = base::GetFileSize(GetTestFilePath(file), &actual_size);
-  ASSERT_TRUE(got_file_size);
+  std::optional<int64_t> file_size = base::GetFileSize(GetTestFilePath(file));
+  ASSERT_TRUE(file_size.has_value());
+  int64_t actual_size = file_size.value();
 
   TestURLLoaderClient loader_client;
   ResourceRequest request = CreateResourceRequest("GET", url);