Bug 1781129 - Part 3: Remove Shmem overload of GetSurfaceData, r=edgar

After the previous changes there was only one consumer left of the Shmem version of GetSurfaceData, which could easily be changed to use BigBuffer, removing the need for that overload. After that consumer is removed, the interface was also simplified as the generic logic in the implementation was no longer necessary. Differential Revision: https://phabricator.services.mozilla.com/D151854
dothq · Aug 2, 2022 · 54f8850 · 54f8850
1 parent 2507c37
commit 54f8850
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 111 deletions.
diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp
@@ -204,7 +204,6 @@
 #include "mozilla/gfx/Types.h"
 #include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/ipc/SharedMemory.h"
-#include "mozilla/ipc/Shmem.h"
 #include "mozilla/net/UrlClassifierCommon.h"
 #include "mozilla/widget/IMEData.h"
 #include "nsAboutProtocolUtils.h"
@@ -8159,62 +8158,12 @@ void nsContentUtils::TransferableToIPCTransferable(
   }
 }
 
-namespace {
-// The default type used for calling GetSurfaceData(). Gets surface data as
-// raw bigbuffer.
-struct GetSurfaceDataBigBuffer {
-  using ReturnType = Maybe<BigBuffer>;
-  using BufferType = char*;
-
-  ReturnType Allocate(size_t aSize) { return Some(BigBuffer(aSize)); }
-
-  static BufferType GetBuffer(ReturnType& aReturnValue) {
-    return aReturnValue ? reinterpret_cast<char*>(aReturnValue->Data())
-                        : nullptr;
-  }
-
-  static ReturnType NullValue() { return ReturnType(); }
-};
-
-// The type used for calling GetSurfaceData() that allocates and writes to
-// a shared memory buffer.
-struct GetSurfaceDataShmem {
-  using ReturnType = Maybe<Shmem>;
-  using BufferType = char*;
-
-  explicit GetSurfaceDataShmem(IShmemAllocator* aAllocator)
-      : mAllocator(aAllocator) {}
-
-  ReturnType Allocate(size_t aSize) {
-    Shmem shmem;
-    if (!mAllocator->AllocShmem(aSize, &shmem)) {
-      return Nothing();
-    }
-
-    return Some(shmem);
-  }
-
-  static BufferType GetBuffer(const ReturnType& aReturnValue) {
-    return aReturnValue.isSome() ? aReturnValue.ref().get<char>() : nullptr;
-  }
-
-  static ReturnType NullValue() { return ReturnType(); }
-
- private:
-  IShmemAllocator* mAllocator;
-};
-
-/*
- * Get the pixel data from the given source surface and return it as a buffer.
- * The length and stride will be assigned from the surface.
- */
-template <typename GetSurfaceDataContext = GetSurfaceDataBigBuffer>
-typename GetSurfaceDataContext::ReturnType GetSurfaceDataImpl(
-    DataSourceSurface& aSurface, size_t* aLength, int32_t* aStride,
-    GetSurfaceDataContext aContext = GetSurfaceDataContext()) {
+Maybe<BigBuffer> nsContentUtils::GetSurfaceData(DataSourceSurface& aSurface,
+                                                size_t* aLength,
+                                                int32_t* aStride) {
   mozilla::gfx::DataSourceSurface::MappedSurface map;
   if (!aSurface.Map(mozilla::gfx::DataSourceSurface::MapType::READ, &map)) {
-    return GetSurfaceDataContext::NullValue();
+    return Nothing();
   }
 
   size_t bufLen = 0;
@@ -8224,38 +8173,18 @@ typename GetSurfaceDataContext::ReturnType GetSurfaceDataImpl(
       &bufLen);
   if (NS_FAILED(rv)) {
     aSurface.Unmap();
-    return GetSurfaceDataContext::NullValue();
+    return Nothing();
   }
 
-  // nsDependentCString wants null-terminated string.
-  typename GetSurfaceDataContext::ReturnType surfaceData =
-      aContext.Allocate(maxBufLen + 1);
-  if (GetSurfaceDataContext::GetBuffer(surfaceData)) {
-    memcpy(GetSurfaceDataContext::GetBuffer(surfaceData),
-           reinterpret_cast<char*>(map.mData), bufLen);
-    memset(GetSurfaceDataContext::GetBuffer(surfaceData) + bufLen, 0,
-           maxBufLen - bufLen + 1);
-  }
+  BigBuffer surfaceData(maxBufLen);
+  memcpy(surfaceData.Data(), map.mData, bufLen);
+  memset(surfaceData.Data() + bufLen, 0, maxBufLen - bufLen);
 
   *aLength = maxBufLen;
   *aStride = map.mStride;
 
   aSurface.Unmap();
-  return surfaceData;
-}
-}  // Anonymous namespace.
-
-Maybe<BigBuffer> nsContentUtils::GetSurfaceData(DataSourceSurface& aSurface,
-                                                size_t* aLength,
-                                                int32_t* aStride) {
-  return GetSurfaceDataImpl(aSurface, aLength, aStride);
-}
-
-Maybe<Shmem> nsContentUtils::GetSurfaceData(DataSourceSurface& aSurface,
-                                            size_t* aLength, int32_t* aStride,
-                                            IShmemAllocator* aAllocator) {
-  return GetSurfaceDataImpl(aSurface, aLength, aStride,
-                            GetSurfaceDataShmem(aAllocator));
+  return Some(std::move(surfaceData));
 }
 
 Maybe<IPCImage> nsContentUtils::SurfaceToIPCImage(DataSourceSurface& aSurface) {

diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h
@@ -197,8 +197,6 @@ enum class ReferrerPolicy : uint8_t;
 namespace ipc {
 class BigBuffer;
 class IProtocol;
-class IShmemAllocator;
-class Shmem;
 }  // namespace ipc
 
 namespace gfx {
@@ -2920,14 +2918,6 @@ class nsContentUtils {
   static mozilla::Maybe<mozilla::ipc::BigBuffer> GetSurfaceData(
       mozilla::gfx::DataSourceSurface&, size_t* aLength, int32_t* aStride);
 
-  /*
-   * Get the pixel data from the given source surface and fill it in Shmem.
-   * The length and stride will be assigned from the surface.
-   */
-  static mozilla::Maybe<mozilla::ipc::Shmem> GetSurfaceData(
-      mozilla::gfx::DataSourceSurface& aSurface, size_t* aLength,
-      int32_t* aStride, mozilla::ipc::IShmemAllocator* aAlloc);
-
   static mozilla::Maybe<mozilla::dom::IPCImage> SurfaceToIPCImage(
       mozilla::gfx::DataSourceSurface&);
   static already_AddRefed<mozilla::gfx::DataSourceSurface> IPCImageToSurface(

diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp
@@ -3791,7 +3791,7 @@ nsresult BrowserParent::HandleEvent(Event* aEvent) {
 
 mozilla::ipc::IPCResult BrowserParent::RecvInvokeDragSession(
     nsTArray<IPCDataTransfer>&& aTransfers, const uint32_t& aAction,
-    Maybe<Shmem>&& aVisualDnDData, const uint32_t& aStride,
+    Maybe<BigBuffer>&& aVisualDnDData, const uint32_t& aStride,
     const gfx::SurfaceFormat& aFormat, const LayoutDeviceIntRect& aDragRect,
     nsIPrincipal* aPrincipal, nsIContentSecurityPolicy* aCsp,
     const CookieJarSettingsArgs& aCookieJarSettingsArgs,
@@ -3814,11 +3814,10 @@ mozilla::ipc::IPCResult BrowserParent::RecvInvokeDragSession(
       this, std::move(aTransfers), aDragRect, aPrincipal, aCsp,
       cookieJarSettings, aSourceWindowContext.GetMaybeDiscarded());
 
-  if (!aVisualDnDData.isNothing() && aVisualDnDData.ref().IsReadable() &&
-      aVisualDnDData.ref().Size<char>() >= aDragRect.height * aStride) {
+  if (aVisualDnDData && aVisualDnDData->Size() >= aDragRect.height * aStride) {
     dragStartData->SetVisualization(gfx::CreateDataSourceSurfaceFromData(
         gfx::IntSize(aDragRect.width, aDragRect.height), aFormat,
-        aVisualDnDData.ref().get<uint8_t>(), aStride));
+        aVisualDnDData->Data(), aStride));
   }
 
   nsCOMPtr<nsIDragService> dragService =
@@ -3831,10 +3830,6 @@ mozilla::ipc::IPCResult BrowserParent::RecvInvokeDragSession(
       ->EventStateManager()
       ->BeginTrackingRemoteDragGesture(mFrameElement, dragStartData);
 
-  if (aVisualDnDData.isSome()) {
-    Unused << DeallocShmem(aVisualDnDData.ref());
-  }
-
   return IPC_OK();
 }
 

diff --git a/dom/ipc/BrowserParent.h b/dom/ipc/BrowserParent.h
@@ -675,7 +675,7 @@ class BrowserParent final : public PBrowserParent,
 
   mozilla::ipc::IPCResult RecvInvokeDragSession(
       nsTArray<IPCDataTransfer>&& aTransfers, const uint32_t& aAction,
-      Maybe<Shmem>&& aVisualDnDData, const uint32_t& aStride,
+      Maybe<BigBuffer>&& aVisualDnDData, const uint32_t& aStride,
       const gfx::SurfaceFormat& aFormat, const LayoutDeviceIntRect& aDragRect,
       nsIPrincipal* aPrincipal, nsIContentSecurityPolicy* aCsp,
       const CookieJarSettingsArgs& aCookieJarSettingsArgs,

diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl
@@ -633,7 +633,7 @@ parent:
     [Nested=inside_sync] sync DispatchTouchEvent(WidgetTouchEvent event);
 
     async InvokeDragSession(IPCDataTransfer[] transfers, uint32_t action,
-                            Shmem? visualData,
+                            BigBuffer? visualData,
                             uint32_t stride, SurfaceFormat format,
                             LayoutDeviceIntRect dragRect,
                             nsIPrincipal principal, nsIContentSecurityPolicy csp,

diff --git a/widget/nsDragServiceProxy.cpp b/widget/nsDragServiceProxy.cpp
@@ -65,22 +65,15 @@ nsresult nsDragServiceProxy::InvokeDragSessionImpl(
       if (dataSurface) {
         size_t length;
         int32_t stride;
-        Maybe<Shmem> maybeShm = nsContentUtils::GetSurfaceData(
-            *dataSurface, &length, &stride, child);
-        if (maybeShm.isNothing()) {
-          return NS_ERROR_FAILURE;
-        }
-
-        auto surfaceData = maybeShm.value();
-
-        // Save the surface data to shared memory.
-        if (!surfaceData.IsReadable() || !surfaceData.get<char>()) {
+        auto surfaceData =
+            nsContentUtils::GetSurfaceData(*dataSurface, &length, &stride);
+        if (surfaceData.isNothing()) {
           NS_WARNING("Failed to create shared memory for drag session.");
           return NS_ERROR_FAILURE;
         }
 
         mozilla::Unused << child->SendInvokeDragSession(
-            std::move(dataTransfers), aActionType, Some(std::move(surfaceData)),
+            std::move(dataTransfers), aActionType, std::move(surfaceData),
             stride, dataSurface->GetFormat(), dragRect, principal, csp, csArgs,
             mSourceWindowContext);
         StartDragSession();