Skip to content

Commit ccbb8b2

Browse files
committed
[GStreamer][EME] Reworked the reference counting of sessions
https://bugs.webkit.org/show_bug.cgi?id=273490 Reviewed by Philippe Normand. Now they are properly accounted for and disposed. A fly-by is fixing the name of for the isKeyAvailable methods to make them code-style compliant. * Source/WebCore/platform/encryptedmedia/CDMProxy.cpp: (WebCore::KeyHandle::takeValueIfDifferent): (WebCore::keyStoreBaseNextID): (WebCore::ReferenceAwareKeyStore::unrefAllKeysFrom): (WebCore::ReferenceAwareKeyStore::merge): (WebCore::CDMProxy::tryWaitForKeyHandle const): (WebCore::CDMProxy::isKeyAvailableUnlocked const): (WebCore::CDMProxy::isKeyAvailable const): (WebCore::CDMProxy::getOrWaitForKeyHandle const): (WebCore::KeyStore::containsKeyID const): Deleted. (WebCore::KeyStore::merge): Deleted. (WebCore::KeyStore::allKeysAs const): Deleted. (WebCore::KeyStore::addKeys): Deleted. (WebCore::KeyStore::add): Deleted. (WebCore::KeyStore::unrefAllKeysFrom): Deleted. (WebCore::KeyStore::unrefAllKeys): Deleted. (WebCore::KeyStore::unref): Deleted. (WebCore::KeyStore::keyHandle const): Deleted. (WebCore::KeyStore::convertToJSKeyStatusVector const): Deleted. (WebCore::CDMProxy::keyAvailableUnlocked const): Deleted. (WebCore::CDMProxy::keyAvailable const): Deleted. * Source/WebCore/platform/encryptedmedia/CDMProxy.h: (WebCore::KeyHandle::status const): (WebCore::KeyHandle::operator==): (WebCore::KeyHandle::KeyHandle): (WebCore::KeyStoreBase::KeyStoreBase): (WebCore::KeyStoreBase::add): (WebCore::KeyStoreBase::addKeys): (WebCore::KeyStoreBase::remove): (WebCore::KeyStoreBase::clear): (WebCore::KeyStoreBase::containsKeyID const): (WebCore::KeyStoreBase::keyHandle const): (WebCore::KeyStoreBase::allKeysAs const): (WebCore::KeyStoreBase::convertToJSKeyStatusVector const): (WebCore::KeyStoreBase::numKeys const): (WebCore::KeyStoreBase::values const): (WebCore::KeyStoreBase::id const): (WebCore::ReferenceAwareKeyHandle::createFrom): (WebCore::ReferenceAwareKeyHandle::updateKeyFrom): (WebCore::ReferenceAwareKeyHandle::hasReferences const): (WebCore::ReferenceAwareKeyHandle::ReferenceAwareKeyHandle): (WebCore::ReferenceAwareKeyHandle::removeReference): (WebCore::KeyHandle::mergeKeyInto): Deleted. (WebCore::KeyHandle::operator<): Deleted. (WebCore::KeyHandle::addSessionReference): Deleted. (WebCore::KeyHandle::removeSessionReference): Deleted. (WebCore::KeyHandle::numSessionReferences const): Deleted. (WebCore::KeyHandle::hasReferences const): Deleted. (WebCore::KeyStore::hasKeys const): Deleted. (WebCore::KeyStore::numKeys const): Deleted. (WebCore::KeyStore::isEmpty const): Deleted. (WebCore::KeyStore::addSessionReferenceTo const): Deleted. (WebCore::KeyStore::removeSessionReferenceFrom const): Deleted. (WebCore::KeyStore::begin): Deleted. (WebCore::KeyStore::begin const): Deleted. (WebCore::KeyStore::end): Deleted. (WebCore::KeyStore::end const): Deleted. (WebCore::KeyStore::rbegin): Deleted. (WebCore::KeyStore::rbegin const): Deleted. (WebCore::KeyStore::rend): Deleted. (WebCore::KeyStore::rend const): Deleted. * Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp: (WebCore::CDMInstanceSessionClearKey::updateLicense): (WebCore::CDMInstanceSessionClearKey::removeSessionData): * Source/WebCore/platform/graphics/gstreamer/eme/CDMThunder.cpp: (WebCore::CDMInstanceSessionThunder::closeSession): Canonical link: https://commits.webkit.org/278272@main
1 parent 7b9a46d commit ccbb8b2

4 files changed

Lines changed: 167 additions & 189 deletions

File tree

Source/WebCore/platform/encryptedmedia/CDMProxy.cpp

Lines changed: 36 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,15 @@ Vector<CDMProxyFactory*> CDMProxyFactory::platformRegisterFactories()
8383
}
8484
#endif
8585

86+
bool KeyHandle::takeValueIfDifferent(KeyHandleValueVariant&& value)
87+
{
88+
if (m_value != value) {
89+
m_value = WTFMove(value);
90+
return true;
91+
}
92+
return false;
93+
}
94+
8695
namespace {
8796

8897
static String vectorToHexString(const Vector<uint8_t>& vec)
@@ -100,135 +109,38 @@ String KeyHandle::idAsString() const
100109
return makeString("[", vectorToHexString(m_id), "]");
101110
}
102111

103-
bool KeyHandle::takeValueIfDifferent(KeyHandleValueVariant&& value)
104-
{
105-
if (m_value != value) {
106-
m_value = WTFMove(value);
107-
return true;
108-
}
109-
return false;
110-
}
111-
112-
bool KeyStore::containsKeyID(const KeyIDType& keyID) const
113-
{
114-
return m_keys.findIf([&](const RefPtr<KeyHandle>& storedKey) {
115-
return *storedKey == keyID;
116-
}) != notFound;
117-
}
118-
119-
void KeyStore::merge(const KeyStore& other)
112+
KeyStoreIDType keyStoreBaseNextID()
120113
{
114+
static KeyStoreIDType nextID = 1;
121115
ASSERT(isMainThread());
122-
LOG(EME, "EME - CDMProxy - merging %u new keys into a key store of %u keys", other.numKeys(), numKeys());
123-
for (const auto& key : other)
124-
add(key.copyRef());
125-
126-
#if !LOG_DISABLED
127-
LOG(EME, "EME - CDMProxy - key store now has %u keys", numKeys());
128-
for (const auto& key : m_keys)
129-
LOG(EME, "\tEME - CDMProxy - Key ID: %s", key->idAsString().ascii().data());
130-
#endif // !LOG_DISABLED
131-
}
132-
133-
CDMInstanceSession::KeyStatusVector KeyStore::allKeysAs(CDMInstanceSession::KeyStatus status) const
134-
{
135-
CDMInstanceSession::KeyStatusVector keyStatusVector = convertToJSKeyStatusVector();
136-
for (auto& keyStatus : keyStatusVector)
137-
keyStatus.second = status;
138-
return keyStatusVector;
116+
return nextID++;
139117
}
140118

141-
bool KeyStore::addKeys(Vector<RefPtr<KeyHandle>>&& newKeys)
119+
void ReferenceAwareKeyStore::unrefAllKeysFrom(const KeyStore& otherStore)
142120
{
143-
bool didKeyStoreChange = false;
144-
for (auto& key : newKeys) {
145-
if (add(WTFMove(key)))
146-
didKeyStoreChange = true;
121+
for (const auto& otherKey : otherStore.values()) {
122+
auto findingResult = m_keys.find(otherKey->id());
123+
if (findingResult == m_keys.end())
124+
continue;
125+
const RefPtr<ReferenceAwareKeyHandle>& key = findingResult->value;
126+
RELEASE_ASSERT(key);
127+
key->removeReference(otherStore.id());
128+
if (!key->hasReferences())
129+
remove(key);
147130
}
148-
return didKeyStoreChange;
149131
}
150132

151-
bool KeyStore::add(RefPtr<KeyHandle>&& key)
133+
void ReferenceAwareKeyStore::merge(const KeyStore& otherStore)
152134
{
153-
bool didStoreChange = false;
154-
size_t keyWithMatchingKeyIDIndex = m_keys.findIf([&] (const RefPtr<KeyHandle>& storedKey) {
155-
return *key == *storedKey;
156-
});
157-
158-
addSessionReferenceTo(key);
159-
if (keyWithMatchingKeyIDIndex != notFound) {
160-
auto& keyWithMatchingKeyID = m_keys[keyWithMatchingKeyIDIndex];
161-
didStoreChange = keyWithMatchingKeyID != key;
162-
if (didStoreChange)
163-
keyWithMatchingKeyID->mergeKeyInto(WTFMove(key));
164-
} else {
165-
LOG(EME, "EME - ClearKey - New key with ID %s getting added to key store", key->idAsString().ascii().data());
166-
m_keys.append(WTFMove(key));
167-
didStoreChange = true;
168-
}
169-
170-
if (didStoreChange) {
171-
// Sort the keys lexicographically.
172-
// NOTE: This is not as pathological as it may seem, for all
173-
// practical purposes the store has a maximum of 2 keys.
174-
std::sort(m_keys.begin(), m_keys.end(),
175-
[](const RefPtr<KeyHandle>& a, const RefPtr<KeyHandle>& b) {
176-
return *a < *b;
177-
});
178-
}
179-
180-
return didStoreChange;
181-
}
182-
183-
void KeyStore::unrefAllKeysFrom(const KeyStore& other)
184-
{
185-
for (const auto& key : other)
186-
unref(key);
187-
}
188-
189-
void KeyStore::unrefAllKeys()
190-
{
191-
KeyStore store(*this);
192-
unrefAllKeysFrom(store);
193-
}
194-
195-
bool KeyStore::unref(const RefPtr<KeyHandle>& key)
196-
{
197-
bool storeChanged = false;
198-
199-
size_t keyWithMatchingKeyIDIndex = m_keys.find(key);
200-
LOG(EME, "EME - ClearKey - requested to unref key with ID %s and %d session references", key->idAsString().ascii().data(), key->numSessionReferences());
201-
202-
if (keyWithMatchingKeyIDIndex != notFound) {
203-
auto& keyWithMatchingKeyID = m_keys[keyWithMatchingKeyIDIndex];
204-
removeSessionReferenceFrom(keyWithMatchingKeyID);
205-
if (!keyWithMatchingKeyID->hasReferences()) {
206-
LOG(EME, "EME - ClearKey - unref key with ID %s", keyWithMatchingKeyID->idAsString().ascii().data());
207-
m_keys.remove(keyWithMatchingKeyIDIndex);
208-
storeChanged = true;
209-
}
210-
} else
211-
LOG(EME, "EME - ClearKey - attempt to unref key with ID %s ignored, does not exist", key->idAsString().ascii().data());
212-
213-
return storeChanged;
214-
}
215-
216-
const RefPtr<KeyHandle>& KeyStore::keyHandle(const KeyIDType& keyID) const
217-
{
218-
for (const auto& key : m_keys) {
219-
if (*key == keyID)
220-
return key;
135+
ASSERT(isMainThread());
136+
for (const auto& otherKey : otherStore.values()) {
137+
RefPtr<ReferenceAwareKeyHandle> key = keyHandle(otherKey->id());
138+
auto otherReferenceAwareKey = ReferenceAwareKeyHandle::createFrom(otherKey, otherStore.id());
139+
if (key)
140+
key->updateKeyFrom(WTFMove(otherReferenceAwareKey));
141+
else
142+
add(WTFMove(otherReferenceAwareKey));
221143
}
222-
223-
RELEASE_ASSERT(false && "key must exist to call this method");
224-
UNREACHABLE();
225-
}
226-
227-
CDMInstanceSession::KeyStatusVector KeyStore::convertToJSKeyStatusVector() const
228-
{
229-
return m_keys.map([](auto& key) {
230-
return std::pair { key->idAsSharedBuffer(), key->status() };
231-
});
232144
}
233145

234146
void CDMProxy::updateKeyStore(const KeyStore& newKeyStore)
@@ -306,7 +218,7 @@ std::optional<Ref<KeyHandle>> CDMProxy::tryWaitForKeyHandle(const KeyIDType& key
306218
assertIsHeld(m_keysLock);
307219
if (!client || client->isAborting())
308220
return true;
309-
wasKeyAvailable = keyAvailableUnlocked(keyID);
221+
wasKeyAvailable = isKeyAvailableUnlocked(keyID);
310222
return wasKeyAvailable;
311223
});
312224
}
@@ -321,20 +233,20 @@ std::optional<Ref<KeyHandle>> CDMProxy::tryWaitForKeyHandle(const KeyIDType& key
321233
return std::nullopt;
322234
}
323235

324-
bool CDMProxy::keyAvailableUnlocked(const KeyIDType& keyID) const
236+
bool CDMProxy::isKeyAvailableUnlocked(const KeyIDType& keyID) const
325237
{
326238
return m_keyStore.containsKeyID(keyID);
327239
}
328240

329-
bool CDMProxy::keyAvailable(const KeyIDType& keyID) const
241+
bool CDMProxy::isKeyAvailable(const KeyIDType& keyID) const
330242
{
331243
Locker locker { m_keysLock };
332-
return keyAvailableUnlocked(keyID);
244+
return isKeyAvailableUnlocked(keyID);
333245
}
334246

335247
std::optional<Ref<KeyHandle>> CDMProxy::getOrWaitForKeyHandle(const KeyIDType& keyID, WeakPtr<CDMProxyDecryptionClient>&& client) const
336248
{
337-
if (!keyAvailable(keyID)) {
249+
if (!isKeyAvailable(keyID)) {
338250
LOG(EME, "EME - CDMProxy key cache does not contain key ID %s", vectorToHexString(keyID).ascii().data());
339251
return tryWaitForKeyHandle(keyID, WTFMove(client));
340252
}

0 commit comments

Comments
 (0)