[webkit-changes] [253422] trunk/Source/WebCore

Thu, 12 Dec 2019 00:49:16 -0800

Title: [253422] trunk/Source/WebCore
Revision
253422
Author
[email protected]
Date
2019-12-12 00:48:07 -0800 (Thu, 12 Dec 2019)

Log Message

Remove debug ASSERT in PeerConnectionBackend
https://bugs.webkit.org/show_bug.cgi?id=205061

Reviewed by Eric Carlson.

Remove two wrong debug asserts.
Move the isClosed check just before resolving/rejecting the promise since at resume time, a lot might happen,
including closing the peer connection.
Covered by tests no longer crashing.

* Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::createOfferSucceeded):
(WebCore::PeerConnectionBackend::createOfferFailed):
(WebCore::PeerConnectionBackend::createAnswerSucceeded):
(WebCore::PeerConnectionBackend::createAnswerFailed):
(WebCore::PeerConnectionBackend::setLocalDescriptionSucceeded):
(WebCore::PeerConnectionBackend::setLocalDescriptionFailed):
(WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded):
(WebCore::PeerConnectionBackend::setRemoteDescriptionFailed):
(WebCore::PeerConnectionBackend::addIceCandidateSucceeded):
(WebCore::PeerConnectionBackend::addIceCandidateFailed):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (253421 => 253422)


--- trunk/Source/WebCore/ChangeLog	2019-12-12 08:24:24 UTC (rev 253421)
+++ trunk/Source/WebCore/ChangeLog	2019-12-12 08:48:07 UTC (rev 253422)
@@ -1,3 +1,27 @@
+2019-12-12  youenn fablet  <[email protected]>
+
+        Remove debug ASSERT in PeerConnectionBackend
+        https://bugs.webkit.org/show_bug.cgi?id=205061
+
+        Reviewed by Eric Carlson.
+
+        Remove two wrong debug asserts.
+        Move the isClosed check just before resolving/rejecting the promise since at resume time, a lot might happen,
+        including closing the peer connection.
+        Covered by tests no longer crashing.
+
+        * Modules/mediastream/PeerConnectionBackend.cpp:
+        (WebCore::PeerConnectionBackend::createOfferSucceeded):
+        (WebCore::PeerConnectionBackend::createOfferFailed):
+        (WebCore::PeerConnectionBackend::createAnswerSucceeded):
+        (WebCore::PeerConnectionBackend::createAnswerFailed):
+        (WebCore::PeerConnectionBackend::setLocalDescriptionSucceeded):
+        (WebCore::PeerConnectionBackend::setLocalDescriptionFailed):
+        (WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded):
+        (WebCore::PeerConnectionBackend::setRemoteDescriptionFailed):
+        (WebCore::PeerConnectionBackend::addIceCandidateSucceeded):
+        (WebCore::PeerConnectionBackend::addIceCandidateFailed):
+
 2019-12-11  Wenson Hsieh  <[email protected]>
 
         Add a missing entry in EnumTraits<WebCore::DisplayList::ItemType>

Modified: trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp (253421 => 253422)


--- trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp	2019-12-12 08:24:24 UTC (rev 253421)
+++ trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp	2019-12-12 08:48:07 UTC (rev 253422)
@@ -99,11 +99,11 @@
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER, "Create offer succeeded:\n", sdp);
 
-    if (m_peerConnection.isClosed())
-        return;
+    ASSERT(m_offerAnswerPromise);
+    m_peerConnection.doTask([this, promise = WTFMove(m_offerAnswerPromise), sdp = filterSDP(WTFMove(sdp))]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-    ASSERT(m_offerAnswerPromise);
-    m_peerConnection.doTask([promise = WTFMove(m_offerAnswerPromise), sdp = filterSDP(WTFMove(sdp))]() mutable {
         promise->resolve(RTCSessionDescription::Init { RTCSdpType::Offer, sdp });
     });
 }
@@ -113,11 +113,11 @@
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER, "Create offer failed:", exception.message());
 
-    if (m_peerConnection.isClosed())
-        return;
+    ASSERT(m_offerAnswerPromise);
+    m_peerConnection.doTask([this, promise = WTFMove(m_offerAnswerPromise), exception = WTFMove(exception)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-    ASSERT(m_offerAnswerPromise);
-    m_peerConnection.doTask([promise = WTFMove(m_offerAnswerPromise), exception = WTFMove(exception)]() mutable {
         promise->reject(WTFMove(exception));
     });
 }
@@ -136,11 +136,11 @@
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER, "Create answer succeeded:\n", sdp);
 
-    if (m_peerConnection.isClosed())
-        return;
+    ASSERT(m_offerAnswerPromise);
+    m_peerConnection.doTask([this, promise = WTFMove(m_offerAnswerPromise), sdp = WTFMove(sdp)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-    ASSERT(m_offerAnswerPromise);
-    m_peerConnection.doTask([promise = WTFMove(m_offerAnswerPromise), sdp = WTFMove(sdp)]() mutable {
         promise->resolve(RTCSessionDescription::Init { RTCSdpType::Answer, sdp });
     });
 }
@@ -150,11 +150,11 @@
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER, "Create answer failed:", exception.message());
 
-    if (m_peerConnection.isClosed())
-        return;
+    ASSERT(m_offerAnswerPromise);
+    m_peerConnection.doTask([this, promise = WTFMove(m_offerAnswerPromise), exception = WTFMove(exception)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-    ASSERT(m_offerAnswerPromise);
-    m_peerConnection.doTask([promise = WTFMove(m_offerAnswerPromise), exception = WTFMove(exception)]() mutable {
         promise->reject(WTFMove(exception));
     });
 }
@@ -196,12 +196,11 @@
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER);
 
-    if (m_peerConnection.isClosed())
-        return;
+    ASSERT(m_setDescriptionPromise);
+    m_peerConnection.doTask([this, promise = WTFMove(m_setDescriptionPromise)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-
-    ASSERT(m_setDescriptionPromise);
-    m_peerConnection.doTask([promise = WTFMove(m_setDescriptionPromise)]() mutable {
         promise->resolve();
     });
 }
@@ -211,11 +210,11 @@
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER, "Set local description failed:", exception.message());
 
-    if (m_peerConnection.isClosed())
-        return;
+    ASSERT(m_setDescriptionPromise);
+    m_peerConnection.doTask([this, promise = WTFMove(m_setDescriptionPromise), exception = WTFMove(exception)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-    ASSERT(m_setDescriptionPromise);
-    m_peerConnection.doTask([promise = WTFMove(m_setDescriptionPromise), exception = WTFMove(exception)]() mutable {
         promise->reject(WTFMove(exception));
     });
 }
@@ -256,9 +255,9 @@
 {
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER, "Set remote description succeeded");
+    ASSERT(m_setDescriptionPromise);
 
-    ASSERT(!m_peerConnection.isClosed());
-
+    auto promise = WTFMove(m_setDescriptionPromise);
     auto events = WTFMove(m_pendingTrackEvents);
     for (auto& event : events) {
         auto& track = event.track.get();
@@ -272,12 +271,10 @@
         track.source().setMuted(false);
     }
 
-    if (m_peerConnection.isClosed())
-        return;
+    m_peerConnection.doTask([this, promise = WTFMove(promise)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-
-    ASSERT(m_setDescriptionPromise);
-    m_peerConnection.doTask([promise = WTFMove(m_setDescriptionPromise)]() mutable {
         promise->resolve();
     });
 }
@@ -290,9 +287,11 @@
     ASSERT(m_pendingTrackEvents.isEmpty());
     m_pendingTrackEvents.clear();
 
-    ASSERT(!m_peerConnection.isClosed());
     ASSERT(m_setDescriptionPromise);
-    m_peerConnection.doTask([promise = WTFMove(m_setDescriptionPromise), exception = WTFMove(exception)]() mutable {
+    m_peerConnection.doTask([this, promise = WTFMove(m_setDescriptionPromise), exception = WTFMove(exception)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
+
         promise->reject(WTFMove(exception));
     });
 }
@@ -337,11 +336,11 @@
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER, "Adding ice candidate succeeded");
 
-    if (m_peerConnection.isClosed())
-        return;
+    ASSERT(m_addIceCandidatePromise);
+    m_peerConnection.doTask([this, promise = WTFMove(m_addIceCandidatePromise)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-    ASSERT(m_addIceCandidatePromise);
-    m_peerConnection.doTask([promise = WTFMove(m_addIceCandidatePromise)]() mutable {
         promise->resolve();
     });
 }
@@ -351,11 +350,11 @@
     ASSERT(isMainThread());
     ALWAYS_LOG(LOGIDENTIFIER, "Adding ice candidate failed:", exception.message());
 
-    if (m_peerConnection.isClosed())
-        return;
+    ASSERT(m_addIceCandidatePromise);
+    m_peerConnection.doTask([this, promise = WTFMove(m_addIceCandidatePromise), exception = WTFMove(exception)]() mutable {
+        if (m_peerConnection.isClosed())
+            return;
 
-    ASSERT(m_addIceCandidatePromise);
-    m_peerConnection.doTask([promise = WTFMove(m_addIceCandidatePromise), exception = WTFMove(exception)]() mutable {
         promise->reject(WTFMove(exception));
     });
 }

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to