Attachment 357336 Details for Bug 192713 – Patch

[patch] Patch

bug-192713-20181214133628.patch (text/plain), 8.90 KB, created by Jer Noble on 2018-12-14 13:36:29 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Jer Noble

Created: 2018-12-14 13:36:29 PST

Size: 8.90 KB

patch

obsolete

>Subversion Revision: 238907
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index fbd64e45e3fb9d3330689a443d9ff7bd659d4f07..846de468f84fc60e7aff21245032f03239cce8b2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2018-12-14  Jer Noble  <jer.noble@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 NOBODY (OOPS!).
>+
>+        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-13  Jer Noble  <jer.noble@apple.com>
> 
>         Fix leak of AVPlayer boundaryTimeObserver object.
>diff --git a/Source/WebCore/platform/encryptedmedia/CDMInstanceSession.h b/Source/WebCore/platform/encryptedmedia/CDMInstanceSession.h
>index aa48f373426de8fadc9d3001e4033ec139cdbf99..c538c4efb33aec88576f041d8d8ac7e25eabb522 100644
>--- a/Source/WebCore/platform/encryptedmedia/CDMInstanceSession.h
>+++ b/Source/WebCore/platform/encryptedmedia/CDMInstanceSession.h
>@@ -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 @@ public:
>         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 @@ public:
>         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;
>diff --git a/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp b/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp
>index 5a7ff8631206d85d82a2c04080eae06b775cf2e2..1a1e0ec65cd916656e23dc13c8fbe618e13dddee 100644
>--- a/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp
>+++ b/Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp
>@@ -676,7 +676,7 @@ void CDMInstanceSessionClearKey::loadSession(LicenseType, const String& sessionI
> void CDMInstanceSessionClearKey::closeSession(const String&, CloseSessionCallback&& callback)
> {
>     callOnMainThread(
>-        [weakThis = makeWeakPtr(*this), callback = WTFMove(callback)] {
>+        [weakThis = makeWeakPtr(*this), callback = WTFMove(callback)] () mutable {
>             if (!weakThis)
>                 return;
> 
>diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm
>index c6ec20a7756f9fb075814e445292a1853da6b21e..b3a7a9aae718943e5ab6e4abb2f826d38def2a6d 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm
>+++ b/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm
>@@ -428,15 +428,15 @@ void CDMInstanceSessionFairPlayStreamingAVFObjC::closeSession(const String&, Clo
> {
>     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 @@ void CDMInstanceSessionFairPlayStreamingAVFObjC::didProvideRequest(AVContentKeyR
>     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 @@ void CDMInstanceSessionFairPlayStreamingAVFObjC::didProvideRequest(AVContentKeyR
>                 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 @@ void CDMInstanceSessionFairPlayStreamingAVFObjC::didProvideRenewingRequest(AVCon
>                 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 @@ void CDMInstanceSessionFairPlayStreamingAVFObjC::didFailToProvideRequest(AVConte
> {
>     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::didFailToProvideRequest(AVConte
> 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;
>

Actions: View | Formatted Diff | Diff

Attachments on bug 192713: 357336