Add base::File constructors that take ScopedPlatformFile.

In base::File, |file_| is a ScopedPlatformFile. So add constructors that
can take a ScopedPlatformFile directly, instead of having the caller
release the PlatformFile, and then rewrapping it.

Change-Id: Ie70cacc0a47263ff9ec5ce38f84032f1e469faa3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948195Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721736}

base/files/file.cc

@@ -3,6 +3,9 @@
 // found in the LICENSE file.
 
 #include "base/files/file.h"
+
+#include <utility>
+
 #include "base/files/file_path.h"
 #include "base/files/file_tracing.h"
 #include "base/metrics/histogram.h"
@@ -28,8 +31,18 @@ File::File(const FilePath& path, uint32_t flags) : error_details_(FILE_OK) {
 }
 #endif
 
+File::File(ScopedPlatformFile platform_file)
+    : File(std::move(platform_file), false) {}
+
 File::File(PlatformFile platform_file) : File(platform_file, false) {}
 
+File::File(ScopedPlatformFile platform_file, bool async)
+    : file_(std::move(platform_file)), error_details_(FILE_OK), async_(async) {
+#if defined(OS_POSIX) || defined(OS_FUCHSIA)
+  DCHECK_GE(file_.get(), -1);
+#endif
+}
+
 File::File(PlatformFile platform_file, bool async)
     : file_(platform_file),
       error_details_(FILE_OK),

base/files/file.h

@@ -154,11 +154,13 @@ class BASE_EXPORT File {
   File(const FilePath& path, uint32_t flags);
 
   // Takes ownership of |platform_file| and sets async to false.
+  explicit File(ScopedPlatformFile platform_file);
   explicit File(PlatformFile platform_file);
 
   // Takes ownership of |platform_file| and sets async to the given value.
   // This constructor exists because on Windows you can't check if platform_file
   // is async or not.
+  File(ScopedPlatformFile platform_file, bool async);
   File(PlatformFile platform_file, bool async);
 
   // Creates an object with a specific error_details code.

chrome/browser/chromeos/arc/print/arc_print_service.cc

@@ -633,7 +633,7 @@ void ArcPrintServiceImpl::Print(mojom::PrintJobInstancePtr instance,
   auto job = std::make_unique<PrintJobHostImpl>(
       mojo::MakeRequest(&host_proxy), std::move(instance), this,
       chromeos::CupsPrintJobManagerFactory::GetForBrowserContext(profile_),
-      std::move(settings), base::File(fd.release()), print_job->data_size);
+      std::move(settings), base::File(std::move(fd)), print_job->data_size);
   PrintJobHostImpl* job_raw = job.get();
   jobs_.emplace(job_raw, std::move(job));
   std::move(callback).Run(std::move(host_proxy));

chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc

@@ -101,8 +101,8 @@ bool NativeProcessLauncher::LaunchNativeProcess(
   read_pipe_write_fd.reset();
 
   *process = std::move(local_process);
-  *read_file = base::File(read_pipe_read_fd.release());
-  *write_file = base::File(write_pipe_write_fd.release());
+  *read_file = base::File(std::move(read_pipe_read_fd));
+  *write_file = base::File(std::move(write_pipe_write_fd));
 
   return true;
 }

chrome/browser/extensions/api/messaging/native_process_launcher_win.cc

@@ -171,9 +171,8 @@ bool NativeProcessLauncher::LaunchNativeProcess(
   }
 
   *process = std::move(cmd_process);
-  *read_file = base::File(stdout_pipe.Take(), true /* async */);
-  *write_file = base::File(stdin_pipe.Take(), true /* async */);
-
+  *read_file = base::File(std::move(stdout_pipe), true /* async */);
+  *write_file = base::File(std::move(stdin_pipe), true /* async */);
   return true;
 }
 

chrome/chrome_cleaner/parsers/shortcut_parser/target/lnk_parser.cc

@@ -7,6 +7,7 @@
 #include <windows.h>
 
 #include <memory>
+#include <utility>
 #include <vector>
 
 #include "base/files/file.h"
@@ -358,12 +359,13 @@ mojom::LnkParsingResult ParseLnk(base::win::ScopedHandle file_handle,
     return mojom::LnkParsingResult::INVALID_HANDLE;
   }
 
-  base::File lnk_file(file_handle.Take());
+  base::File lnk_file(std::move(file_handle));
   int64_t lnk_file_size = lnk_file.GetLength();
   if (lnk_file_size <= 0) {
     LOG(ERROR) << "Error getting file size";
     return mojom::LnkParsingResult::INVALID_LNK_FILE_SIZE;
-  } else if (lnk_file_size >= kMaximumFileSize) {
+  }
+  if (lnk_file_size >= kMaximumFileSize) {
     LOG(ERROR) << "Unexpectedly large file size: " << lnk_file_size;
     return mojom::LnkParsingResult::INVALID_LNK_FILE_SIZE;
   }

chrome/chrome_cleaner/parsers/shortcut_parser/target/lnk_parser_unittest.cc

@@ -142,7 +142,7 @@ class LnkParserTest : public testing::Test {
     }
 
     // Load the shortcut into memory to modify the LinkInfoFlags.
-    base::File local_lnk(local_lnk_handle.Take());
+    base::File local_lnk(std::move(local_lnk_handle));
     std::vector<BYTE> shortcut_buffer;
     if (!LoadShortcutIntoMemory(&local_lnk, &shortcut_buffer)) {
       LOG(ERROR) << "Error loading shortcut into memory";
@@ -190,7 +190,7 @@ class LnkParserTest : public testing::Test {
     }
 
     // Load the shortcut into memory to corrupt it.
-    base::File good_lnk_file(good_lnk_handle.Take());
+    base::File good_lnk_file(std::move(good_lnk_handle));
     std::vector<BYTE> shortcut_buffer;
     if (!LoadShortcutIntoMemory(&good_lnk_file, &shortcut_buffer)) {
       LOG(ERROR) << "Error loading shortcut into memory";
@@ -220,7 +220,7 @@ class LnkParserTest : public testing::Test {
     }
 
     // Load the shortcut into memory.
-    base::File good_lnk_file(good_lnk_handle.Take());
+    base::File good_lnk_file(std::move(good_lnk_handle));
     std::vector<BYTE> shortcut_buffer;
     if (!LoadShortcutIntoMemory(&good_lnk_file, &shortcut_buffer)) {
       LOG(ERROR) << "Error loading shortcut into memory";
@@ -278,7 +278,7 @@ class LnkParserTest : public testing::Test {
     }
 
     // Load the shortcut into memory.
-    base::File good_lnk_file(good_lnk_handle.Take());
+    base::File good_lnk_file(std::move(good_lnk_handle));
     std::vector<BYTE> shortcut_buffer;
     if (!LoadShortcutIntoMemory(&good_lnk_file, &shortcut_buffer)) {
       LOG(ERROR) << "Error loading shortcut into memory";

content/app/content_main_runner_impl.cc

@@ -212,7 +212,7 @@ void LoadV8SnapshotFile() {
   base::ScopedFD fd =
       file_descriptor_store.MaybeTakeFD(snapshot_data_descriptor, &region);
   if (fd.is_valid()) {
-    base::File file(fd.release());
+    base::File file(std::move(fd));
     gin::V8Initializer::LoadV8SnapshotFromFile(std::move(file), &region,
                                                kSnapshotType);
     return;

fuchsia/engine/browser/content_directory_loader_factory.cc

@@ -7,6 +7,7 @@
 #include <lib/fdio/directory.h>
 #include <lib/fdio/fdio.h>
 
+#include <algorithm>
 #include <map>
 #include <memory>
 #include <utility>
@@ -181,7 +182,7 @@ class ContentDirectoryURLLoader : public network::mojom::URLLoader {
     }
 
     // Map the file into memory.
-    if (!mmap->Initialize(base::File(fd.release()),
+    if (!mmap->Initialize(base::File(std::move(fd)),
                           base::MemoryMappedFile::READ_ONLY)) {
       return false;
     }

mojo/public/cpp/platform/tests/platform_handle_unittest.cc

@@ -7,7 +7,6 @@
 #include "base/files/platform_file.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
-#include "base/macros.h"
 #include "base/memory/unsafe_shared_memory_region.h"
 #include "base/rand_util.h"
 #include "base/unguessable_token.h"
@@ -59,6 +58,8 @@ class PlatformHandleTest : public testing::Test,
                            public testing::WithParamInterface<HandleType> {
  public:
   PlatformHandleTest() = default;
+  PlatformHandleTest(const PlatformHandleTest&) = delete;
+  PlatformHandleTest& operator=(const PlatformHandleTest&) = delete;
 
   void SetUp() override {
     test_type_ = TestType::kFile;
@@ -86,11 +87,11 @@ class PlatformHandleTest : public testing::Test,
     if (test_type_ == TestType::kFile)
       return GetFileContents(handle);
 #if defined(OS_FUCHSIA) || (defined(OS_MACOSX) && !defined(OS_IOS))
-    else
-      return GetSharedMemoryContents(handle);
-#endif
+    return GetSharedMemoryContents(handle);
+#else
     NOTREACHED();
     return std::string();
+#endif
   }
 
  protected:
@@ -123,9 +124,10 @@ class PlatformHandleTest : public testing::Test,
 #if defined(OS_WIN)
     // We must temporarily release ownership of the handle due to how File
     // interacts with ScopedHandle.
-    base::File file(handle.TakeHandle().Take());
+    base::File file(handle.TakeHandle());
 #else
-    base::File file(handle.GetFD().get());
+    // Do the same as Windows for consistency, even though it is not necessary.
+    base::File file(handle.TakeFD());
 #endif
     std::vector<char> buffer(kTestData.size());
     file.Read(0, buffer.data(), static_cast<int>(buffer.size()));
@@ -135,7 +137,7 @@ class PlatformHandleTest : public testing::Test,
 #if defined(OS_WIN)
     handle = PlatformHandle(base::win::ScopedHandle(file.TakePlatformFile()));
 #else
-    ignore_result(file.TakePlatformFile());
+    handle = PlatformHandle(base::ScopedFD(file.TakePlatformFile()));
 #endif
 
     return contents;
@@ -194,8 +196,6 @@ class PlatformHandleTest : public testing::Test,
   // Needed to reconstitute a base::PlatformSharedMemoryRegion from an unwrapped
   // PlatformHandle.
   base::UnguessableToken shm_guid_;
-
-  DISALLOW_COPY_AND_ASSIGN(PlatformHandleTest);
 };
 
 TEST_P(PlatformHandleTest, BasicConstruction) {

remoting/host/win/elevated_native_messaging_host.cc

@@ -68,8 +68,8 @@ ProcessLaunchResult ElevatedNativeMessagingHost::EnsureElevatedHostCreated() {
 
   // Set up the native messaging channel to talk to the elevated host.
   // Note that input for the elevated channel is output for the elevated host.
-  elevated_channel_.reset(new PipeMessagingChannel(
-      base::File(read_handle.Take()), base::File(write_handle.Take())));
+  elevated_channel_ = std::make_unique<PipeMessagingChannel>(
+      base::File(std::move(read_handle)), base::File(std::move(write_handle)));
   elevated_channel_->Start(this);
 
   if (!host_process_timeout_.is_zero()) {

services/device/serial/serial_io_handler.cc

@@ -78,7 +78,7 @@ void SerialIoHandler::Open(const mojom::SerialConnectionOptions& options,
 void SerialIoHandler::OnPathOpened(
     scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner,
     base::ScopedFD fd) {
-  base::File file(fd.release());
+  base::File file(std::move(fd));
   io_thread_task_runner->PostTask(
       FROM_HERE,
       base::BindOnce(&SerialIoHandler::FinishOpen, this, std::move(file)));

services/device/usb/mojo/device_manager_impl.cc

@@ -145,7 +145,7 @@ void DeviceManagerImpl::OpenFileDescriptor(
 void DeviceManagerImpl::OnOpenFileDescriptor(
     OpenFileDescriptorCallback callback,
     base::ScopedFD fd) {
-  std::move(callback).Run(base::File(fd.release()));
+  std::move(callback).Run(base::File(std::move(fd)));
 }
 
 void DeviceManagerImpl::OnOpenFileDescriptorError(

ui/ozone/platform/drm/host/host_drm_device.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/run_loop.h"
@@ -218,7 +219,7 @@ bool HostDrmDevice::GpuAddGraphicsDeviceOnUIThread(const base::FilePath& path,
   DCHECK_CALLED_ON_VALID_THREAD(on_ui_thread_);
   if (!IsConnected())
     return false;
-  base::File file(fd.release());
+  base::File file(std::move(fd));
 
   drm_device_->AddGraphicsDevice(path, std::move(file));
 
@@ -229,7 +230,7 @@ void HostDrmDevice::GpuAddGraphicsDeviceOnIOThread(const base::FilePath& path,
                                                    base::ScopedFD fd) {
   DCHECK_CALLED_ON_VALID_THREAD(on_io_thread_);
   DCHECK(drm_device_on_io_thread_.is_bound());
-  base::File file(fd.release());
+  base::File file(std::move(fd));
   drm_device_on_io_thread_->AddGraphicsDevice(path, std::move(file));
 }