Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r=cpearce a=lizzard
authorBryce Van Dyk <bvandyk@mozilla.com>
Tue, 09 Apr 2019 15:07:49 +0000
changeset 523226 5710827ee9cf5011b6698590a7d31e922f636cf2
parent 523225 ecb3ee5969c19fe8c52cedb2bd0291edf11dc83c
child 523227 89768dc13409426b5b0ff972b258a27e2454115a
push id11107
push userarchaeopteryx@coole-files.de
push dateThu, 18 Apr 2019 15:56:49 +0000
treeherdermozilla-beta@f764410a3326 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, lizzard
bugs1540136
milestone67.0
Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r=cpearce a=lizzard - Watch for if a proxy shuts down during init and if so, shutdown the CDM parent that is being initialized. - Make ChromiumCDMParent only store a pointer to a ChromiumCDMProxy when it has successfully initialized. This avoid the lopsided relationship where a if a ChromiumCDMParent fails to initialize it may keep a pointer to a proxy, but the proxy will never have a reference to that CDM parent. Differential Revision: https://phabricator.services.mozilla.com/D26208
dom/media/gmp/ChromiumCDMParent.cpp
dom/media/gmp/ChromiumCDMProxy.cpp
dom/media/gmp/ChromiumCDMProxy.h
--- a/dom/media/gmp/ChromiumCDMParent.cpp
+++ b/dom/media/gmp/ChromiumCDMParent.cpp
@@ -56,38 +56,38 @@ RefPtr<ChromiumCDMParent::InitPromise> C
     return ChromiumCDMParent::InitPromise::CreateAndReject(
         MediaResult(NS_ERROR_FAILURE,
                     nsPrintfCString("ChromiumCDMParent::Init() failed "
                                     "nullCallback=%s nullMainThread=%s",
                                     !aCDMCallback ? "true" : "false",
                                     !aMainThread ? "true" : "false")),
         __func__);
   }
-  mCDMCallback = aCDMCallback;
 
   RefPtr<ChromiumCDMParent::InitPromise> promise =
       mInitPromise.Ensure(__func__);
   RefPtr<ChromiumCDMParent> self = this;
   SendInit(aAllowDistinctiveIdentifier, aAllowPersistentState)
       ->Then(
           AbstractThread::GetCurrent(), __func__,
-          [self](bool aSuccess) {
+          [self, aCDMCallback](bool aSuccess) {
             if (!aSuccess) {
               GMP_LOG(
                   "ChromiumCDMParent::Init() failed with callback from "
                   "child indicating CDM failed init");
               self->mInitPromise.RejectIfExists(
                   MediaResult(NS_ERROR_FAILURE,
                               "ChromiumCDMParent::Init() failed with callback "
                               "from child indicating CDM failed init"),
                   __func__);
               return;
             }
             GMP_LOG(
                 "ChromiumCDMParent::Init() succeeded with callback from child");
+            self->mCDMCallback = aCDMCallback;
             self->mInitPromise.ResolveIfExists(true /* unused */, __func__);
           },
           [self](ResponseRejectReason&& aReason) {
             RefPtr<gmp::GeckoMediaPluginService> service =
                 gmp::GeckoMediaPluginService::GetGeckoMediaPluginService();
             bool xpcomWillShutdown =
                 service && service->XPCOMWillShutdownReceived();
             GMP_LOG(
--- a/dom/media/gmp/ChromiumCDMProxy.cpp
+++ b/dom/media/gmp/ChromiumCDMProxy.cpp
@@ -96,16 +96,28 @@ void ChromiumCDMProxy::Init(PromiseId aP
                         self->mPersistentStateRequired, self->mMainThread)
                   ->Then(self->mMainThread, __func__,
                          [self, aPromiseId, cdm](bool /* unused */) {
                            // CDM init succeeded
                            {
                              MutexAutoLock lock(self->mCDMMutex);
                              self->mCDM = cdm;
                            }
+                           if (self->mIsShutdown) {
+                             self->RejectPromise(
+                                 aPromiseId, NS_ERROR_DOM_INVALID_STATE_ERR,
+                                 NS_LITERAL_CSTRING(
+                                     "ChromiumCDMProxy shutdown during "
+                                     "ChromiumCDMProxy::Init"));
+                             // If shutdown happened while waiting to init, we
+                             // need to explicitly shutdown the CDM to avoid it
+                             // referencing this proxy which is on its way out.
+                             self->ShutdownCDMIfExists();
+                             return;
+                           }
                            self->OnCDMCreated(aPromiseId);
                          },
                          [self, aPromiseId](MediaResult aResult) {
                            // CDM init failed
                            self->RejectPromise(aPromiseId, aResult.Code(),
                                                aResult.Message());
                          });
             },
@@ -133,16 +145,39 @@ void ChromiumCDMProxy::OnCDMCreated(uint
     mKeys->OnCDMCreated(aPromiseId, cdm->PluginId());
   } else {
     // No CDM? Shouldn't be possible, but reject the promise anyway...
     mKeys->RejectPromise(aPromiseId, NS_ERROR_DOM_INVALID_STATE_ERR,
                          NS_LITERAL_CSTRING("Null CDM in OnCDMCreated()"));
   }
 }
 
+void ChromiumCDMProxy::ShutdownCDMIfExists() {
+  EME_LOG(
+      "ChromiumCDMProxy::ShutdownCDMIfExists(this=%p) mCDM=%p, mIsShutdown=%s",
+      this, mCDM.get(), mIsShutdown ? "true" : "false");
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(mGMPThread);
+  MOZ_ASSERT(mIsShutdown,
+             "Should only shutdown the CDM if the proxy is shutting down");
+  RefPtr<gmp::ChromiumCDMParent> cdm;
+  {
+    MutexAutoLock lock(mCDMMutex);
+    cdm.swap(mCDM);
+  }
+  if (cdm) {
+    // We need to keep this proxy alive until the parent has finished its
+    // Shutdown (as it may still try to use the proxy until then).
+    RefPtr<ChromiumCDMProxy> self(this);
+    nsCOMPtr<nsIRunnable> task = NS_NewRunnableFunction(
+        "ChromiumCDMProxy::Shutdown", [self, cdm]() { cdm->Shutdown(); });
+    mGMPThread->Dispatch(task.forget());
+  }
+}
+
 #ifdef DEBUG
 bool ChromiumCDMProxy::IsOnOwnerThread() {
   return mGMPThread->IsCurrentThreadIn();
 }
 #endif
 
 static uint32_t ToCDMSessionType(dom::MediaKeySessionType aSessionType) {
   switch (aSessionType) {
@@ -292,31 +327,24 @@ void ChromiumCDMProxy::RemoveSession(con
   mGMPThread->Dispatch(NewRunnableMethod<nsCString, uint32_t>(
       "gmp::ChromiumCDMParent::RemoveSession", cdm,
       &gmp::ChromiumCDMParent::RemoveSession, NS_ConvertUTF16toUTF8(aSessionId),
       aPromiseId));
 }
 
 void ChromiumCDMProxy::Shutdown() {
   MOZ_ASSERT(NS_IsMainThread());
-  EME_LOG("ChromiumCDMProxy::Shutdown(this=%p) mCDM=%p", this, mCDM.get());
-  mKeys.Clear();
-  RefPtr<gmp::ChromiumCDMParent> cdm;
-  {
-    MutexAutoLock lock(mCDMMutex);
-    cdm.swap(mCDM);
+  EME_LOG("ChromiumCDMProxy::Shutdown(this=%p) mCDM=%p, mIsShutdown=%s", this,
+          mCDM.get(), mIsShutdown ? "true" : "false");
+  if (mIsShutdown) {
+    return;
   }
-  if (cdm) {
-    // We need to keep this proxy alive until the parent has finished its
-    // Shutdown (as it may still try to use the proxy until then).
-    RefPtr<ChromiumCDMProxy> self(this);
-    nsCOMPtr<nsIRunnable> task = NS_NewRunnableFunction(
-        "ChromiumCDMProxy::Shutdown", [self, cdm]() { cdm->Shutdown(); });
-    mGMPThread->Dispatch(task.forget());
-  }
+  mIsShutdown = true;
+  mKeys.Clear();
+  ShutdownCDMIfExists();
 }
 
 void ChromiumCDMProxy::RejectPromise(PromiseId aId, nsresult aCode,
                                      const nsCString& aReason) {
   if (!NS_IsMainThread()) {
     mMainThread->Dispatch(
         NewRunnableMethod<PromiseId, nsresult, nsCString>(
             "ChromiumCDMProxy::RejectPromise", this,
--- a/dom/media/gmp/ChromiumCDMProxy.h
+++ b/dom/media/gmp/ChromiumCDMProxy.h
@@ -2,18 +2,18 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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 ChromiumCDMProxy_h_
 #define ChromiumCDMProxy_h_
 
+#include "mozilla/AbstractThread.h"
 #include "mozilla/CDMProxy.h"
-#include "mozilla/AbstractThread.h"
 #include "ChromiumCDMParent.h"
 
 namespace mozilla {
 
 class MediaRawData;
 class DecryptJob;
 class ChromiumCDMCallbackProxy;
 class ChromiumCDMProxy : public CDMProxy {
@@ -103,19 +103,24 @@ class ChromiumCDMProxy : public CDMProxy
   // CDM, which will fail on all operations.
   already_AddRefed<gmp::ChromiumCDMParent> GetCDMParent();
 
   void OnResolvePromiseWithKeyStatus(uint32_t aPromiseId,
                                      dom::MediaKeyStatus aKeyStatus);
 
  private:
   void OnCDMCreated(uint32_t aPromiseId);
+  void ShutdownCDMIfExists();
 
   ~ChromiumCDMProxy();
 
+  // True if Shutdown() has been called. Should only be read and written on
+  // main thread.
+  bool mIsShutdown = false;
+
   RefPtr<GMPCrashHelper> mCrashHelper;
 
   Mutex mCDMMutex;
   RefPtr<gmp::ChromiumCDMParent> mCDM;
   RefPtr<AbstractThread> mGMPThread;
   UniquePtr<ChromiumCDMCallbackProxy> mCallback;
 };