Log Message
Use WeakPtr instead of storing raw pointers in WebSocket code https://bugs.webkit.org/show_bug.cgi?id=196034
Reviewed by Geoff Garen. This could prevent using freed memory if we forget to reset a pointer somewhere. * Modules/websockets/WebSocketChannel.cpp: (WebCore::WebSocketChannel::WebSocketChannel): (WebCore::WebSocketChannel::connect): (WebCore::WebSocketChannel::fail): (WebCore::WebSocketChannel::disconnect): (WebCore::WebSocketChannel::didOpenSocketStream): (WebCore::WebSocketChannel::didCloseSocketStream): (WebCore::WebSocketChannel::didFailSocketStream): (WebCore::WebSocketChannel::processBuffer): (WebCore::WebSocketChannel::processFrame): (WebCore::WebSocketChannel::processOutgoingFrameQueue): (WebCore::WebSocketChannel::sendFrame): * Modules/websockets/WebSocketChannel.h: * Modules/websockets/WebSocketChannelClient.h: * Modules/websockets/WebSocketHandshake.cpp: (WebCore::WebSocketHandshake::WebSocketHandshake): * Modules/websockets/WebSocketHandshake.h:
Modified Paths
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp
- trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h
- trunk/Source/WebCore/Modules/websockets/WebSocketChannelClient.h
- trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp
- trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.h
Diff
Modified: trunk/Source/WebCore/ChangeLog (243251 => 243252)
--- trunk/Source/WebCore/ChangeLog 2019-03-20 23:03:14 UTC (rev 243251)
+++ trunk/Source/WebCore/ChangeLog 2019-03-20 23:15:04 UTC (rev 243252)
@@ -1,3 +1,30 @@
+2019-03-20 Alex Christensen <[email protected]>
+
+ Use WeakPtr instead of storing raw pointers in WebSocket code
+ https://bugs.webkit.org/show_bug.cgi?id=196034
+
+ Reviewed by Geoff Garen.
+
+ This could prevent using freed memory if we forget to reset a pointer somewhere.
+
+ * Modules/websockets/WebSocketChannel.cpp:
+ (WebCore::WebSocketChannel::WebSocketChannel):
+ (WebCore::WebSocketChannel::connect):
+ (WebCore::WebSocketChannel::fail):
+ (WebCore::WebSocketChannel::disconnect):
+ (WebCore::WebSocketChannel::didOpenSocketStream):
+ (WebCore::WebSocketChannel::didCloseSocketStream):
+ (WebCore::WebSocketChannel::didFailSocketStream):
+ (WebCore::WebSocketChannel::processBuffer):
+ (WebCore::WebSocketChannel::processFrame):
+ (WebCore::WebSocketChannel::processOutgoingFrameQueue):
+ (WebCore::WebSocketChannel::sendFrame):
+ * Modules/websockets/WebSocketChannel.h:
+ * Modules/websockets/WebSocketChannelClient.h:
+ * Modules/websockets/WebSocketHandshake.cpp:
+ (WebCore::WebSocketHandshake::WebSocketHandshake):
+ * Modules/websockets/WebSocketHandshake.h:
+
2019-03-20 Dean Jackson <[email protected]>
[iOS] Crash in WebCore::Node::renderRect
Modified: trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp (243251 => 243252)
--- trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp 2019-03-20 23:03:14 UTC (rev 243251)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp 2019-03-20 23:15:04 UTC (rev 243252)
@@ -63,8 +63,8 @@
const Seconds TCPMaximumSegmentLifetime { 2_min };
WebSocketChannel::WebSocketChannel(Document& document, WebSocketChannelClient& client, SocketProvider& provider)
- : m_document(&document)
- , m_client(&client)
+ : m_document(makeWeakPtr(document))
+ , m_client(makeWeakPtr(client))
, m_resumeTimer(*this, &WebSocketChannel::resumeTimerFired)
, m_closingTimer(*this, &WebSocketChannel::closingTimerFired)
, m_socketProvider(provider)
@@ -112,12 +112,12 @@
ASSERT(!m_handle);
ASSERT(!m_suspended);
- m_handshake = std::make_unique<WebSocketHandshake>(url, protocol, m_document, allowCookies);
+ m_handshake = std::make_unique<WebSocketHandshake>(url, protocol, m_document.get(), allowCookies);
m_handshake->reset();
if (m_deflateFramer.canDeflate())
m_handshake->addExtensionProcessor(m_deflateFramer.createExtensionProcessor());
if (m_identifier)
- InspectorInstrumentation::didCreateWebSocket(m_document, m_identifier, url);
+ InspectorInstrumentation::didCreateWebSocket(m_document.get(), m_identifier, url);
if (Frame* frame = m_document->frame()) {
ref();
@@ -214,7 +214,7 @@
LOG(Network, "WebSocketChannel %p fail() reason='%s'", this, reason.utf8().data());
ASSERT(!m_suspended);
if (m_document) {
- InspectorInstrumentation::didReceiveWebSocketFrameError(m_document, m_identifier, reason);
+ InspectorInstrumentation::didReceiveWebSocketFrameError(m_document.get(), m_identifier, reason);
String consoleMessage;
if (m_handshake)
@@ -245,7 +245,7 @@
{
LOG(Network, "WebSocketChannel %p disconnect()", this);
if (m_identifier && m_document)
- InspectorInstrumentation::didCloseWebSocket(m_document, m_identifier);
+ InspectorInstrumentation::didCloseWebSocket(m_document.get(), m_identifier);
if (m_handshake)
m_handshake->clearDocument();
m_client = nullptr;
@@ -273,7 +273,7 @@
if (!m_document)
return;
if (m_identifier && UNLIKELY(InspectorInstrumentation::hasFrontends()))
- InspectorInstrumentation::willSendWebSocketHandshakeRequest(m_document, m_identifier, m_handshake->clientHandshakeRequest());
+ InspectorInstrumentation::willSendWebSocketHandshakeRequest(m_document.get(), m_identifier, m_handshake->clientHandshakeRequest());
auto handshakeMessage = m_handshake->clientHandshakeMessage();
auto cookieRequestHeaderFieldProxy = m_handshake->clientHandshakeCookieRequestHeaderFieldProxy();
handle.sendHandshake(WTFMove(handshakeMessage), WTFMove(cookieRequestHeaderFieldProxy), [this, protectedThis = makeRef(*this)] (bool success, bool didAccessSecureCookies) {
@@ -289,7 +289,7 @@
{
LOG(Network, "WebSocketChannel %p didCloseSocketStream()", this);
if (m_identifier && m_document)
- InspectorInstrumentation::didCloseWebSocket(m_document, m_identifier);
+ InspectorInstrumentation::didCloseWebSocket(m_document.get(), m_identifier);
ASSERT_UNUSED(handle, &handle == m_handle || !m_handle);
m_closed = true;
if (m_closingTimer.isActive())
@@ -300,7 +300,7 @@
m_unhandledBufferedAmount = m_handle->bufferedAmount();
if (m_suspended)
return;
- WebSocketChannelClient* client = m_client;
+ WebSocketChannelClient* client = m_client.get();
m_client = nullptr;
m_document = nullptr;
m_handle = nullptr;
@@ -363,7 +363,7 @@
message = makeString("WebSocket network error: error code ", error.errorCode());
else
message = "WebSocket network error: " + error.localizedDescription();
- InspectorInstrumentation::didReceiveWebSocketFrameError(m_document, m_identifier, message);
+ InspectorInstrumentation::didReceiveWebSocketFrameError(m_document.get(), m_identifier, message);
m_document->addConsoleMessage(MessageSource::Network, MessageLevel::Error, message);
}
m_shouldDiscardReceivedData = true;
@@ -448,7 +448,7 @@
return false;
if (m_handshake->mode() == WebSocketHandshake::Connected) {
if (m_identifier)
- InspectorInstrumentation::didReceiveWebSocketHandshakeResponse(m_document, m_identifier, m_handshake->serverHandshakeResponse());
+ InspectorInstrumentation::didReceiveWebSocketHandshakeResponse(m_document.get(), m_identifier, m_handshake->serverHandshakeResponse());
String serverSetCookie = m_handshake->serverSetCookie();
if (!serverSetCookie.isEmpty()) {
if (m_document && m_document->page() && m_document->page()->cookieJar().cookiesEnabled(*m_document))
@@ -582,7 +582,7 @@
return false;
}
- InspectorInstrumentation::didReceiveWebSocketFrame(m_document, m_identifier, frame);
+ InspectorInstrumentation::didReceiveWebSocketFrame(m_document.get(), m_identifier, frame);
switch (frame.opCode) {
case WebSocketFrame::OpCodeContinuation:
@@ -768,7 +768,7 @@
ASSERT(frame->blobData);
m_blobLoader = std::make_unique<FileReaderLoader>(FileReaderLoader::ReadAsArrayBuffer, this);
m_blobLoaderStatus = BlobLoaderStarted;
- m_blobLoader->start(m_document, *frame->blobData);
+ m_blobLoader->start(m_document.get(), *frame->blobData);
m_outgoingFrameQueue.prepend(WTFMove(frame));
return;
@@ -820,7 +820,7 @@
ASSERT(!m_suspended);
WebSocketFrame frame(opCode, true, false, true, data, dataLength);
- InspectorInstrumentation::didSendWebSocketFrame(m_document, m_identifier, frame);
+ InspectorInstrumentation::didSendWebSocketFrame(m_document.get(), m_identifier, frame);
auto deflateResult = m_deflateFramer.deflate(frame);
if (!deflateResult->succeeded()) {
Modified: trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h (243251 => 243252)
--- trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h 2019-03-20 23:03:14 UTC (rev 243251)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h 2019-03-20 23:15:04 UTC (rev 243252)
@@ -193,8 +193,8 @@
BlobLoaderFailed
};
- Document* m_document;
- WebSocketChannelClient* m_client;
+ WeakPtr<Document> m_document;
+ WeakPtr<WebSocketChannelClient> m_client;
std::unique_ptr<WebSocketHandshake> m_handshake;
RefPtr<SocketStreamHandle> m_handle;
Vector<char> m_buffer;
Modified: trunk/Source/WebCore/Modules/websockets/WebSocketChannelClient.h (243251 => 243252)
--- trunk/Source/WebCore/Modules/websockets/WebSocketChannelClient.h 2019-03-20 23:03:14 UTC (rev 243251)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketChannelClient.h 2019-03-20 23:15:04 UTC (rev 243252)
@@ -31,10 +31,11 @@
#pragma once
#include <wtf/Forward.h>
+#include <wtf/WeakPtr.h>
namespace WebCore {
-class WebSocketChannelClient {
+class WebSocketChannelClient : public CanMakeWeakPtr<WebSocketChannelClient> {
public:
virtual ~WebSocketChannelClient() = default;
virtual void didConnect() = 0;
Modified: trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp (243251 => 243252)
--- trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp 2019-03-20 23:03:14 UTC (rev 243251)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp 2019-03-20 23:15:04 UTC (rev 243252)
@@ -123,7 +123,7 @@
: m_url(url)
, m_clientProtocol(protocol)
, m_secure(m_url.protocolIs("wss"))
- , m_document(document)
+ , m_document(makeWeakPtr(document))
, m_mode(Incomplete)
, m_allowCookies(allowCookies)
{
Modified: trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.h (243251 => 243252)
--- trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.h 2019-03-20 23:03:14 UTC (rev 243251)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.h 2019-03-20 23:15:04 UTC (rev 243252)
@@ -35,6 +35,7 @@
#include "ResourceResponse.h"
#include "WebSocketExtensionDispatcher.h"
#include "WebSocketExtensionProcessor.h"
+#include <wtf/WeakPtr.h>
#include <wtf/text/WTFString.h>
namespace WebCore {
@@ -100,7 +101,7 @@
URL m_url;
String m_clientProtocol;
bool m_secure;
- Document* m_document;
+ WeakPtr<Document> m_document;
Mode m_mode;
bool m_allowCookies;
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
