Bug 1522547 - Have MediaKeys observe if the owning document becomes inactive so it can shutdown. r=cpearce

MediaKeys objects are typically created and associated with an HTMLMediaElement,
however it is possible to create a MediaKeys object and not associate it with an
HTMLMediaElement.

This resulted in an issue where these MediaKeys would keep alive other
components that would assert during bowrser shutdown (see bug 1522547). We
anticipated that MediaKeys associated with an HTMLMediaElement would need to be
shutdown if their owning document became inactive, but were not handling the
case where the keys never became associated with an element.

This patch has the MediaKeys listen directly to their owning document for
activity change. The MediaKeys will shutdown if their document becomes inactive.
This avoids MediaKeys not associated with HTMLMediaElements keeping other
objects alive during browser shutdown.

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

--HG--
extra : moz-landing-system : lando

dom/html/HTMLMediaElement.cpp

@@ -6027,7 +6027,8 @@ void HTMLMediaElement::NotifyOwnerDocumentActivityChanged() {
 
   // If the owning document has become inactive we should shutdown the CDM.
   if (!OwnerDoc()->IsCurrentActiveDocument() && mMediaKeys) {
-    mMediaKeys->Shutdown();
+    // We don't shutdown MediaKeys here because it also listens for document
+    // activity and will take care of shutting down itself.
     DDUNLINKCHILD(mMediaKeys.get());
     mMediaKeys = nullptr;
     if (mDecoder) {

dom/media/eme/MediaKeys.cpp

@@ -35,14 +35,37 @@ namespace mozilla {
 
 namespace dom {
 
-NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(MediaKeys, mElement, mParent,
-                                      mKeySessions, mPromises,
-                                      mPendingSessions);
+// We don't use NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE because we need to
+// unregister our MediaKeys from mDocument's activity listeners. If we don't do
+// this then cycle collection can null mDocument before our dtor runs and the
+// observer ptr held by mDocument will dangle.
+NS_IMPL_CYCLE_COLLECTION_CLASS(MediaKeys)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(MediaKeys)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mElement)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParent)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mKeySessions)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPromises)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingSessions)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDocument)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(MediaKeys)
+
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(MediaKeys)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mElement)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mParent)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mKeySessions)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mPromises)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mPendingSessions)
+  tmp->UnregisterActivityObserver();
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocument)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
 NS_IMPL_CYCLE_COLLECTING_ADDREF(MediaKeys)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(MediaKeys)
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(MediaKeys)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
-  NS_INTERFACE_MAP_ENTRY(nsISupports)
+  NS_INTERFACE_MAP_ENTRY(nsIDocumentActivity)
 NS_INTERFACE_MAP_END
 
 MediaKeys::MediaKeys(nsPIDOMWindowInner* aParent, const nsAString& aKeySystem,
@@ -56,10 +79,38 @@ MediaKeys::MediaKeys(nsPIDOMWindowInner* aParent, const nsAString& aKeySystem,
 }
 
 MediaKeys::~MediaKeys() {
+  UnregisterActivityObserver();
+  mDocument = nullptr;
   Shutdown();
   EME_LOG("MediaKeys[%p] destroyed", this);
 }
 
+void MediaKeys::RegisterActivityObserver() {
+  MOZ_ASSERT(mDocument);
+  if (mDocument) {
+    mDocument->RegisterActivityObserver(this);
+  }
+}
+
+void MediaKeys::UnregisterActivityObserver() {
+  if (mDocument) {
+    mDocument->UnregisterActivityObserver(this);
+  }
+}
+
+// NS_DECL_NSIDOCUMENTACTIVITY
+void MediaKeys::NotifyOwnerDocumentActivityChanged() {
+  EME_LOG("MediaKeys[%p] NotifyOwnerDocumentActivityChanged()", this);
+  // If our owning document is no longer active we should shutdown.
+  if (!mDocument->IsCurrentActiveDocument()) {
+    EME_LOG(
+        "MediaKeys[%p] NotifyOwnerDocumentActivityChanged() owning document is "
+        "not active, shutting down!",
+        this);
+    Shutdown();
+  }
+}
+
 void MediaKeys::Terminated() {
   EME_LOG("MediaKeys[%p] CDM crashed unexpectedly", this);
 
@@ -378,7 +429,9 @@ already_AddRefed<DetailedPromise> MediaKeys::Init(ErrorResult& aRv) {
     return promise.forget();
   }
 
-  mTopLevelPrincipal = top->GetExtantDoc()->NodePrincipal();
+  mDocument = top->GetExtantDoc();
+
+  mTopLevelPrincipal = mDocument->NodePrincipal();
 
   if (!mPrincipal || !mTopLevelPrincipal) {
     NS_WARNING("Failed to get principals when creating MediaKeys");
@@ -429,6 +482,8 @@ already_AddRefed<DetailedPromise> MediaKeys::Init(ErrorResult& aRv) {
                NS_ConvertUTF8toUTF16(topLevelOrigin),
                KeySystemToGMPName(mKeySystem));
 
+  RegisterActivityObserver();
+
   return promise.forget();
 }
 

dom/media/eme/MediaKeys.h

@@ -14,6 +14,7 @@
 #include "mozilla/RefPtr.h"
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
+#include "nsIDocumentActivity.h"
 #include "nsRefPtrHashtable.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/MediaKeysBinding.h"
@@ -48,7 +49,7 @@ typedef uint32_t PromiseId;
 
 // This class is used on the main thread only.
 // Note: its addref/release is not (and can't be) thread safe!
-class MediaKeys final : public nsISupports,
+class MediaKeys final : public nsIDocumentActivity,
                         public nsWrapperCache,
                         public SupportsWeakPtr<MediaKeys>,
                         public DecoderDoctorLifeLogger<MediaKeys> {
@@ -58,6 +59,9 @@ class MediaKeys final : public nsISupports,
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(MediaKeys)
   MOZ_DECLARE_WEAKREFERENCE_TYPENAME(MediaKeys)
+  // We want to listen to the owning document so we can shutdown if it goes
+  // inactive.
+  NS_DECL_NSIDOCUMENTACTIVITY
 
   MediaKeys(nsPIDOMWindowInner* aParentWindow, const nsAString& aKeySystem,
             const MediaKeySystemConfiguration& aConfig);
@@ -156,11 +160,15 @@ class MediaKeys final : public nsISupports,
   // Removes promise from mPromises, and returns it.
   already_AddRefed<DetailedPromise> RetrievePromise(PromiseId aId);
 
+  void RegisterActivityObserver();
+  void UnregisterActivityObserver();
+
   // Owning ref to proxy. The proxy has a weak reference back to the MediaKeys,
   // and the MediaKeys destructor clears the proxy's reference to the MediaKeys.
   RefPtr<CDMProxy> mProxy;
 
   RefPtr<HTMLMediaElement> mElement;
+  RefPtr<Document> mDocument;
 
   nsCOMPtr<nsPIDOMWindowInner> mParent;
   const nsString mKeySystem;