- Revision
- 239236
- Author
- jer.no...@apple.com
- Date
- 2018-12-14 14:59:36 -0800 (Fri, 14 Dec 2018)
Log Message
CRASH in CDMInstanceSessionFairPlayStreamingAVFObjC::closeSession(WTF::String const&, WTF::Function<void ()>&&)
https://bugs.webkit.org/show_bug.cgi?id=192713
<rdar://problem/46739706>
Reviewed by Eric Carlson.
A callback is being called twice, and the second time has a null Promise. Instead of these
callbacks being WTF::Function, make them WTF::CompletionHandlers, which self-nullify and
have ASSERTS() that they are called once-and-only-once.
* platform/encryptedmedia/CDMInstanceSession.h:
* platform/encryptedmedia/clearkey/CDMClearKey.cpp:
(WebCore::CDMInstanceSessionClearKey::closeSession):
* platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:
(WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::closeSession):
(WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::didProvideRequest):
(WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::didProvideRenewingRequest):
(WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::didFailToProvideRequest):
(WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::requestDidSucceed):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (239235 => 239236)
--- trunk/Source/WebCore/ChangeLog 2018-12-14 22:49:07 UTC (rev 239235)
+++ trunk/Source/WebCore/ChangeLog 2018-12-14 22:59:36 UTC (rev 239236)
@@ -1,3 +1,25 @@
+2018-12-14 Jer Noble <jer.no...@apple.com>
+
+ CRASH in CDMInstanceSessionFairPlayStreamingAVFObjC::closeSession(WTF::String const&, WTF::Function<void ()>&&)
+ https://bugs.webkit.org/show_bug.cgi?id=192713
+ <rdar://problem/46739706>
+
+ Reviewed by Eric Carlson.
+
+ A callback is being called twice, and the second time has a null Promise. Instead of these
+ callbacks being WTF::Function, make them WTF::CompletionHandlers, which self-nullify and
+ have ASSERTS() that they are called once-and-only-once.
+
+ * platform/encryptedmedia/CDMInstanceSession.h:
+ * platform/encryptedmedia/clearkey/CDMClearKey.cpp:
+ (WebCore::CDMInstanceSessionClearKey::closeSession):
+ * platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:
+ (WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::closeSession):
+ (WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::didProvideRequest):
+ (WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::didProvideRenewingRequest):
+ (WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::didFailToProvideRequest):
+ (WebCore::CDMInstanceSessionFairPlayStreamingAVFObjC::requestDidSucceed):
+
2018-12-14 David Kilzer <ddkil...@apple.com>
clang-tidy: Fix unnecessary object copies in WebCore/platform/graphics/avfoundation/objc/
Modified: trunk/Source/WebCore/platform/encryptedmedia/CDMInstanceSession.h (239235 => 239236)
--- trunk/Source/WebCore/platform/encryptedmedia/CDMInstanceSession.h 2018-12-14 22:49:07 UTC (rev 239235)
+++ trunk/Source/WebCore/platform/encryptedmedia/CDMInstanceSession.h 2018-12-14 22:59:36 UTC (rev 239236)
@@ -30,6 +30,7 @@
#include "CDMKeyStatus.h"
#include "CDMMessageType.h"
#include "CDMSessionType.h"
+#include <wtf/CompletionHandler.h>
#include <wtf/RefCounted.h>
#include <wtf/Vector.h>
#include <wtf/WeakPtr.h>
@@ -64,12 +65,12 @@
Succeeded,
};
- using LicenseCallback = Function<void(Ref<SharedBuffer>&& message, const String& sessionId, bool needsIndividualization, SuccessValue succeeded)>;
+ using LicenseCallback = CompletionHandler<void(Ref<SharedBuffer>&& message, const String& sessionId, bool needsIndividualization, SuccessValue succeeded)>;
virtual void requestLicense(LicenseType, const AtomicString& initDataType, Ref<SharedBuffer>&& initData, LicenseCallback&&) = 0;
using KeyStatusVector = CDMInstanceSessionClient::KeyStatusVector;
using Message = std::pair<MessageType, Ref<SharedBuffer>>;
- using LicenseUpdateCallback = Function<void(bool sessionWasClosed, std::optional<KeyStatusVector>&& changedKeys, std::optional<double>&& changedExpiration, std::optional<Message>&& message, SuccessValue succeeded)>;
+ using LicenseUpdateCallback = CompletionHandler<void(bool sessionWasClosed, std::optional<KeyStatusVector>&& changedKeys, std::optional<double>&& changedExpiration, std::optional<Message>&& message, SuccessValue succeeded)>;
virtual void updateLicense(const String& sessionId, LicenseType, const SharedBuffer& response, LicenseUpdateCallback&&) = 0;
enum class SessionLoadFailure {
@@ -80,13 +81,13 @@
Other,
};
- using LoadSessionCallback = Function<void(std::optional<KeyStatusVector>&&, std::optional<double>&&, std::optional<Message>&&, SuccessValue, SessionLoadFailure)>;
+ using LoadSessionCallback = CompletionHandler<void(std::optional<KeyStatusVector>&&, std::optional<double>&&, std::optional<Message>&&, SuccessValue, SessionLoadFailure)>;
virtual void loadSession(LicenseType, const String& sessionId, const String& origin, LoadSessionCallback&&) = 0;
- using CloseSessionCallback = Function<void()>;
+ using CloseSessionCallback = CompletionHandler<void()>;
virtual void closeSession(const String& sessionId, CloseSessionCallback&&) = 0;
- using RemoveSessionDataCallback = Function<void(KeyStatusVector&&, std::optional<Ref<SharedBuffer>>&&, SuccessValue)>;
+ using RemoveSessionDataCallback = CompletionHandler<void(KeyStatusVector&&, std::optional<Ref<SharedBuffer>>&&, SuccessValue)>;
virtual void removeSessionData(const String& sessionId, LicenseType, RemoveSessionDataCallback&&) = 0;
virtual void storeRecordOfKeyUsage(const String& sessionId) = 0;
Modified: trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp (239235 => 239236)
--- trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp 2018-12-14 22:49:07 UTC (rev 239235)
+++ trunk/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp 2018-12-14 22:59:36 UTC (rev 239236)
@@ -676,7 +676,7 @@
void CDMInstanceSessionClearKey::closeSession(const String&, CloseSessionCallback&& callback)
{
callOnMainThread(
- [weakThis = makeWeakPtr(*this), callback = WTFMove(callback)] {
+ [weakThis = makeWeakPtr(*this), callback = WTFMove(callback)] () mutable {
if (!weakThis)
return;
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm (239235 => 239236)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm 2018-12-14 22:49:07 UTC (rev 239235)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm 2018-12-14 22:59:36 UTC (rev 239236)
@@ -428,15 +428,15 @@
{
if (m_requestLicenseCallback) {
m_requestLicenseCallback(SharedBuffer::create(), m_sessionId, false, Failed);
- m_requestLicenseCallback = nullptr;
+ ASSERT(!m_requestLicenseCallback);
}
if (m_updateLicenseCallback) {
m_updateLicenseCallback(true, std::nullopt, std::nullopt, std::nullopt, Failed);
- m_updateLicenseCallback = nullptr;
+ ASSERT(!m_updateLicenseCallback);
}
if (m_removeSessionDataCallback) {
m_removeSessionDataCallback({ }, std::nullopt, Failed);
- m_removeSessionDataCallback = nullptr;
+ ASSERT(!m_removeSessionDataCallback);
}
m_currentRequest = nullptr;
m_pendingRequests.clear();
@@ -516,7 +516,7 @@
if (keyIDs.isEmpty()) {
if (m_requestLicenseCallback) {
m_requestLicenseCallback(SharedBuffer::create(), m_sessionId, false, Failed);
- m_requestLicenseCallback = nullptr;
+ ASSERT(!m_requestLicenseCallback);
}
return;
}
@@ -533,7 +533,7 @@
m_requestLicenseCallback(SharedBuffer::create(contentKeyRequestData.get()), m_sessionId, false, Succeeded);
else if (m_client)
m_client->sendMessage(CDMMessageType::LicenseRequest, SharedBuffer::create(contentKeyRequestData.get()));
- m_requestLicenseCallback = nullptr;
+ ASSERT(!m_requestLicenseCallback);
});
}];
}
@@ -569,7 +569,7 @@
m_updateLicenseCallback(false, std::nullopt, std::nullopt, Message(MessageType::LicenseRenewal, SharedBuffer::create(contentKeyRequestData.get())), Succeeded);
else if (m_client)
m_client->sendMessage(CDMMessageType::LicenseRenewal, SharedBuffer::create(contentKeyRequestData.get()));
- m_updateLicenseCallback = nullptr;
+ ASSERT(!m_updateLicenseCallback);
});
}];
}
@@ -583,8 +583,10 @@
{
UNUSED_PARAM(request);
UNUSED_PARAM(error);
- if (m_updateLicenseCallback)
+ if (m_updateLicenseCallback) {
m_updateLicenseCallback(false, std::nullopt, std::nullopt, std::nullopt, Failed);
+ ASSERT(!m_updateLicenseCallback);
+ }
m_currentRequest = nullptr;
@@ -594,8 +596,10 @@
void CDMInstanceSessionFairPlayStreamingAVFObjC::requestDidSucceed(AVContentKeyRequest *request)
{
UNUSED_PARAM(request);
- if (m_updateLicenseCallback)
+ if (m_updateLicenseCallback) {
m_updateLicenseCallback(false, std::make_optional(keyStatuses()), std::nullopt, std::nullopt, Succeeded);
+ ASSERT(!m_updateLicenseCallback);
+ }
m_currentRequest = nullptr;
