WebSocket should not fire events after being stopped
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Feb 2019 19:08:06 +0000 (19:08 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Feb 2019 19:08:06 +0000 (19:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194690

Reviewed by Geoffrey Garen.

dispatchOrQueueErrorEvent is scheduled using RunLoop::main().dispatch or dispatch_async.
This makes it possible to dispatch an event while WebSocket is already stopped.
Instead, use Document::postTask so that the task is only executed if WebSocket is not stopped.

As a refactoring, make use of PendingActivity to keep track of setPendingActivity/unsetPendingActivity more easily.

* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::stop):
(WebCore::WebSocket::connect):
* Modules/websockets/WebSocket.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241599 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/websockets/WebSocket.cpp
Source/WebCore/Modules/websockets/WebSocket.h

index 6add399..0400914 100644 (file)
@@ -1,5 +1,23 @@
 2019-02-15  Youenn Fablet  <youenn@apple.com>
 
+        WebSocket should not fire events after being stopped
+        https://bugs.webkit.org/show_bug.cgi?id=194690
+
+        Reviewed by Geoffrey Garen.
+
+        dispatchOrQueueErrorEvent is scheduled using RunLoop::main().dispatch or dispatch_async.
+        This makes it possible to dispatch an event while WebSocket is already stopped.
+        Instead, use Document::postTask so that the task is only executed if WebSocket is not stopped.
+
+        As a refactoring, make use of PendingActivity to keep track of setPendingActivity/unsetPendingActivity more easily.
+
+        * Modules/websockets/WebSocket.cpp:
+        (WebCore::WebSocket::stop):
+        (WebCore::WebSocket::connect):
+        * Modules/websockets/WebSocket.h:
+
+2019-02-15  Youenn Fablet  <youenn@apple.com>
+
         Performance should not fire events when its context is stopped
         https://bugs.webkit.org/show_bug.cgi?id=194689
 
index 5e9ea52..351be8b 100644 (file)
@@ -288,28 +288,18 @@ ExceptionOr<void> WebSocket::connect(const String& url, const Vector<String>& pr
         Document& document = downcast<Document>(context);
         RefPtr<Frame> frame = document.frame();
         if (!frame || !frame->loader().mixedContentChecker().canRunInsecureContent(document.securityOrigin(), m_url)) {
-            // Balanced by the call to unsetPendingActivity() in WebSocket::stop().
-            setPendingActivity(*this);
+            m_pendingActivity = makePendingActivity(*this);
 
             // We must block this connection. Instead of throwing an exception, we indicate this
             // using the error event. But since this code executes as part of the WebSocket's
             // constructor, we have to wait until the constructor has completed before firing the
             // event; otherwise, users can't connect to the event.
-#if USE(WEB_THREAD)
-            ref();
-            dispatch_async(dispatch_get_main_queue(), ^{
-                WebThreadRun(^{
-                    dispatchOrQueueErrorEvent();
-                    stop();
-                    deref();
-                });
-            });
-#else
-            RunLoop::main().dispatch([this, protectedThis = makeRef(*this)]() {
-                dispatchOrQueueErrorEvent();
-                stop();
+
+            document.postTask([this, protectedThis = makeRef(*this)](auto&) {
+                this->dispatchOrQueueErrorEvent();
+                this->stop();
             });
-#endif
+
             return { };
         }
     }
@@ -319,7 +309,7 @@ ExceptionOr<void> WebSocket::connect(const String& url, const Vector<String>& pr
         protocolString = joinStrings(protocols, subprotocolSeparator());
 
     m_channel->connect(m_url, protocolString);
-    setPendingActivity(*this);
+    m_pendingActivity = makePendingActivity(*this);
 
     return { };
 }
@@ -540,15 +530,13 @@ void WebSocket::resumeTimerFired()
 
 void WebSocket::stop()
 {
-    bool pending = hasPendingActivity();
     if (m_channel)
         m_channel->disconnect();
     m_channel = nullptr;
     m_state = CLOSED;
     m_pendingEvents.clear();
     ActiveDOMObject::stop();
-    if (pending)
-        unsetPendingActivity(*this);
+    m_pendingActivity = nullptr;
 }
 
 const char* WebSocket::activeDOMObjectName() const
@@ -631,8 +619,7 @@ void WebSocket::didClose(unsigned unhandledBufferedAmount, ClosingHandshakeCompl
         m_channel->disconnect();
         m_channel = nullptr;
     }
-    if (hasPendingActivity())
-        unsetPendingActivity(*this);
+    m_pendingActivity = nullptr;
 }
 
 void WebSocket::didUpgradeURL()
index ca6790c..b669660 100644 (file)
@@ -143,6 +143,7 @@ private:
     bool m_shouldDelayEventFiring { false };
     Deque<Ref<Event>> m_pendingEvents;
     bool m_dispatchedErrorEvent { false };
+    RefPtr<PendingActivity<WebSocket>> m_pendingActivity;
 };
 
 } // namespace WebCore