Bug 1233274 - Don't shutdown detached MediaKeys. r=gerald

dom/html/HTMLMediaElement.cpp

@@ -4842,20 +4842,8 @@ HTMLMediaElement::SetMediaKeys(mozilla::dom::MediaKeys* aMediaKeys,
   if (aRv.Failed()) {
     return nullptr;
   }
-  if (mMediaKeys == aMediaKeys) {
-    promise->MaybeResolve(JS::UndefinedHandleValue);
-    return promise.forget();
-  }
-  if (aMediaKeys && aMediaKeys->IsBoundToMediaElement()) {
-    promise->MaybeReject(NS_ERROR_DOM_QUOTA_EXCEEDED_ERR,
-                         NS_LITERAL_CSTRING("MediaKeys object is already bound to another HTMLMediaElement"));
-    return promise.forget();
-  }
-  if (mMediaKeys) {
-    // Existing MediaKeys object. Shut it down.
-    mMediaKeys->Shutdown();
-    mMediaKeys = nullptr;
-  }
+
+  // We only support EME for MSE content by default.
   if (mDecoder &&
       !mMediaSource &&
       Preferences::GetBool("media.eme.mse-only", true)) {
@@ -4865,19 +4853,92 @@ HTMLMediaElement::SetMediaKeys(mozilla::dom::MediaKeys* aMediaKeys,
     return promise.forget();
   }
 
-  mMediaKeys = aMediaKeys;
-  if (mMediaKeys) {
-    if (NS_FAILED(mMediaKeys->Bind(this))) {
-      promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR,
-                           NS_LITERAL_CSTRING("Failed to bind MediaKeys object to HTMLMediaElement"));
-      mMediaKeys = nullptr;
+  // 1. If mediaKeys and the mediaKeys attribute are the same object,
+  // return a resolved promise.
+  if (mMediaKeys == aMediaKeys) {
+    promise->MaybeResolve(JS::UndefinedHandleValue);
     return promise.forget();
   }
+
+  // Note: Our attaching code is synchronous, so we can skip the following steps.
+
+  // 2. If this object's attaching media keys value is true, return a
+  // promise rejected with a new DOMException whose name is InvalidStateError.
+  // 3. Let this object's attaching media keys value be true.
+  // 4. Let promise be a new promise.
+  // 5. Run the following steps in parallel:
+
+  // 5.1 If mediaKeys is not null, CDM instance represented by mediaKeys is
+  // already in use by another media element, and the user agent is unable
+  // to use it with this element, let this object's attaching media keys
+  // value be false and reject promise with a new DOMException whose name
+  // is QuotaExceededError.
+  if (aMediaKeys && aMediaKeys->IsBoundToMediaElement()) {
+    promise->MaybeReject(NS_ERROR_DOM_QUOTA_EXCEEDED_ERR,
+      NS_LITERAL_CSTRING("MediaKeys object is already bound to another HTMLMediaElement"));
+    return promise.forget();
+  }
+
+  // 5.2 If the mediaKeys attribute is not null, run the following steps:
+  if (mMediaKeys) {
+    // 5.2.1 If the user agent or CDM do not support removing the association,
+    // let this object's attaching media keys value be false and reject promise
+    // with a new DOMException whose name is NotSupportedError.
+
+    // 5.2.2 If the association cannot currently be removed, let this object's
+    // attaching media keys value be false and reject promise with a new
+    // DOMException whose name is InvalidStateError.
     if (mDecoder) {
-      mDecoder->SetCDMProxy(mMediaKeys->GetCDMProxy());
+      // We don't support swapping out the MediaKeys once we've started to
+      // setup the playback pipeline. Note this also means we don't need to worry
+      // about handling disassociating the MediaKeys from the MediaDecoder. 
+      promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR,
+        NS_LITERAL_CSTRING("Can't change MediaKeys on HTMLMediaElement after load has started"));
+      return promise.forget();
+    }
+
+    // 5.2.3 Stop using the CDM instance represented by the mediaKeys attribute
+    // to decrypt media data and remove the association with the media element.
+    mMediaKeys->Unbind();
+    mMediaKeys = nullptr;
+
+    // 5.2.4 If the preceding step failed, let this object's attaching media
+    // keys value be false and reject promise with a new DOMException whose
+    // name is the appropriate error name.
+  }
+
+  // 5.3. If mediaKeys is not null, run the following steps:
+  if (aMediaKeys) {
+    // 5.3.1 Associate the CDM instance represented by mediaKeys with the
+    // media element for decrypting media data.
+    if (NS_FAILED(aMediaKeys->Bind(this))) {
+      // 5.3.2 If the preceding step failed, run the following steps:
+      // 5.3.2.1 Set the mediaKeys attribute to null.
+      mMediaKeys = nullptr;
+      // 5.3.2.2 Let this object's attaching media keys value be false.
+      // 5.3.2.3 Reject promise with a new DOMException whose name is
+      // the appropriate error name.
+      promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR,
+                           NS_LITERAL_CSTRING("Failed to bind MediaKeys object to HTMLMediaElement"));
+      return promise.forget();
+    }
+    // 5.3.3 Queue a task to run the "Attempt to Resume Playback If Necessary"
+    // algorithm on the media element.
+    // Note: Setting the CDMProxy on the MediaDecoder will unblock playback.
+    if (mDecoder) {
+      mDecoder->SetCDMProxy(aMediaKeys->GetCDMProxy());
     }
   }
+
+  // 5.4 Set the mediaKeys attribute to mediaKeys.
+  mMediaKeys = aMediaKeys;
+
+  // 5.5 Let this object's attaching media keys value be false.
+
+  // 5.6 Resolve promise.
   promise->MaybeResolve(JS::UndefinedHandleValue);
+
+  // 6. Return promise.
   return promise.forget();
 }
 

dom/media/eme/MediaKeys.cpp

@@ -504,5 +504,12 @@ MediaKeys::Bind(HTMLMediaElement* aElement)
   return NS_OK;
 }
 
+void
+MediaKeys::Unbind()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  mElement = nullptr;
+}
+
 } // namespace dom
 } // namespace mozilla

dom/media/eme/MediaKeys.h

@@ -55,6 +55,7 @@ public:
   virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   nsresult Bind(HTMLMediaElement* aElement);
+  void Unbind();
 
   // Javascript: readonly attribute DOMString keySystem;
   void GetKeySystem(nsString& retval) const;

dom/media/test/mochitest.ini

@@ -623,6 +623,8 @@ skip-if = (os == 'win' && os_version == '5.1') || (os != 'win' && toolkit != 'go
 [test_eme_session_callable_value.html]
 [test_eme_canvas_blocked.html]
 skip-if = toolkit == 'android' # bug 1149374
+[test_eme_detach_media_keys.html]
+skip-if = toolkit == 'android' # bug 1149374
 [test_eme_initDataTypes.html]
 skip-if = toolkit == 'android' # bug 1149374
 [test_eme_non_mse_fails.html]

dom/media/test/test_eme_detach_media_keys.html

@@ -0,0 +1,63 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test Encrypted Media Extensions</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <script type="text/javascript" src="manifest.js"></script>
+  <script type="text/javascript" src="eme.js"></script>
+</head>
+<body>
+<pre id="test">
+<video id="v" controls></video>
+<script class="testbody" type="text/javascript">
+
+SimpleTest.waitForExplicitFinish();
+
+const keysystem = 'org.w3.clearkey';
+
+function createAndSet() {
+  return new Promise(function(resolve, reject) {
+    var m;
+    navigator.requestMediaKeySystemAccess(keysystem, [{initDataType: 'cenc'}])
+    .then(function (access) {
+      return access.createMediaKeys();
+    }).then(function (mediaKeys) {
+      m = mediaKeys;
+      return document.getElementById("v").setMediaKeys(mediaKeys);
+    }).then(function() {
+      resolve(m);
+    });
+  }
+)}
+
+var m1,m2;
+
+// Test that if we create and set two MediaKeys on one video element,
+// that if the first MediaKeys we set on the media elemnt is still usable
+// after the second MediaKeys has been set on the media element.
+SetupEMEPref(() => {
+  createAndSet().then((m) => {
+    m1 = m; // Stash MediaKeys.
+    return createAndSet();
+  })
+  .then((m) => {
+    m2 = m;
+    is(document.getElementById("v").mediaKeys, m2, "Should have set MediaKeys on media element");
+    ok(document.getElementById("v").mediaKeys != m1, "First MediaKeys should no longer be set on media element");
+    var s = m1.createSession("temporary");
+    return s.generateRequest("webm", StringToArrayBuffer(atob('YAYeAX5Hfod+V9ANHtANHg==')));
+  })
+  .then(() => {
+    ok(true, "Was able to generateRequest using second CDM");
+    SimpleTest.finish();
+  }, () => {
+    ok(false, "Was *NOT* able to generateRequest using second CDM");
+    SimpleTest.finish();
+  });
+});
+  
+</script>
+</pre>
+</body>
+</html>