Bug 1267918 - Add GMPCrashHelperHolder to automatically disconnect crash helpers on actor destory. r=gerald

Disconnecting the GMPCrashHelpers at the right time is hard, because in the
crashing case we're all shutdown before the GMPCrashHelpers can be invoked to
help handle the crash report. So add a class to help the helpers;
GMPCrashHelperHolder. This composes into the GMP content protocol actors, to
help them disconnect the crash helpers at the right time. See the comment in
GMPCrashHelperHolder for the details.


MozReview-Commit-ID: E5rE6e5Jues

--HG--
extra : rebase_source : f688bf727f23f773eb995611cec6412ebf021029

dom/media/gmp/GMPAudioDecoderParent.cpp

@@ -226,6 +226,7 @@ GMPAudioDecoderParent::ActorDestroy(ActorDestroyReason aWhy)
     mPlugin->AudioDecoderDestroyed(this);
     mPlugin = nullptr;
   }
+  MaybeDisconnect(aWhy == AbnormalShutdown);
 }
 
 bool

dom/media/gmp/GMPAudioDecoderParent.h

@@ -12,6 +12,7 @@
 #include "mozilla/gmp/PGMPAudioDecoderParent.h"
 #include "GMPMessageUtils.h"
 #include "GMPAudioDecoderProxy.h"
+#include "GMPCrashHelperHolder.h"
 
 namespace mozilla {
 namespace gmp {
@@ -20,6 +21,7 @@ class GMPContentParent;
 
 class GMPAudioDecoderParent final : public GMPAudioDecoderProxy
                                   , public PGMPAudioDecoderParent
+                                  , public GMPCrashHelperHolder
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(GMPAudioDecoderParent)

dom/media/gmp/GMPCrashHelperHolder.h

@@ -0,0 +1,81 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+* License, v. 2.0. If a copy of the MPL was not distributed with this
+* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef GMPCrashHelperHolder_h_
+#define GMPCrashHelperHolder_h_
+
+#include "GMPService.h"
+#include "mozilla/RefPtr.h"
+#include "nsPIDOMWindow.h"
+#include "mozilla/ipc/ProtocolUtils.h"
+
+class GMPCrashHelper;
+
+namespace mozilla {
+
+// Disconnecting the GMPCrashHelpers at the right time is hard. We need to
+// ensure that in the crashing case that we stay connected until the
+// "gmp-plugin-crashed" message is processed in the content process.
+//
+// We have two channels connecting to the GMP; PGMP which connects from
+// chrome to GMP process, and PGMPContent, which bridges between the content
+// and GMP process. If the GMP crashes both PGMP and PGMPContent receive
+// ActorDestroy messages and begin to shutdown at the same time.
+//
+// However the crash report mini dump must be generated in the chrome
+// process' ActorDestroy, before the "gmp-plugin-crashed" message can be sent
+// to the content process. We fire the "PluginCrashed" event when we handle
+// the "gmp-plugin-crashed" message in the content process, and we need the
+// crash helpers to do that.
+//
+// The PGMPContent's managed actors' ActorDestroy messages are the only shutdown
+// notification we get in the content process, but we can't disconnect the
+// crash handlers there in the crashing case, as ActorDestroy happens before
+// we've received the "gmp-plugin-crashed" message and had a chance for the
+// crash helpers to generate the window to dispatch PluginCrashed to initiate
+// the crash report submission notification box.
+//
+// So we need to ensure that in the content process, the GMPCrashHelpers stay
+// connected to the GMPService until after ActorDestroy messages are received
+// if there's an abnormal shutdown. In the case where the GMP doesn't crash,
+// we do actually want to disconnect GMPCrashHandlers in ActorDestroy, since
+// we don't have any other signal that a GMP actor is shutting down. If we don't
+// disconnect the crash helper there in the normal shutdown case, the helper
+// will stick around forever and leak.
+//
+// In the crashing case, the GMPCrashHelpers are deallocated when the crash
+// report is processed in GeckoMediaPluginService::RunPluginCrashCallbacks().
+//
+// It's a bit yuck that we have to have two paths for disconnecting the crash
+// helpers, but there aren't really any better options.
+class GMPCrashHelperHolder
+{
+public:
+  
+  void SetCrashHelper(GMPCrashHelper* aHelper)
+  {
+    mCrashHelper = aHelper;
+  }
+  
+  GMPCrashHelper* GetCrashHelper()
+  {
+    return mCrashHelper;
+  }
+
+  void MaybeDisconnect(bool aAbnormalShutdown)
+  {
+    if (!aAbnormalShutdown) {
+      RefPtr<gmp::GeckoMediaPluginService> service(gmp::GeckoMediaPluginService::GetGeckoMediaPluginService());
+      service->DisconnectCrashHelper(GetCrashHelper());
+    }
+  }
+
+private:
+  RefPtr<GMPCrashHelper> mCrashHelper;
+};
+
+}
+
+#endif // GMPCrashHelperHolder_h_

dom/media/gmp/GMPDecryptorParent.cpp

@@ -426,6 +426,7 @@ GMPDecryptorParent::ActorDestroy(ActorDestroyReason aWhy)
     mPlugin->DecryptorDestroyed(this);
     mPlugin = nullptr;
   }
+  MaybeDisconnect(aWhy == AbnormalShutdown);
 }
 
 bool

dom/media/gmp/GMPDecryptorParent.h

@@ -10,6 +10,7 @@
 #include "mozilla/RefPtr.h"
 #include "gmp-decryption.h"
 #include "GMPDecryptorProxy.h"
+#include "GMPCrashHelperHolder.h"
 
 namespace mozilla {
 
@@ -21,6 +22,7 @@ class GMPContentParent;
 
 class GMPDecryptorParent final : public GMPDecryptorProxy
                                , public PGMPDecryptorParent
+                               , public GMPCrashHelperHolder
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(GMPDecryptorParent)

dom/media/gmp/GMPService.cpp

@@ -413,22 +413,25 @@ GeckoMediaPluginService::GetAbstractGMPThread()
 class GetGMPContentParentForAudioDecoderDone : public GetGMPContentParentCallback
 {
 public:
-  explicit GetGMPContentParentForAudioDecoderDone(UniquePtr<GetGMPAudioDecoderCallback>&& aCallback)
+  explicit GetGMPContentParentForAudioDecoderDone(UniquePtr<GetGMPAudioDecoderCallback>&& aCallback,
+                                                  GMPCrashHelper* aHelper)
    : mCallback(Move(aCallback))
+   , mHelper(aHelper)
   {
   }
 
   void Done(GMPContentParent* aGMPParent) override
   {
     GMPAudioDecoderParent* gmpADP = nullptr;
-    if (aGMPParent) {
+    if (aGMPParent && NS_SUCCEEDED(aGMPParent->GetGMPAudioDecoder(&gmpADP))) {
-      aGMPParent->GetGMPAudioDecoder(&gmpADP);
+      gmpADP->SetCrashHelper(mHelper);
     }
     mCallback->Done(gmpADP);
   }
 
 private:
   UniquePtr<GetGMPAudioDecoderCallback> mCallback;
+  RefPtr<GMPCrashHelper> mHelper;
 };
 
 NS_IMETHODIMP
@@ -446,7 +449,7 @@ GeckoMediaPluginService::GetGMPAudioDecoder(GMPCrashHelper* aHelper,
   }
 
   UniquePtr<GetGMPContentParentCallback> callback(
-    new GetGMPContentParentForAudioDecoderDone(Move(aCallback)));
+    new GetGMPContentParentForAudioDecoderDone(Move(aCallback), aHelper));
   if (!GetContentParentFrom(aHelper,
                             aNodeId,
                             NS_LITERAL_CSTRING(GMP_API_AUDIO_DECODER),
@@ -461,8 +464,10 @@ GeckoMediaPluginService::GetGMPAudioDecoder(GMPCrashHelper* aHelper,
 class GetGMPContentParentForVideoDecoderDone : public GetGMPContentParentCallback
 {
 public:
-  explicit GetGMPContentParentForVideoDecoderDone(UniquePtr<GetGMPVideoDecoderCallback>&& aCallback)
+  explicit GetGMPContentParentForVideoDecoderDone(UniquePtr<GetGMPVideoDecoderCallback>&& aCallback,
+                                                  GMPCrashHelper* aHelper)
    : mCallback(Move(aCallback))
+   , mHelper(aHelper)
   {
   }
 
@@ -472,12 +477,14 @@ public:
     GMPVideoHostImpl* videoHost = nullptr;
     if (aGMPParent && NS_SUCCEEDED(aGMPParent->GetGMPVideoDecoder(&gmpVDP))) {
       videoHost = &gmpVDP->Host();
+      gmpVDP->SetCrashHelper(mHelper);
     }
     mCallback->Done(gmpVDP, videoHost);
   }
 
 private:
   UniquePtr<GetGMPVideoDecoderCallback> mCallback;
+  RefPtr<GMPCrashHelper> mHelper;
 };
 
 NS_IMETHODIMP
@@ -495,7 +502,7 @@ GeckoMediaPluginService::GetGMPVideoDecoder(GMPCrashHelper* aHelper,
   }
 
   UniquePtr<GetGMPContentParentCallback> callback(
-    new GetGMPContentParentForVideoDecoderDone(Move(aCallback)));
+    new GetGMPContentParentForVideoDecoderDone(Move(aCallback), aHelper));
   if (!GetContentParentFrom(aHelper,
                             aNodeId,
                             NS_LITERAL_CSTRING(GMP_API_VIDEO_DECODER),
@@ -510,8 +517,10 @@ GeckoMediaPluginService::GetGMPVideoDecoder(GMPCrashHelper* aHelper,
 class GetGMPContentParentForVideoEncoderDone : public GetGMPContentParentCallback
 {
 public:
-  explicit GetGMPContentParentForVideoEncoderDone(UniquePtr<GetGMPVideoEncoderCallback>&& aCallback)
+  explicit GetGMPContentParentForVideoEncoderDone(UniquePtr<GetGMPVideoEncoderCallback>&& aCallback,
+                                                  GMPCrashHelper* aHelper)
    : mCallback(Move(aCallback))
+   , mHelper(aHelper)
   {
   }
 
@@ -521,12 +530,14 @@ public:
     GMPVideoHostImpl* videoHost = nullptr;
     if (aGMPParent && NS_SUCCEEDED(aGMPParent->GetGMPVideoEncoder(&gmpVEP))) {
       videoHost = &gmpVEP->Host();
+      gmpVEP->SetCrashHelper(mHelper);
     }
     mCallback->Done(gmpVEP, videoHost);
   }
 
 private:
   UniquePtr<GetGMPVideoEncoderCallback> mCallback;
+  RefPtr<GMPCrashHelper> mHelper;
 };
 
 NS_IMETHODIMP
@@ -544,7 +555,7 @@ GeckoMediaPluginService::GetGMPVideoEncoder(GMPCrashHelper* aHelper,
   }
 
   UniquePtr<GetGMPContentParentCallback> callback(
-    new GetGMPContentParentForVideoEncoderDone(Move(aCallback)));
+    new GetGMPContentParentForVideoEncoderDone(Move(aCallback), aHelper));
   if (!GetContentParentFrom(aHelper,
                             aNodeId,
                             NS_LITERAL_CSTRING(GMP_API_VIDEO_ENCODER),
@@ -559,22 +570,25 @@ GeckoMediaPluginService::GetGMPVideoEncoder(GMPCrashHelper* aHelper,
 class GetGMPContentParentForDecryptorDone : public GetGMPContentParentCallback
 {
 public:
-  explicit GetGMPContentParentForDecryptorDone(UniquePtr<GetGMPDecryptorCallback>&& aCallback)
+  explicit GetGMPContentParentForDecryptorDone(UniquePtr<GetGMPDecryptorCallback>&& aCallback,
+                                               GMPCrashHelper* aHelper)
    : mCallback(Move(aCallback))
+   , mHelper(aHelper)
   {
   }
 
   void Done(GMPContentParent* aGMPParent) override
   {
     GMPDecryptorParent* ksp = nullptr;
-    if (aGMPParent) {
+    if (aGMPParent && NS_SUCCEEDED(aGMPParent->GetGMPDecryptor(&ksp))) {
-      aGMPParent->GetGMPDecryptor(&ksp);
+      ksp->SetCrashHelper(mHelper);
     }
     mCallback->Done(ksp);
   }
 
 private:
   UniquePtr<GetGMPDecryptorCallback> mCallback;
+  RefPtr<GMPCrashHelper> mHelper;
 };
 
 NS_IMETHODIMP
@@ -600,7 +614,7 @@ GeckoMediaPluginService::GetGMPDecryptor(GMPCrashHelper* aHelper,
   }
 
   UniquePtr<GetGMPContentParentCallback> callback(
-    new GetGMPContentParentForDecryptorDone(Move(aCallback)));
+    new GetGMPContentParentForDecryptorDone(Move(aCallback), aHelper));
   if (!GetContentParentFrom(aHelper,
                             aNodeId,
                             NS_LITERAL_CSTRING(GMP_API_DECRYPTOR),

dom/media/gmp/GMPVideoDecoderParent.cpp

@@ -300,6 +300,7 @@ GMPVideoDecoderParent::ActorDestroy(ActorDestroyReason aWhy)
     mPlugin = nullptr;
   }
   mVideoHost.ActorDestroyed();
+  MaybeDisconnect(aWhy == AbnormalShutdown);
 }
 
 bool

dom/media/gmp/GMPVideoDecoderParent.h

@@ -15,6 +15,7 @@
 #include "GMPVideoHost.h"
 #include "GMPVideoDecoderProxy.h"
 #include "VideoUtils.h"
+#include "GMPCrashHelperHolder.h"
 
 namespace mozilla {
 namespace gmp {
@@ -24,6 +25,7 @@ class GMPContentParent;
 class GMPVideoDecoderParent final : public PGMPVideoDecoderParent
                                   , public GMPVideoDecoderProxy
                                   , public GMPSharedMemManager
+                                  , public GMPCrashHelperHolder
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(GMPVideoDecoderParent)

dom/media/gmp/GMPVideoEncoderParent.cpp

@@ -269,6 +269,7 @@ GMPVideoEncoderParent::ActorDestroy(ActorDestroyReason aWhy)
     mPlugin = nullptr;
   }
   mVideoHost.ActorDestroyed(); // same as DoneWithAPI
+  MaybeDisconnect(aWhy == AbnormalShutdown);
 }
 
 static void

dom/media/gmp/GMPVideoEncoderParent.h

@@ -14,6 +14,7 @@
 #include "GMPUtils.h"
 #include "GMPVideoHost.h"
 #include "GMPVideoEncoderProxy.h"
+#include "GMPCrashHelperHolder.h"
 
 namespace mozilla {
 namespace gmp {
@@ -22,7 +23,8 @@ class GMPContentParent;
 
 class GMPVideoEncoderParent : public GMPVideoEncoderProxy,
                               public PGMPVideoEncoderParent,
-                              public GMPSharedMemManager
+                              public GMPSharedMemManager,
+                              public GMPCrashHelperHolder
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(GMPVideoEncoderParent)

dom/media/gmp/moz.build

@@ -38,6 +38,7 @@ EXPORTS += [
     'GMPChild.h',
     'GMPContentChild.h',
     'GMPContentParent.h',
+    'GMPCrashHelperHolder.h',
     'GMPDecryptorChild.h',
     'GMPDecryptorParent.h',
     'GMPDecryptorProxy.h',