Bug 1732343 - Part 9: Directly use DuplicateHandle in RemoteBackbuffer, r=cmartin
☠☠ backed out by 0d855795667b ☠ ☠
authorNika Layzell <nika@thelayzells.com>
Thu, 04 Nov 2021 19:20:20 +0000
changeset 598310 d30fa1e1f60556ba98bb7fe17c4cdefeb75727b1
parent 598309 ed0b4f757c4bd31cf3e3b8b1c9105aa3f32e9b17
child 598311 bba94c79f3e1bbd7ad158d362cf0df252c9f3e39
push id38950
push userctuns@mozilla.com
push dateFri, 05 Nov 2021 09:34:21 +0000
treeherdermozilla-central@7e8e3747c3f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscmartin
bugs1732343
milestone96.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1732343 - Part 9: Directly use DuplicateHandle in RemoteBackbuffer, r=cmartin Previously this code was using ipc::DuplicateHandle, which was unnecessary, as the code is being run in the parent process which has access to perform direct handle transfers. After this patch we'd like to remove ipc::DuplicateHandle and rely only on attached handles for serialization, so the code is converted to instead directly use DuplicateHandle. Differential Revision: https://phabricator.services.mozilla.com/D126571
widget/windows/RemoteBackbuffer.cpp
widget/windows/RemoteBackbuffer.h
--- a/widget/windows/RemoteBackbuffer.cpp
+++ b/widget/windows/RemoteBackbuffer.cpp
@@ -144,22 +144,23 @@ class SharedImage {
     bitmapInfo.bmiHeader.biBitCount = 32;
     bitmapInfo.bmiHeader.biCompression = BI_RGB;
     void* dummy = nullptr;
     return ::CreateDIBSection(nullptr /*paletteDC*/, &bitmapInfo,
                               DIB_RGB_COLORS, &dummy, mFileMapping,
                               0 /*offset*/);
   }
 
-  HANDLE CreateRemoteFileMapping(DWORD aTargetProcessId) {
-    MOZ_ASSERT(aTargetProcessId);
+  HANDLE CreateRemoteFileMapping(HANDLE aTargetProcess) {
+    MOZ_ASSERT(aTargetProcess);
 
     HANDLE fileMapping = nullptr;
-    if (!ipc::DuplicateHandle(mFileMapping, aTargetProcessId, &fileMapping,
-                              0 /*desiredAccess*/, DUPLICATE_SAME_ACCESS)) {
+    if (!::DuplicateHandle(GetCurrentProcess(), mFileMapping, aTargetProcess,
+                           &fileMapping, 0 /*desiredAccess*/,
+                           FALSE /*inheritHandle*/, DUPLICATE_SAME_ACCESS)) {
       return nullptr;
     }
     return fileMapping;
   }
 
   already_AddRefed<gfx::DrawTarget> CreateDrawTarget() {
     return gfx::Factory::CreateDrawTargetForData(
         gfx::BackendType::CAIRO, mPixelData, IntSize(mWidth, mHeight),
@@ -309,18 +310,18 @@ class PresentableSharedImage {
       }
     }
 
     MOZ_ALWAYS_TRUE(::ReleaseDC(aWindowHandle, windowDC));
 
     return result;
   }
 
-  HANDLE CreateRemoteFileMapping(DWORD aTargetProcessId) {
-    return mSharedImage.CreateRemoteFileMapping(aTargetProcessId);
+  HANDLE CreateRemoteFileMapping(HANDLE aTargetProcess) {
+    return mSharedImage.CreateRemoteFileMapping(aTargetProcess);
   }
 
   already_AddRefed<gfx::DrawTarget> CreateDrawTarget() {
     return mSharedImage.CreateDrawTarget();
   }
 
   void CopyPixelsFrom(const PresentableSharedImage& other) {
     mSharedImage.CopyPixelsFrom(other.mSharedImage);
@@ -339,17 +340,17 @@ class PresentableSharedImage {
   SharedImage mSharedImage;
   HDC mDeviceContext;
   HBITMAP mDIBSection;
   HGDIOBJ mSavedObject;
 };
 
 Provider::Provider()
     : mWindowHandle(nullptr),
-      mTargetProcessId(0),
+      mTargetProcess(nullptr),
       mFileMapping(nullptr),
       mRequestReadyEvent(nullptr),
       mResponseReadyEvent(nullptr),
       mSharedDataPtr(nullptr),
       mStopServiceThread(false),
       mServiceThread(nullptr),
       mBackbuffer() {}
 
@@ -372,25 +373,34 @@ Provider::~Provider() {
 
   if (mRequestReadyEvent) {
     MOZ_ALWAYS_TRUE(::CloseHandle(mRequestReadyEvent));
   }
 
   if (mFileMapping) {
     MOZ_ALWAYS_TRUE(::CloseHandle(mFileMapping));
   }
+
+  if (mTargetProcess) {
+    MOZ_ALWAYS_TRUE(::CloseHandle(mTargetProcess));
+  }
 }
 
 bool Provider::Initialize(HWND aWindowHandle, DWORD aTargetProcessId,
                           nsTransparencyMode aTransparencyMode) {
   MOZ_ASSERT(aWindowHandle);
   MOZ_ASSERT(aTargetProcessId);
 
   mWindowHandle = aWindowHandle;
-  mTargetProcessId = aTargetProcessId;
+
+  mTargetProcess = ::OpenProcess(PROCESS_DUP_HANDLE, FALSE /*inheritHandle*/,
+                                 aTargetProcessId);
+  if (!mTargetProcess) {
+    return false;
+  }
 
   mFileMapping = ::CreateFileMappingW(
       INVALID_HANDLE_VALUE, nullptr /*secattr*/, PAGE_READWRITE, 0 /*sizeHigh*/,
       static_cast<DWORD>(sizeof(SharedData)), nullptr /*name*/);
   if (!mFileMapping) {
     return false;
   }
 
@@ -527,17 +537,17 @@ void Provider::HandleBorrowRequest(Borro
 
   // Preserve the contents of the old backbuffer (if it exists)
   if (mBackbuffer) {
     newBackbuffer->CopyPixelsFrom(*mBackbuffer);
     mBackbuffer.reset();
   }
 
   HANDLE remoteFileMapping =
-      newBackbuffer->CreateRemoteFileMapping(mTargetProcessId);
+      newBackbuffer->CreateRemoteFileMapping(mTargetProcess);
   if (!remoteFileMapping) {
     return;
   }
 
   aResponseData->result = ResponseResult::BorrowSuccess;
   aResponseData->width = width;
   aResponseData->height = height;
   aResponseData->fileMapping = remoteFileMapping;
--- a/widget/windows/RemoteBackbuffer.h
+++ b/widget/windows/RemoteBackbuffer.h
@@ -45,17 +45,17 @@ class Provider {
   void ThreadMain();
 
   void HandleBorrowRequest(BorrowResponseData* aResponseData,
                            bool aAllowSameBuffer);
   void HandlePresentRequest(const PresentRequestData& aRequestData,
                             PresentResponseData* aResponseData);
 
   HWND mWindowHandle;
-  DWORD mTargetProcessId;
+  HANDLE mTargetProcess;
   HANDLE mFileMapping;
   HANDLE mRequestReadyEvent;
   HANDLE mResponseReadyEvent;
   SharedData* mSharedDataPtr;
   bool mStopServiceThread;
   PRThread* mServiceThread;
   std::unique_ptr<PresentableSharedImage> mBackbuffer;
   mozilla::Atomic<nsTransparencyMode, MemoryOrdering::Relaxed>