Title: [249548] trunk/Source/WebCore
Revision
249548
Author
ctur...@igalia.com
Date
2019-09-05 14:58:58 -0700 (Thu, 05 Sep 2019)

Log Message

[EME] Introduce a Proxy CDM for thread-safe access to CDM instances from background decryption threads
https://bugs.webkit.org/show_bug.cgi?id=201339

Reviewed by Xabier Rodriguez-Calvar.

Covered by existing tests.

* platform/encryptedmedia/CDMInstance.h:
* platform/encryptedmedia/clearkey/CDMClearKey.cpp:
(WebCore::isolatedKey):
(WebCore::ProxyCDMClearKey::isolatedKeys const):
(WebCore::CDMInstanceClearKey::CDMInstanceClearKey):
* platform/encryptedmedia/clearkey/CDMClearKey.h:
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::cdmInstanceAttached):
* platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:
(handleKeyResponse):
(findAndSetKey):
(decrypt):
* platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:
(isCDMInstanceAvailable):
(sinkEventHandler):
(setContext):
* platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.h:
* testing/MockCDMFactory.cpp:
(WebCore::MockCDMInstance::proxyCDM const):
* testing/MockCDMFactory.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (249547 => 249548)


--- trunk/Source/WebCore/ChangeLog	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/ChangeLog	2019-09-05 21:58:58 UTC (rev 249548)
@@ -1,3 +1,33 @@
+2019-09-05  Charlie Turner  <ctur...@igalia.com>
+
+        [EME] Introduce a Proxy CDM for thread-safe access to CDM instances from background decryption threads
+        https://bugs.webkit.org/show_bug.cgi?id=201339
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Covered by existing tests.
+
+        * platform/encryptedmedia/CDMInstance.h:
+        * platform/encryptedmedia/clearkey/CDMClearKey.cpp:
+        (WebCore::isolatedKey):
+        (WebCore::ProxyCDMClearKey::isolatedKeys const):
+        (WebCore::CDMInstanceClearKey::CDMInstanceClearKey):
+        * platform/encryptedmedia/clearkey/CDMClearKey.h:
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+        (WebCore::MediaPlayerPrivateGStreamerBase::cdmInstanceAttached):
+        * platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:
+        (handleKeyResponse):
+        (findAndSetKey):
+        (decrypt):
+        * platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:
+        (isCDMInstanceAvailable):
+        (sinkEventHandler):
+        (setContext):
+        * platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.h:
+        * testing/MockCDMFactory.cpp:
+        (WebCore::MockCDMInstance::proxyCDM const):
+        * testing/MockCDMFactory.h:
+
 2019-09-05  Zalan Bujtas  <za...@apple.com>
 
         [LFC] LayoutState should not need the initial containing block

Modified: trunk/Source/WebCore/platform/encryptedmedia/CDMInstance.h (249547 => 249548)


--- trunk/Source/WebCore/platform/encryptedmedia/CDMInstance.h	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/platform/encryptedmedia/CDMInstance.h	2019-09-05 21:58:58 UTC (rev 249548)
@@ -33,6 +33,7 @@
 #include <utility>
 #include <wtf/Forward.h>
 #include <wtf/RefCounted.h>
+#include <wtf/ThreadSafeRefCounted.h>
 #include <wtf/TypeCasts.h>
 
 namespace WebCore {
@@ -40,8 +41,19 @@
 class SharedBuffer;
 
 class CDMInstanceSession;
+class ProxyCDM;
+
+// Handle to a "real" CDM, not the _javascript_ facade. This can be used
+// from background threads (i.e. decryptors).
+class ProxyCDM : public ThreadSafeRefCounted<ProxyCDM> {
+public:
+    virtual ~ProxyCDM() = default;
+};
+
 struct CDMKeySystemConfiguration;
 
+// _javascript_'s handle to a CDMInstance, must be used from the
+// main-thread only!
 class CDMInstance : public RefCounted<CDMInstance> {
 public:
     virtual ~CDMInstance() = default;
@@ -65,6 +77,7 @@
     virtual SuccessValue setStorageDirectory(const String&) = 0;
     virtual const String& keySystem() const = 0;
     virtual RefPtr<CDMInstanceSession> createSession() = 0;
+    virtual RefPtr<ProxyCDM> proxyCDM() const = 0;
 
     enum class HDCPStatus {
         Unknown,

Modified: trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp (249547 => 249548)


--- trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp	2019-09-05 21:58:58 UTC (rev 249548)
@@ -455,7 +455,41 @@
     return sessionId;
 }
 
+// This is for thread-safety during an architectural situation that is
+// less than ideal. The GStreamer decryptors currently need to iterate
+// all known session keys to find the key data for priming
+// GCrypt. Ideally, all decryption would be the responsibility of
+// ProxyCDM object like this one. What the background GStreamer
+// thread was doing was getting copies (i.e. ref()'s) of SharedBuffers
+// created on the main-thread. With the new safety assertions in
+// WebKit, we can no longer do this. Instead, convert the refcounted
+// SharedBuffers into Strings which can be safely copied across
+// threads.
+static ProxyCDMClearKey::Key isolatedKey(const CDMInstanceClearKey::Key& key)
+{
+    return { key.status, String(key.keyIDData->data(), key.keyIDData->size()), String(key.keyValueData->data(), key.keyValueData->size()) };
+}
+
+const Vector<ProxyCDMClearKey::Key> ProxyCDMClearKey::isolatedKeys() const
+{
+    // Return the keys of all sessions, may be copied to background threads.
+    Vector<ProxyCDMClearKey::Key> allKeys { };
+    auto locker = holdLock(m_keysMutex);
+    size_t initialCapacity = 0;
+    for (auto& keyVector : ClearKeyState::singleton().keys().values())
+        initialCapacity += keyVector.size();
+    allKeys.reserveInitialCapacity(initialCapacity);
+
+    for (auto& keyVector : ClearKeyState::singleton().keys().values()) {
+        for (auto& key : keyVector)
+            allKeys.uncheckedAppend(isolatedKey(key));
+    }
+
+    return allKeys;
+}
+
 CDMInstanceClearKey::CDMInstanceClearKey()
+    : m_proxyCDM(adoptRef(*new ProxyCDMClearKey()))
 {
 }
 
@@ -503,22 +537,6 @@
     return adoptRef(new CDMInstanceSessionClearKey());
 }
 
-const Vector<CDMInstanceClearKey::Key> CDMInstanceClearKey::keys() const
-{
-    // Return the keys of all sessions.
-    Vector<CDMInstanceClearKey::Key> allKeys { };
-    auto locker = holdLock(m_keysMutex);
-    size_t initialCapacity = 0;
-    for (auto& key : ClearKeyState::singleton().keys().values())
-        initialCapacity += key.size();
-    allKeys.reserveInitialCapacity(initialCapacity);
-
-    for (auto& key : ClearKeyState::singleton().keys().values())
-        allKeys.appendVector(key);
-
-    return allKeys;
-}
-
 void CDMInstanceSessionClearKey::requestLicense(LicenseType, const AtomString& initDataType, Ref<SharedBuffer>&& initData, LicenseCallback&& callback)
 {
     static uint32_t s_sessionIdValue = 0;

Modified: trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.h (249547 => 249548)


--- trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.h	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.h	2019-09-05 21:58:58 UTC (rev 249548)
@@ -77,6 +77,20 @@
     Optional<String> sanitizeSessionId(const String&) const final;
 };
 
+class ProxyCDMClearKey final : public ProxyCDM {
+public:
+    struct Key {
+        CDMInstanceSession::KeyStatus status;
+        String keyIDData;
+        String keyValueData;
+    };
+
+    virtual ~ProxyCDMClearKey() = default;
+    const Vector<Key> isolatedKeys() const;
+private:
+    mutable Lock m_keysMutex;
+};
+
 class CDMInstanceClearKey final : public CDMInstance, public CanMakeWeakPtr<CDMInstanceClearKey> {
 public:
     CDMInstanceClearKey();
@@ -98,10 +112,10 @@
         RefPtr<SharedBuffer> keyValueData;
     };
 
-    const Vector<Key> keys() const;
+    RefPtr<ProxyCDM> proxyCDM() const final { return m_proxyCDM; }
 
 private:
-    mutable Lock m_keysMutex;
+    RefPtr<ProxyCDM> m_proxyCDM;
 };
 
 class CDMInstanceSessionClearKey final : public CDMInstanceSession, public CanMakeWeakPtr<CDMInstanceSessionClearKey> {

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.h (249547 => 249548)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.h	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.h	2019-09-05 21:58:58 UTC (rev 249548)
@@ -63,6 +63,7 @@
     SuccessValue setServerCertificate(Ref<SharedBuffer>&&) final;
     SuccessValue setStorageDirectory(const String&) final;
     RefPtr<CDMInstanceSession> createSession() final;
+    RefPtr<ProxyCDM> proxyCDM() const final { return nullptr; }
 
     const String& keySystem() const final;
 

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (249547 => 249548)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp	2019-09-05 21:58:58 UTC (rev 249548)
@@ -1430,10 +1430,10 @@
 
     GRefPtr<GstContext> context = adoptGRef(gst_context_new("drm-cdm-instance", FALSE));
     GstStructure* contextStructure = gst_context_writable_structure(context.get());
-    gst_structure_set(contextStructure, "cdm-instance", G_TYPE_POINTER, m_cdmInstance.get(), nullptr);
+    gst_structure_set(contextStructure, "cdm-instance", G_TYPE_POINTER, m_cdmInstance->proxyCDM().get(), nullptr);
     gst_element_set_context(GST_ELEMENT(m_pipeline.get()), context.get());
 
-    GST_DEBUG_OBJECT(m_pipeline.get(), "CDM instance %p dispatched as context", m_cdmInstance.get());
+    GST_DEBUG_OBJECT(m_pipeline.get(), "CDM proxy instance %p dispatched as context", m_cdmInstance->proxyCDM().get());
 
     m_cdmAttachmentSemaphore.signal();
 }

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp (249547 => 249548)


--- trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp	2019-09-05 21:58:58 UTC (rev 249548)
@@ -37,12 +37,12 @@
 
 #define WEBKIT_MEDIA_CK_DECRYPT_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE((obj), WEBKIT_TYPE_MEDIA_CK_DECRYPT, WebKitMediaClearKeyDecryptPrivate))
 struct _WebKitMediaClearKeyDecryptPrivate {
-    RefPtr<CDMInstanceClearKey> cdmInstance;
+    RefPtr<ProxyCDMClearKey> proxyCDM;
     gcry_cipher_hd_t handle;
 };
 
 static void finalize(GObject*);
-static bool handleKeyResponse(WebKitMediaCommonEncryptionDecrypt* self, RefPtr<CDMInstance>);
+static bool handleKeyResponse(WebKitMediaCommonEncryptionDecrypt* self, RefPtr<ProxyCDM>);
 static bool decrypt(WebKitMediaCommonEncryptionDecrypt*, GstBuffer* iv, GstBuffer* keyid, GstBuffer* sample, unsigned subSamplesCount, GstBuffer* subSamples);
 
 GST_DEBUG_CATEGORY_STATIC(webkit_media_clear_key_decrypt_debug_category);
@@ -112,30 +112,31 @@
     GST_CALL_PARENT(G_OBJECT_CLASS, finalize, (object));
 }
 
-static bool handleKeyResponse(WebKitMediaCommonEncryptionDecrypt* self, RefPtr<CDMInstance> cdmInstance)
+static bool handleKeyResponse(WebKitMediaCommonEncryptionDecrypt* self, RefPtr<ProxyCDM> proxyCDM)
 {
     WebKitMediaClearKeyDecryptPrivate* priv = WEBKIT_MEDIA_CK_DECRYPT_GET_PRIVATE(WEBKIT_MEDIA_CK_DECRYPT(self));
-    priv->cdmInstance = downcast<CDMInstanceClearKey>(cdmInstance.get());
-    return priv->cdmInstance;
+    priv->proxyCDM = reinterpret_cast<ProxyCDMClearKey*>(proxyCDM.get());
+    return priv->proxyCDM;
 }
 
-static bool findAndSetKey(WebKitMediaClearKeyDecryptPrivate* priv, const Ref<SharedBuffer>& keyID)
+static bool findAndSetKey(WebKitMediaClearKeyDecryptPrivate* priv, const String&& keyID)
 {
-    RefPtr<SharedBuffer> keyValue;
-    for (const auto& key : priv->cdmInstance->keys()) {
-        if (*key.keyIDData == keyID) {
+    String keyValue;
+
+    for (const auto& key : priv->proxyCDM->isolatedKeys()) {
+        if (key.keyIDData == keyID) {
             keyValue = key.keyValueData;
             break;
         }
     }
 
-    if (!keyValue) {
+    if (keyValue.isEmpty()) {
         GST_ERROR_OBJECT(priv, "failed to find a decryption key");
         return false;
     }
 
-    ASSERT(keyValue->size() == CLEARKEY_SIZE);
-    if (gcry_error_t error = gcry_cipher_setkey(priv->handle, keyValue->data(), keyValue->size())) {
+    ASSERT(keyValue.sizeInBytes() == CLEARKEY_SIZE);
+    if (gcry_error_t error = gcry_cipher_setkey(priv->handle, keyValue.utf8().data(), keyValue.sizeInBytes())) {
         GST_ERROR_OBJECT(priv, "gcry_cipher_setkey failed: %s", gpg_strerror(error));
         return false;
     }
@@ -189,7 +190,7 @@
         return false;
     }
 
-    findAndSetKey(priv, mappedKeyIdBuffer->createSharedBuffer());
+    findAndSetKey(priv, String(mappedKeyIdBuffer->data(), mappedKeyIdBuffer->size()));
 
     unsigned position = 0;
     unsigned sampleIndex = 0;

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp (249547 => 249548)


--- trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp	2019-09-05 21:58:58 UTC (rev 249548)
@@ -31,12 +31,12 @@
 #include <wtf/PrintStream.h>
 #include <wtf/RunLoop.h>
 
-using WebCore::CDMInstance;
+using WebCore::ProxyCDM;
 
 #define WEBKIT_MEDIA_CENC_DECRYPT_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE((obj), WEBKIT_TYPE_MEDIA_CENC_DECRYPT, WebKitMediaCommonEncryptionDecryptPrivate))
 struct _WebKitMediaCommonEncryptionDecryptPrivate {
     GRefPtr<GstEvent> protectionEvent;
-    RefPtr<CDMInstance> cdmInstance;
+    RefPtr<ProxyCDM> proxyCDM;
     bool keyReceived { false };
     bool waitingForKey { false };
     Lock mutex;
@@ -284,7 +284,7 @@
 
     ASSERT(priv->mutex.isLocked());
 
-    if (!priv->cdmInstance) {
+    if (!priv->proxyCDM) {
         GRefPtr<GstContext> context = adoptGRef(gst_element_get_context(GST_ELEMENT(self), "drm-cdm-instance"));
         // According to the GStreamer documentation, if we can't find the context, we should run a downstream query, then an upstream one and then send a bus
         // message. In this case that does not make a lot of sense since only the app (player) answers it, meaning that no query is going to solve it. A message
@@ -293,16 +293,16 @@
         // requests are useful here.
         if (context) {
             const GValue* value = gst_structure_get_value(gst_context_get_structure(context.get()), "cdm-instance");
-            priv->cdmInstance = value ? reinterpret_cast<CDMInstance*>(g_value_get_pointer(value)) : nullptr;
-            if (priv->cdmInstance)
-                GST_DEBUG_OBJECT(self, "received a new CDM instance %p, refcount %u", priv->cdmInstance.get(), priv->cdmInstance->refCount());
+            priv->proxyCDM = value ? reinterpret_cast<ProxyCDM*>(g_value_get_pointer(value)) : nullptr;
+            if (priv->proxyCDM)
+                GST_DEBUG_OBJECT(self, "received a new CDM proxy instance %p, refcount %u", priv->proxyCDM.get(), priv->proxyCDM->refCount());
             else
                 GST_TRACE_OBJECT(self, "CDM instance was detached");
         }
     }
 
-    GST_TRACE_OBJECT(self, "CDM instance available %s", boolForPrinting(priv->cdmInstance.get()));
-    return priv->cdmInstance;
+    GST_TRACE_OBJECT(self, "CDM instance available %s", boolForPrinting(priv->proxyCDM.get()));
+    return priv->proxyCDM;
 }
 
 static gboolean sinkEventHandler(GstBaseTransform* trans, GstEvent* event)
@@ -327,7 +327,7 @@
             break;
         }
 
-        if (klass->handleKeyResponse(self, priv->cdmInstance)) {
+        if (klass->handleKeyResponse(self, priv->proxyCDM)) {
             GST_DEBUG_OBJECT(self, "key received");
             priv->keyReceived = true;
             priv->condition.notifyOne();
@@ -384,8 +384,8 @@
     if (gst_context_has_context_type(context, "drm-cdm-instance")) {
         const GValue* value = gst_structure_get_value(gst_context_get_structure(context), "cdm-instance");
         LockHolder locker(priv->mutex);
-        priv->cdmInstance = value ? reinterpret_cast<CDMInstance*>(g_value_get_pointer(value)) : nullptr;
-        GST_DEBUG_OBJECT(self, "received new CDMInstance %p", priv->cdmInstance.get());
+        priv->proxyCDM = value ? reinterpret_cast<ProxyCDM*>(g_value_get_pointer(value)) : nullptr;
+        GST_DEBUG_OBJECT(self, "received new CDMInstance %p", priv->proxyCDM.get());
         return;
     }
 

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.h (249547 => 249548)


--- trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.h	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.h	2019-09-05 21:58:58 UTC (rev 249548)
@@ -55,7 +55,7 @@
     GstBaseTransformClass parentClass;
 
     const char* protectionSystemId;
-    bool (*handleKeyResponse)(WebKitMediaCommonEncryptionDecrypt*, RefPtr<WebCore::CDMInstance>);
+    bool (*handleKeyResponse)(WebKitMediaCommonEncryptionDecrypt*, RefPtr<WebCore::ProxyCDM>);
     bool (*decrypt)(WebKitMediaCommonEncryptionDecrypt*, GstBuffer* ivBuffer, GstBuffer* keyIDBuffer, GstBuffer* buffer, unsigned subSamplesCount, GstBuffer* subSamplesBuffer);
 };
 

Modified: trunk/Source/WebCore/testing/MockCDMFactory.cpp (249547 => 249548)


--- trunk/Source/WebCore/testing/MockCDMFactory.cpp	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/testing/MockCDMFactory.cpp	2019-09-05 21:58:58 UTC (rev 249548)
@@ -226,6 +226,8 @@
     return WTF::nullopt;
 }
 
+ProxyCDMMock::~ProxyCDMMock() = default;
+
 MockCDMInstance::MockCDMInstance(WeakPtr<MockCDM> cdm)
     : m_cdm(cdm)
 {
@@ -294,6 +296,11 @@
     return adoptRef(new MockCDMInstanceSession(makeWeakPtr(*this)));
 }
 
+RefPtr<ProxyCDM> MockCDMInstance::proxyCDM() const
+{
+    return adoptRef(new ProxyCDMMock());
+}
+
 MockCDMInstanceSession::MockCDMInstanceSession(WeakPtr<MockCDMInstance>&& instance)
     : m_instance(WTFMove(instance))
 {

Modified: trunk/Source/WebCore/testing/MockCDMFactory.h (249547 => 249548)


--- trunk/Source/WebCore/testing/MockCDMFactory.h	2019-09-05 21:48:43 UTC (rev 249547)
+++ trunk/Source/WebCore/testing/MockCDMFactory.h	2019-09-05 21:58:58 UTC (rev 249548)
@@ -99,6 +99,11 @@
     HashMap<String, Vector<Ref<SharedBuffer>>> m_sessions;
 };
 
+class ProxyCDMMock final : public ProxyCDM {
+public:
+    virtual ~ProxyCDMMock();
+};
+
 class MockCDM : public CDMPrivate, public CanMakeWeakPtr<MockCDM> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
@@ -145,6 +150,7 @@
     SuccessValue setStorageDirectory(const String&) final;
     const String& keySystem() const final;
     RefPtr<CDMInstanceSession> createSession() final;
+    RefPtr<ProxyCDM> proxyCDM() const final;
 
     WeakPtr<MockCDM> m_cdm;
     bool m_distinctiveIdentifiersAllowed { true };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to