Backed out changeset 7b7eadc28250 (bug 1876818) for causing hazard-linux bustage in ClientWebGLContext.h. CLOSED TREE

dom/canvas/ClientWebGLContext.cpp

@@ -405,40 +405,29 @@ void ClientWebGLContext::ThrowEvent_WebGLContextCreationError(
   target->DispatchEvent(*event);
 }
 
-// -------------------------------------------------------------------------
-// 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 {
+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.
-
-  // `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;
-  }
+  if (IsContextLost()) return;
 
   const auto& inProcess = notLost->inProcess;
   if (inProcess) {
-    (inProcess.get()->*method)(args...);
-    return;
+    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) {
-    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;
@@ -447,7 +436,47 @@ void ClientWebGLContext::Run_WithDestArgTypes(
   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.
+// -------------------------------------------------------------------------
 
 #define RPROC(_METHOD) \
   decltype(&HostWebGLContext::_METHOD), &HostWebGLContext::_METHOD

dom/canvas/ClientWebGLContext.h

@@ -36,9 +36,6 @@ namespace mozilla {
 class ClientWebGLExtensionBase;
 class HostWebGLContext;
 
-template <typename MethodT, MethodT Method>
-size_t IdByMethod();
-
 namespace dom {
 class OwningHTMLCanvasElementOrOffscreenCanvas;
 class WebGLChild;
@@ -2312,46 +2309,22 @@ 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... CallerArgs>
-  void Run(const CallerArgs&... args) const {
-    const auto id = IdByMethod<MethodType, method>();
-    Run_WithDestArgTypes_ConstnessHelper({}, method, id, args...);
-  }
+  template <typename MethodType, MethodType method, typename... Args>
+  void Run(Args&&... aArgs) const;
 
   // 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... 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;
+  template <typename MethodType, MethodType method, typename... Args>
+  void RunWithGCData(JS::AutoCheckCannotGC&& aNoGC, Args&&... aArgs) 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();
+size_t IdByMethod() = delete;
 
 #define DEFINE_METHOD_DISPATCHER(_ID, _METHOD)                  \
   template <>                                                   \