Implement ScopedFD in terms of ScopedGeneric.

Move to a new file base/files/scoped_file.h. I will also add ScopedFILE to here (currently in file_util.h) later.

I think there is a crash in the old code in content/browser/zygote_host/zygote_host_impl_linux.cc that this patch should fix. The old ScopedFD took the address of something in a vector that is being modified.

I removed SafeScopedFD from content/common/sandbox_linux/sandbox_linux.cc since base's ScopedFD not CHECKs on close failure (this is a more recent addition).

Reland of https://codereview.chromium.org/191673003/
R=agl, viettrungluu

Review URL: https://codereview.chromium.org/202113004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257473 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 51 insertions, 51 deletions

diff --git a/chrome/browser/chromeos/system/automatic_reboot_manager.cc b/chrome/browser/chromeos/system/automatic_reboot_manager.cc
index 1e00316..2ca1409 100644
--- a/chrome/browser/chromeos/system/automatic_reboot_manager.cc
+++ b/chrome/browser/chromeos/system/automatic_reboot_manager.cc
@@ -18,6 +18,7 @@
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
@@ -56,15 +57,15 @@ const int kOneKilobyte = 1 << 10;                  // 1 kB in bytes.
 
 base::TimeDelta ReadTimeDeltaFromFile(const base::FilePath& path) {
   base::ThreadRestrictions::AssertIOAllowed();
-  int fd = HANDLE_EINTR(open(path.value().c_str(), O_RDONLY | O_NOFOLLOW));
-  if (fd < 0)
+  base::ScopedFD fd(
+      HANDLE_EINTR(open(path.value().c_str(), O_RDONLY | O_NOFOLLOW)));
+  if (!fd.is_valid())
     return base::TimeDelta();
-  file_util::ScopedFD fd_closer(&fd);
 
   std::string contents;
   char buffer[kOneKilobyte];
   ssize_t length;
-  while ((length = read(fd, buffer, sizeof(buffer))) > 0)
+  while ((length = read(fd.get(), buffer, sizeof(buffer))) > 0)
     contents.append(buffer, length);
 
   double seconds;
@@ -108,16 +109,16 @@ void SaveUpdateRebootNeededUptime() {
   if (uptime == kZeroTimeDelta)
     return;
 
-  int fd = HANDLE_EINTR(open(update_reboot_needed_uptime_file.value().c_str(),
-                             O_CREAT | O_WRONLY | O_TRUNC | O_NOFOLLOW,
-                             0666));
-  if (fd < 0)
+  base::ScopedFD fd(HANDLE_EINTR(
+      open(update_reboot_needed_uptime_file.value().c_str(),
+           O_CREAT | O_WRONLY | O_TRUNC | O_NOFOLLOW,
+           0666)));
+  if (!fd.is_valid())
     return;
-  file_util::ScopedFD fd_closer(&fd);
 
   std::string update_reboot_needed_uptime =
       base::DoubleToString(uptime.InSecondsF());
-  base::WriteFileDescriptor(fd, update_reboot_needed_uptime.c_str(),
+  base::WriteFileDescriptor(fd.get(), update_reboot_needed_uptime.c_str(),
                             update_reboot_needed_uptime.size());
 }
 
diff --git a/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc b/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc
index a57fb74..cf5b80b 100644
--- a/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc
+++ b/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc
@@ -5,6 +5,7 @@
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/json/json_reader.h"
 #include "base/memory/scoped_ptr.h"
@@ -214,8 +215,8 @@ TEST_F(NativeMessagingTest, SingleSendMessageWrite) {
 #else  // defined(OS_WIN)
   base::PlatformFile pipe_handles[2];
   ASSERT_EQ(0, pipe(pipe_handles));
-  file_util::ScopedFD read_fd(pipe_handles);
-  file_util::ScopedFD write_fd(pipe_handles + 1);
+  base::ScopedFD read_fd(pipe_handles[0]);
+  base::ScopedFD write_fd(pipe_handles[1]);
 
   read_file = pipe_handles[0];
 #endif  // !defined(OS_WIN)
diff --git a/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc b/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc
index f4523fc..70353d3 100644
--- a/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc
+++ b/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc
@@ -6,6 +6,7 @@
 
 #include "base/command_line.h"
 #include "base/file_util.h"
+#include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/posix/eintr_wrapper.h"
@@ -58,18 +59,18 @@ bool NativeProcessLauncher::LaunchNativeProcess(
     LOG(ERROR) << "Bad read pipe";
     return false;
   }
-  file_util::ScopedFD read_pipe_read_fd(&read_pipe_fds[0]);
-  file_util::ScopedFD read_pipe_write_fd(&read_pipe_fds[1]);
-  fd_map.push_back(std::make_pair(*read_pipe_write_fd, STDOUT_FILENO));
+  base::ScopedFD read_pipe_read_fd(read_pipe_fds[0]);
+  base::ScopedFD read_pipe_write_fd(read_pipe_fds[1]);
+  fd_map.push_back(std::make_pair(read_pipe_write_fd.get(), STDOUT_FILENO));
 
   int write_pipe_fds[2] = {0};
   if (HANDLE_EINTR(pipe(write_pipe_fds)) != 0) {
     LOG(ERROR) << "Bad write pipe";
     return false;
   }
-  file_util::ScopedFD write_pipe_read_fd(&write_pipe_fds[0]);
-  file_util::ScopedFD write_pipe_write_fd(&write_pipe_fds[1]);
-  fd_map.push_back(std::make_pair(*write_pipe_read_fd, STDIN_FILENO));
+  base::ScopedFD write_pipe_read_fd(write_pipe_fds[0]);
+  base::ScopedFD write_pipe_write_fd(write_pipe_fds[1]);
+  fd_map.push_back(std::make_pair(write_pipe_read_fd.get(), STDIN_FILENO));
 
   base::LaunchOptions options;
   options.fds_to_remap = &fd_map;
@@ -82,8 +83,8 @@ bool NativeProcessLauncher::LaunchNativeProcess(
   write_pipe_read_fd.reset();
   read_pipe_write_fd.reset();
 
-  *read_file = *read_pipe_read_fd.release();
-  *write_file = *write_pipe_write_fd.release();
+  *read_file = read_pipe_read_fd.release();
+  *write_file = write_pipe_write_fd.release();
 
   return true;
 }
diff --git a/chrome/browser/mac/relauncher.cc b/chrome/browser/mac/relauncher.cc
index 5d8357f..e659793 100644
--- a/chrome/browser/mac/relauncher.cc
+++ b/chrome/browser/mac/relauncher.cc
@@ -19,6 +19,7 @@
 
 #include "base/basictypes.h"
 #include "base/file_util.h"
+#include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/mac/mac_logging.h"
 #include "base/mac/mac_util.h"
@@ -132,8 +133,8 @@ bool RelaunchAppWithHelper(const std::string& helper,
   // write side of the pipe. In that process, the read side will be closed by
   // base::LaunchApp because it won't be present in fd_map, and the write side
   // will be remapped to kRelauncherSyncFD by fd_map.
-  file_util::ScopedFD pipe_read_fd(&pipe_fds[0]);
-  file_util::ScopedFD pipe_write_fd(&pipe_fds[1]);
+  base::ScopedFD pipe_read_fd(pipe_fds[0]);
+  base::ScopedFD pipe_write_fd(pipe_fds[1]);
 
   // Make sure kRelauncherSyncFD is a safe value. base::LaunchProcess will
   // preserve these three FDs in forked processes, so kRelauncherSyncFD should
@@ -144,7 +145,7 @@ bool RelaunchAppWithHelper(const std::string& helper,
                  kRelauncherSyncFD_must_not_conflict_with_stdio_fds);
 
   base::FileHandleMappingVector fd_map;
-  fd_map.push_back(std::make_pair(*pipe_write_fd, kRelauncherSyncFD));
+  fd_map.push_back(std::make_pair(pipe_write_fd.get(), kRelauncherSyncFD));
 
   base::LaunchOptions options;
   options.fds_to_remap = &fd_map;
@@ -160,7 +161,7 @@ bool RelaunchAppWithHelper(const std::string& helper,
 
   // Synchronize with the relauncher process.
   char read_char;
-  int read_result = HANDLE_EINTR(read(*pipe_read_fd, &read_char, 1));
+  int read_result = HANDLE_EINTR(read(pipe_read_fd.get(), &read_char, 1));
   if (read_result != 1) {
     if (read_result < 0) {
       PLOG(ERROR) << "read";
@@ -185,9 +186,7 @@ namespace {
 // situations, it can be assumed that something went wrong with the parent
 // process and the best recovery approach is to attempt relaunch anyway.
 void RelauncherSynchronizeWithParent() {
-  // file_util::ScopedFD needs something non-const to operate on.
-  int relauncher_sync_fd = kRelauncherSyncFD;
-  file_util::ScopedFD relauncher_sync_fd_closer(&relauncher_sync_fd);
+  base::ScopedFD relauncher_sync_fd(kRelauncherSyncFD);
 
   int parent_pid = getppid();
 
@@ -201,22 +200,21 @@ void RelauncherSynchronizeWithParent() {
   }
 
   // Set up a kqueue to monitor the parent process for exit.
-  int kq = kqueue();
-  if (kq < 0) {
+  base::ScopedFD kq(kqueue());
+  if (!kq.is_valid()) {
     PLOG(ERROR) << "kqueue";
     return;
   }
-  file_util::ScopedFD kq_closer(&kq);
 
   struct kevent change = { 0 };
   EV_SET(&change, parent_pid, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL);
-  if (kevent(kq, &change, 1, NULL, 0, NULL) == -1) {
+  if (kevent(kq.get(), &change, 1, NULL, 0, NULL) == -1) {
     PLOG(ERROR) << "kevent (add)";
     return;
   }
 
   // Write a '\0' character to the pipe.
-  if (HANDLE_EINTR(write(relauncher_sync_fd, "", 1)) != 1) {
+  if (HANDLE_EINTR(write(relauncher_sync_fd.get(), "", 1)) != 1) {
     PLOG(ERROR) << "write";
     return;
   }
@@ -225,7 +223,7 @@ void RelauncherSynchronizeWithParent() {
   // write above to complete. The parent process is now free to exit. Wait for
   // that to happen.
   struct kevent event;
-  int events = kevent(kq, NULL, 0, &event, 1, NULL);
+  int events = kevent(kq.get(), NULL, 0, &event, 1, NULL);
   if (events != 1) {
     if (events < 0) {
       PLOG(ERROR) << "kevent (monitor)";
diff --git a/chrome/browser/process_singleton_mac_unittest.cc b/chrome/browser/process_singleton_mac_unittest.cc
index 4246c27..b6a5b7b 100644
--- a/chrome/browser/process_singleton_mac_unittest.cc
+++ b/chrome/browser/process_singleton_mac_unittest.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/process_singleton.h"
 
 #include "base/file_util.h"
+#include "base/files/scoped_file.h"
 #include "base/path_service.h"
 #include "base/posix/eintr_wrapper.h"
 #include "chrome/common/chrome_constants.h"
@@ -39,15 +40,13 @@ class ProcessSingletonMacTest : public PlatformTest {
   // Return |true| if the file exists and is locked.  Forces a failure
   // in the containing test in case of error condition.
   bool IsLocked() {
-    int fd = HANDLE_EINTR(open(lock_path_.value().c_str(), O_RDONLY));
-    if (fd == -1) {
+    base::ScopedFD fd(HANDLE_EINTR(open(lock_path_.value().c_str(), O_RDONLY)));
+    if (!fd.is_valid()) {
       EXPECT_EQ(ENOENT, errno) << "Unexpected error opening lockfile.";
       return false;
     }
 
-    file_util::ScopedFD auto_close(&fd);
-
-    int rc = HANDLE_EINTR(flock(fd, LOCK_EX|LOCK_NB));
+    int rc = HANDLE_EINTR(flock(fd.get(), LOCK_EX|LOCK_NB));
 
     // Got the lock, so it wasn't already locked.  Close releases.
     if (rc != -1)
diff --git a/chrome/test/automation/proxy_launcher.cc b/chrome/test/automation/proxy_launcher.cc
index 6171f28..e49c4d6 100644
--- a/chrome/test/automation/proxy_launcher.cc
+++ b/chrome/test/automation/proxy_launcher.cc
@@ -9,6 +9,7 @@
 #include "base/environment.h"
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
+#include "base/files/scoped_file.h"
 #include "base/process/kill.h"
 #include "base/process/launch.h"
 #include "base/strings/string_number_conversions.h"
@@ -489,12 +490,11 @@ bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state,
 #if defined(OS_WIN)
   options.start_hidden = !state.show_window;
 #elif defined(OS_POSIX)
-  int ipcfd = -1;
-  file_util::ScopedFD ipcfd_closer(&ipcfd);
+  base::ScopedFD ipcfd;
   base::FileHandleMappingVector fds;
   if (main_launch && automation_proxy_.get()) {
-    ipcfd = automation_proxy_->channel()->TakeClientFileDescriptor();
-    fds.push_back(std::make_pair(ipcfd,
+    ipcfd.reset(automation_proxy_->channel()->TakeClientFileDescriptor());
+    fds.push_back(std::make_pair(ipcfd.get(),
         kPrimaryIPCChannel + base::GlobalDescriptors::kBaseDescriptor));
     options.fds_to_remap = &fds;
   }
diff --git a/chrome/test/chromedriver/chrome_launcher.cc b/chrome/test/chromedriver/chrome_launcher.cc
index 44a4ad8..1ca47c3 100644
--- a/chrome/test/chromedriver/chrome_launcher.cc
+++ b/chrome/test/chromedriver/chrome_launcher.cc
@@ -12,6 +12,7 @@
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
 #include "base/format_macros.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
@@ -250,15 +251,14 @@ Status LaunchDesktopChrome(
 
 #if defined(OS_POSIX)
   base::FileHandleMappingVector no_stderr;
-  int devnull = -1;
-  file_util::ScopedFD scoped_devnull(&devnull);
+  base::ScopedFD devnull;
   if (!CommandLine::ForCurrentProcess()->HasSwitch("verbose")) {
     // Redirect stderr to /dev/null, so that Chrome log spew doesn't confuse
     // users.
-    devnull = open("/dev/null", O_WRONLY);
-    if (devnull == -1)
+    devnull.reset(open("/dev/null", O_WRONLY));
+    if (!devnull.is_valid())
       return Status(kUnknownError, "couldn't open /dev/null");
-    no_stderr.push_back(std::make_pair(devnull, STDERR_FILENO));
+    no_stderr.push_back(std::make_pair(devnull.get(), STDERR_FILENO));
     options.fds_to_remap = &no_stderr;
   }
 #endif
diff --git a/chrome/utility/importer/firefox_importer_unittest_utils_mac.cc b/chrome/utility/importer/firefox_importer_unittest_utils_mac.cc
index 6b21149..93961c5 100644
--- a/chrome/utility/importer/firefox_importer_unittest_utils_mac.cc
+++ b/chrome/utility/importer/firefox_importer_unittest_utils_mac.cc
@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
 #include "base/message_loop/message_loop.h"
 #include "base/posix/global_descriptors.h"
 #include "base/process/kill.h"
@@ -47,13 +48,12 @@ bool LaunchNSSDecrypterChildProcess(const base::FilePath& nss_path,
   base::LaunchOptions options;
   options.environ["DYLD_FALLBACK_LIBRARY_PATH"] = nss_path.value();
 
-  int ipcfd = channel->TakeClientFileDescriptor();
-  if (ipcfd == -1)
+  base::ScopedFD ipcfd(channel->TakeClientFileDescriptor());
+  if (!ipcfd.is_valid())
     return false;
 
-  file_util::ScopedFD client_file_descriptor_closer(&ipcfd);
   base::FileHandleMappingVector fds_to_map;
-  fds_to_map.push_back(std::pair<int,int>(ipcfd,
+  fds_to_map.push_back(std::pair<int,int>(ipcfd.get(),
       kPrimaryIPCChannel + base::GlobalDescriptors::kBaseDescriptor));
 
   bool debug_on_start = CommandLine::ForCurrentProcess()->HasSwitch(