Bug 1264566 - Part 2: Refactor all usage of FileDescriptor. r=valentin

Callers should use a UniquePtr to hold the platform handle.

MozReview-Commit-ID: 6BWnyAf4b3a

--HG--
extra : transplant_source : %26%CA%0D%28%08%9BT%97Z%A1%3Dq%CD%21%A1_%EFE%83%0E
extra : histedit_source : 77f8ed3d0fdec6cce0c95469130ade0fb547bb91

devtools/shared/heapsnapshot/FileDescriptorOutputStream.cpp

@@ -15,7 +15,8 @@ FileDescriptorOutputStream::Create(const ipc::FileDescriptor& fileDescriptor)
   if (NS_WARN_IF(!fileDescriptor.IsValid()))
     return nullptr;
 
-  PRFileDesc* prfd = PR_ImportFile(PROsfd(fileDescriptor.PlatformHandle()));
+  auto rawFD = fileDescriptor.ClonePlatformHandle();
+  PRFileDesc* prfd = PR_ImportFile(PROsfd(rawFD.release()));
   if (NS_WARN_IF(!prfd))
     return nullptr;
 

dom/asmjscache/AsmJSCache.cpp

@@ -1310,7 +1310,8 @@ private:
 
     mFileSize = aFileSize;
 
-    mFileDesc = PR_ImportFile(PROsfd(aFileDesc.PlatformHandle()));
+    auto rawFD = aFileDesc.ClonePlatformHandle();
+    mFileDesc = PR_ImportFile(PROsfd(rawFD.release()));
     if (!mFileDesc) {
       return false;
     }

dom/camera/GonkCameraControl.cpp

@@ -1252,15 +1252,15 @@ nsGonkCameraControl::StartRecordingImpl(DeviceStorageFileDescriptor* aFileDescri
     closer = new CloseFileRunnable(aFileDescriptor->mFileDescriptor);
   }
   nsresult rv;
-  int fd = aFileDescriptor->mFileDescriptor.PlatformHandle();
+  auto rawFD = aFileDescriptor->mFileDescriptor.ClonePlatformHandle();
   if (aOptions) {
-    rv = SetupRecording(fd, aOptions->rotation, aOptions->maxFileSizeBytes,
+    rv = SetupRecording(rawFD.get(), aOptions->rotation, aOptions->maxFileSizeBytes,
                         aOptions->maxVideoLengthMs);
     if (NS_SUCCEEDED(rv)) {
       rv = SetupRecordingFlash(aOptions->autoEnableLowLightTorch);
     }
   } else {
-    rv = SetupRecording(fd, 0, 0, 0);
+    rv = SetupRecording(rawFD.get(), 0, 0, 0);
   }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;

dom/ipc/ContentChild.cpp

@@ -1422,7 +1422,8 @@ ContentChild::RecvSetProcessSandbox(const MaybeFileDesc& aBroker)
 #endif
   int brokerFd = -1;
   if (aBroker.type() == MaybeFileDesc::TFileDescriptor) {
-      brokerFd = aBroker.get_FileDescriptor().PlatformHandle();
+      auto fd = aBroker.get_FileDescriptor().ClonePlatformHandle();
+      brokerFd = fd.release();
       // brokerFd < 0 means to allow direct filesystem access, so
       // make absolutely sure that doesn't happen if the parent
       // didn't intend it.

dom/ipc/ContentParent.cpp

@@ -4922,9 +4922,9 @@ ContentParent::RecvBackUpXResources(const FileDescriptor& aXSocketFd)
 #else
   MOZ_ASSERT(0 > mChildXSocketFdDup.get(),
              "Already backed up X resources??");
-  mChildXSocketFdDup.forget();
   if (aXSocketFd.IsValid()) {
-    mChildXSocketFdDup.reset(aXSocketFd.PlatformHandle());
+    auto rawFD = aXSocketFd.ClonePlatformHandle();
+    mChildXSocketFdDup.reset(rawFD.release());
   }
 #endif
   return true;

dom/plugins/ipc/PluginModuleParent.cpp

@@ -1914,9 +1914,9 @@ PluginModuleParent::RecvBackUpXResources(const FileDescriptor& aXSocketFd)
 #else
     MOZ_ASSERT(0 > mPluginXSocketFdDup.get(),
                "Already backed up X resources??");
-    mPluginXSocketFdDup.forget();
     if (aXSocketFd.IsValid()) {
-      mPluginXSocketFdDup.reset(aXSocketFd.PlatformHandle());
+      auto rawFD = aXSocketFd.ClonePlatformHandle();
+      mPluginXSocketFdDup.reset(rawFD.release());
     }
 #endif
     return true;

ipc/glue/FileDescriptorUtils.cpp

@@ -62,19 +62,7 @@ CloseFileRunnable::CloseFile()
 {
   // It's possible for this to happen on the main thread if the dispatch to the
   // stream service fails so we can't assert the thread on which we're running.
-
-  MOZ_ASSERT(mFileDescriptor.IsValid());
-
-  PRFileDesc* fd =
-    PR_ImportFile(PROsfd(mFileDescriptor.PlatformHandle()));
-  NS_WARN_IF_FALSE(fd, "Failed to import file handle!");
-
   mFileDescriptor = FileDescriptor();
-
-  if (fd) {
-    PR_Close(fd);
-    fd = nullptr;
-  }
 }
 
 NS_IMETHODIMP
@@ -93,21 +81,19 @@ FILE*
 FileDescriptorToFILE(const FileDescriptor& aDesc,
                      const char* aOpenMode)
 {
-  // Debug builds check whether the handle was "used", even if it's
-  // invalid, so that needs to happen first.
-  FileDescriptor::PlatformHandleType handle = aDesc.PlatformHandle();
   if (!aDesc.IsValid()) {
     errno = EBADF;
     return nullptr;
   }
+  auto handle = aDesc.ClonePlatformHandle();
 #ifdef XP_WIN
-  int fd = _open_osfhandle(reinterpret_cast<intptr_t>(handle), 0);
+  int fd = _open_osfhandle(static_cast<intptr_t>(handle.get()), 0);
   if (fd == -1) {
-    CloseHandle(handle);
     return nullptr;
   }
+  Unused << handle.release();
 #else
-  int fd = handle;
+  int fd = handle.release();
 #endif
   FILE* file = fdopen(fd, aOpenMode);
   if (!file) {

ipc/glue/FileDescriptorUtils.h

@@ -44,8 +44,8 @@ private:
   void CloseFile();
 };
 
-// On failure, FileDescriptorToFILE closes the given descriptor; on
-// success, fclose()ing the returned FILE* will close the handle.
+// On failure, FileDescriptorToFILE returns nullptr; on success,
+// returns duplicated FILE*.
 // This is meant for use with FileDescriptors received over IPC.
 FILE* FileDescriptorToFILE(const FileDescriptor& aDesc,
                            const char* aOpenMode);

ipc/glue/Transport_posix.cpp

@@ -60,7 +60,8 @@ OpenDescriptor(const TransportDescriptor& aTd, Transport::Mode aMode)
 UniquePtr<Transport>
 OpenDescriptor(const FileDescriptor& aFd, Transport::Mode aMode)
 {
-  return MakeUnique<Transport>(aFd.PlatformHandle(), aMode, nullptr);
+  auto rawFD = aFd.ClonePlatformHandle();
+  return MakeUnique<Transport>(rawFD.release(), aMode, nullptr);
 }
 
 TransportDescriptor

netwerk/base/nsFileStreams.cpp

@@ -635,7 +635,8 @@ nsFileInputStream::Deserialize(const InputStreamParams& aParams,
     }
 
     if (fd.IsValid()) {
-        PRFileDesc* fileDesc = PR_ImportFile(PROsfd(fd.PlatformHandle()));
+        auto rawFD = fd.ClonePlatformHandle();
+        PRFileDesc* fileDesc = PR_ImportFile(PROsfd(rawFD.release()));
         if (!fileDesc) {
             NS_WARNING("Failed to import file handle!");
             return false;

netwerk/ipc/RemoteOpenFileChild.cpp

@@ -294,7 +294,8 @@ RemoteOpenFileChild::HandleFileDescriptorAndNotifyListener(
   }
 
   if (aFD.IsValid()) {
-    mNSPRFileDesc = PR_ImportFile(aFD.PlatformHandle());
+    auto rawFD = aFD.ClonePlatformHandle();
+    mNSPRFileDesc = PR_ImportFile(rawFD.release());
     if (!mNSPRFileDesc) {
       NS_WARNING("Failed to import file handle!");
     }

security/sandbox/linux/gtest/TestBroker.cpp

@@ -67,7 +67,8 @@ protected:
     mServer = SandboxBroker::Create(GetPolicy(), getpid(), fd);
     ASSERT_NE(mServer, nullptr);
     ASSERT_TRUE(fd.IsValid());
-    mClient.reset(new SandboxBrokerClient(dup(fd.PlatformHandle())));
+    auto rawFD = fd.ClonePlatformHandle();
+    mClient.reset(new SandboxBrokerClient(rawFD.release()));
   }
 
   template<class C, void (C::* Main)()>

xpcom/io/nsAnonymousTemporaryFile.cpp

@@ -126,8 +126,8 @@ NS_OpenAnonymousTemporaryFile(PRFileDesc** aOutFileDesc)
       MOZ_ASSERT(NS_FAILED(rv));
       return rv;
     }
-    *aOutFileDesc =
-      PR_ImportFile(PROsfd(fd.get_FileDescriptor().PlatformHandle()));
+    auto rawFD = fd.get_FileDescriptor().ClonePlatformHandle();
+    *aOutFileDesc = PR_ImportFile(PROsfd(rawFD.release()));
     return NS_OK;
   }