Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r=cpearce
authorBryce Van Dyk <bvandyk@mozilla.com>
Tue, 09 Apr 2019 15:07:49 +0000
changeset 527398 e217cf252bb06d2c4f4e61b49ccd1e77614a54df
parent 527397 f09291096c4f9589a14f43468a04970202ae2468
child 527399 917d4b516ba958539d8145c13fab63a5344e953d
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1540136
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r=cpearce - 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
@@ -97,16 +97,28 @@ void ChromiumCDMProxy::Init(PromiseId aP
                   ->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());
                       });
             },
@@ -134,16 +146,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) {
@@ -293,31 +328,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;
 };