Bug 1654969: Always disable COM pings for mscom::FastMarshaler (and thus mscom::Interceptor). r=aklotz

Previously, we only did this when IsCallerExternalProcess() returned false.
There are three reasons for changing this:

1. There seem to be cases where IsCallerExternalProcess() returns true even when marshaling for a COM query in the MTA.
2. After bug 1627084, we pre-build a11y handler payloads on the main thread for bulk fetch calls. That will marshal interceptors. However, IsCallerExternalProcess() can't work in that case because it's not running on the thread on which the COM call is being handled.
3. If MSHLFLAGS_NOPING is used, Release calls from remote clients are never sent to the server. So, as soon as we use NOPING for our parent process, we're already going to leak references, even if we don't use NOPING for external callers. Put another way, as soon as we use NOPING for one caller, we may as well use it for all callers because COM pinging will never release the object anyway.

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

ipc/mscom/FastMarshaler.cpp

@@ -91,11 +91,6 @@ FastMarshaler::GetMarshalFlags(DWORD aDestContext, DWORD aMshlFlags) {
     return aMshlFlags;
   }
 
-  if (IsCallerExternalProcess()) {
-    return aMshlFlags;
-  }
-
-  // The caller is our parent main thread. Disable ping functionality.
   return aMshlFlags | MSHLFLAGS_NOPING;
 }
 

ipc/mscom/FastMarshaler.h

@@ -17,13 +17,16 @@ namespace mozilla {
 namespace mscom {
 
 /**
- * When we are marshaling to the parent process main thread, we want to turn
- * off COM's ping functionality. That functionality is designed to free
- * strong references held by defunct client processes. Since our content
- * processes cannot outlive our parent process, we turn off pings when we know
- * that the COM client is going to be our parent process. This provides a
- * significant performance boost in a11y code due to large numbers of remote
- * objects being created and destroyed within a short period of time.
+ * COM ping functionality is enabled by default and is designed to free strong
+ * references held by defunct client processes. However, this incurs a
+ * significant performance penalty in a11y code due to large numbers of remote
+ * objects being created and destroyed within a short period of time. Thus, we
+ * turn off pings to improve performance.
+ * ACHTUNG! When COM pings are disabled, Release calls from remote clients are
+ * never sent to the server! If you use this marshaler, you *must* explicitly
+ * disconnect clients using CoDisconnectObject when the object is no longer
+ * relevant. Otherwise, references to the object will never be released, causing
+ * a leak.
  */
 class FastMarshaler final : public IMarshal {
  public:

ipc/mscom/Interceptor.h

@@ -63,6 +63,11 @@ struct IInterceptor : public IUnknown {
  * We accomplish this by using COM aggregation, which means that the
  * ICallInterceptor delegates its IUnknown implementation to its outer object
  * (the mscom::Interceptor we implement and control).
+ *
+ * ACHTUNG! mscom::Interceptor uses FastMarshaler to disable COM garbage
+ * collection. If you use this class, you *must* call
+ * Interceptor::DisconnectRemotesForTarget when an object is no longer relevant.
+ * Otherwise, the object will never be released, causing a leak.
  */
 class Interceptor final : public WeakReferenceSupport,
                           public IStdMarshalInfo,