Bug 1876818 - ClientWebGLContext::Run serialization now coerces to HostWebGLContext arg types. r=gfx-reviewers,lsalzman

Differential Revision: https://phabricator.services.mozilla.com/D199768

dom/canvas/ClientWebGLContext.cpp

@@ -405,79 +405,50 @@ void ClientWebGLContext::ThrowEvent_WebGLContextCreationError(
   target->DispatchEvent(*event);
 }
 
-// -
-
-// If we are running WebGL in this process then call the HostWebGLContext
-// method directly.  Otherwise, dispatch over IPC.
-template <typename MethodType, MethodType method, typename... Args>
-void ClientWebGLContext::Run(Args&&... args) const {
-  const auto notLost =
-      mNotLost;  // Hold a strong-ref to prevent LoseContext=>UAF.
-  if (IsContextLost()) return;
-
-  const auto& inProcess = notLost->inProcess;
-  if (inProcess) {
-    return (inProcess.get()->*method)(std::forward<Args>(args)...);
-  }
-
-  const auto& child = notLost->outOfProcess;
-
-  const auto id = IdByMethod<MethodType, method>();
-
-  const auto info = webgl::SerializationInfo(id, args...);
-  const auto maybeDest = child->AllocPendingCmdBytes(info.requiredByteCount,
-                                                     info.alignmentOverhead);
-  if (!maybeDest) {
-    JsWarning("Failed to allocate internal command buffer.");
-    OnContextLoss(webgl::ContextLossReason::None);
-    return;
-  }
-  const auto& destBytes = *maybeDest;
-  webgl::Serialize(destBytes, id, args...);
-}
-
-template <typename MethodType, MethodType method, typename... Args>
-void ClientWebGLContext::RunWithGCData(JS::AutoCheckCannotGC&& aNoGC,
-                                       Args&&... args) const {
-  // Hold a strong-ref to prevent LoseContext=>UAF.
-  //
-  // Note that `aNoGC` must be reset after the GC data is done being used and
-  // before the `notLost` destructor runs, since it could GC.
-  const auto notLost = mNotLost;
-  if (IsContextLost()) {
-    aNoGC.reset();  // GC data will not be used.
-    return;
-  }
-
-  const auto& inProcess = notLost->inProcess;
-  if (inProcess) {
-    (inProcess.get()->*method)(std::forward<Args>(args)...);
-    aNoGC.reset();  // Done with any GC data
-    return;
-  }
-
-  const auto& child = notLost->outOfProcess;
-
-  const auto id = IdByMethod<MethodType, method>();
-
-  const auto info = webgl::SerializationInfo(id, args...);
-  const auto maybeDest = child->AllocPendingCmdBytes(info.requiredByteCount,
-                                                     info.alignmentOverhead);
-  if (!maybeDest) {
-    aNoGC.reset();  // GC data will not be used.
-    JsWarning("Failed to allocate internal command buffer.");
-    OnContextLoss(webgl::ContextLossReason::None);
-    return;
-  }
-  const auto& destBytes = *maybeDest;
-  webgl::Serialize(destBytes, id, args...);
-  aNoGC.reset();  // Done with any GC data
-}
-
 // -------------------------------------------------------------------------
 // Client-side helper methods.  Dispatch to a Host method.
 // -------------------------------------------------------------------------
 
+// If we are running WebGL in this process then call the HostWebGLContext
+// method directly.  Otherwise, dispatch over IPC.
+template <typename MethodT, typename... Args>
+void ClientWebGLContext::Run_WithDestArgTypes(
+    std::optional<JS::AutoCheckCannotGC> noGc, const MethodT method,
+    const size_t id, const Args&... args) const {
+  const auto notLost =
+      mNotLost;  // Hold a strong-ref to prevent LoseContext=>UAF.
+
+  // `AutoCheckCannotGC` must be reset after the GC data is done being used but
+  // *before* the `notLost` destructor runs, since the latter can GC.
+  const auto cleanup = MakeScopeExit([&]() { noGc.reset(); });
+  if (IsContextLost()) {
+    return;
+  }
+
+  const auto& inProcess = notLost->inProcess;
+  if (inProcess) {
+    (inProcess.get()->*method)(args...);
+    return;
+  }
+
+  const auto& child = notLost->outOfProcess;
+
+  const auto info = webgl::SerializationInfo(id, args...);
+  const auto maybeDest = child->AllocPendingCmdBytes(info.requiredByteCount,
+                                                     info.alignmentOverhead);
+  if (!maybeDest) {
+    noGc.reset();  // Reset early, as GC data will not be used, but JsWarning
+                   // can GC.
+    JsWarning("Failed to allocate internal command buffer.");
+    OnContextLoss(webgl::ContextLossReason::None);
+    return;
+  }
+  const auto& destBytes = *maybeDest;
+  webgl::Serialize(destBytes, id, args...);
+}
+
+// -
+
 #define RPROC(_METHOD) \
   decltype(&HostWebGLContext::_METHOD), &HostWebGLContext::_METHOD
 

dom/canvas/ClientWebGLContext.h

@@ -36,6 +36,9 @@ namespace mozilla {
 class ClientWebGLExtensionBase;
 class HostWebGLContext;
 
+template <typename MethodT, MethodT Method>
+size_t IdByMethod();
+
 namespace dom {
 class OwningHTMLCanvasElementOrOffscreenCanvas;
 class WebGLChild;
@@ -2309,22 +2312,46 @@ class ClientWebGLContext final : public nsICanvasRenderingContextInternal,
   // The cross-process communication mechanism
   // -------------------------------------------------------------------------
  protected:
-  template <typename ReturnType>
-  friend struct WebGLClientDispatcher;
-
-  template <typename MethodType, MethodType method, typename ReturnType,
-            typename... Args>
-  friend ReturnType RunOn(const ClientWebGLContext& context, Args&&... aArgs);
-
   // If we are running WebGL in this process then call the HostWebGLContext
   // method directly.  Otherwise, dispatch over IPC.
-  template <typename MethodType, MethodType method, typename... Args>
-  void Run(Args&&... aArgs) const;
+  template <typename MethodType, MethodType method, typename... CallerArgs>
+  void Run(const CallerArgs&... args) const {
+    const auto id = IdByMethod<MethodType, method>();
+    Run_WithDestArgTypes_ConstnessHelper({}, method, id, args...);
+  }
 
   // Same as above for use when using potentially GC-controlled data. The scope
   // of `aNoGC` will be ended after the data is no longer needed.
-  template <typename MethodType, MethodType method, typename... Args>
-  void RunWithGCData(JS::AutoCheckCannotGC&& aNoGC, Args&&... aArgs) const;
+  template <typename MethodType, MethodType method, typename... CallerArgs>
+  void RunWithGCData(JS::AutoCheckCannotGC&& aNoGC,
+                     const CallerArgs&... aArgs) const {
+    const auto id = IdByMethod<MethodType, method>();
+    Run_WithDestArgTypes_ConstnessHelper(std::move(aNoGC), method, id,
+                                         aArgs...);
+  }
+
+  // Because we're trying to explicitly pull `DestArgs` via `method`, we have
+  // one overload for mut-methods and one for const-methods.
+  template <typename... DestArgs>
+  void Run_WithDestArgTypes_ConstnessHelper(
+      std::optional<JS::AutoCheckCannotGC> noGc,
+      void (HostWebGLContext::*method)(DestArgs...), const size_t id,
+      const std::remove_reference_t<std::remove_const_t<DestArgs>>&... args)
+      const {
+    Run_WithDestArgTypes(std::move(noGc), method, id, args...);
+  }
+  template <typename... DestArgs>
+  void Run_WithDestArgTypes_ConstnessHelper(
+      std::optional<JS::AutoCheckCannotGC> noGc,
+      void (HostWebGLContext::*method)(DestArgs...) const, const size_t id,
+      const std::remove_reference_t<std::remove_const_t<DestArgs>>&... args)
+      const {
+    Run_WithDestArgTypes(std::move(noGc), method, id, args...);
+  }
+
+  template <typename MethodT, typename... DestArgs>
+  void Run_WithDestArgTypes(std::optional<JS::AutoCheckCannotGC>, MethodT,
+                            const size_t id, const DestArgs&...) const;
 
   // -------------------------------------------------------------------------
   // Helpers for DOM operations, composition, actors, etc

dom/canvas/WebGLMethodDispatcher.h

@@ -17,7 +17,7 @@ class WebGLMethodDispatcher
     : public EmptyMethodDispatcher<WebGLMethodDispatcher> {};
 
 template <typename MethodT, MethodT Method>
-size_t IdByMethod() = delete;
+size_t IdByMethod();
 
 #define DEFINE_METHOD_DISPATCHER(_ID, _METHOD)                  \
   template <>                                                   \