Attachment 362093 Details for Bug 194690 – Patch

[patch] Patch

bug-194690-20190214201026.patch (text/plain), 4.50 KB, created by youenn fablet on 2019-02-14 20:10:27 PST

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-02-14 20:10:27 PST

Size: 4.50 KB

patch

obsolete

>Subversion Revision: 241548
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b797776e2a515e5ff06935965ba4de4e91d15a8f..aaabae10357d025ceab7862fe1dcd2e6f4b4b2ed 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-02-14  Youenn Fablet  <youenn@apple.com>
>+
>+        WebSocket should not fire events after being stopped
>+        https://bugs.webkit.org/show_bug.cgi?id=194690
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-14  Youenn Fablet  <youenn@apple.com>
> 
>         Performance should not fire events when its context is stopped
>diff --git a/Source/WebCore/Modules/websockets/WebSocket.cpp b/Source/WebCore/Modules/websockets/WebSocket.cpp
>index 5e9ea526f5da3c74a9229bcff1c88f92c3f9667b..1453979ea4f6cf83d844c07b83578c9b64629203 100644
>--- a/Source/WebCore/Modules/websockets/WebSocket.cpp
>+++ b/Source/WebCore/Modules/websockets/WebSocket.cpp
>@@ -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)]() {
>+
>+            document.postTask([this, protectedThis = makeRef(*this)](auto&) {
>                 dispatchOrQueueErrorEvent();
>                 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()
>diff --git a/Source/WebCore/Modules/websockets/WebSocket.h b/Source/WebCore/Modules/websockets/WebSocket.h
>index ca6790c304f959f12d33a230f36f90d12b2cba75..b669660e75dee6aed0494fd68035bfe47509dcbb 100644
>--- a/Source/WebCore/Modules/websockets/WebSocket.h
>+++ b/Source/WebCore/Modules/websockets/WebSocket.h
>@@ -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

Actions: View | Formatted Diff | Diff

Attachments on bug 194690: 362093 | 362094