Modernize WebSocket code
Created attachment 280835 [details] Patch
Comment on attachment 280835 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280835&action=review > Source/WebCore/ChangeLog:8 > + No new tests (OOPS!). I'll put "no change in behavior" here
Attachment 280835 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 280840 [details] Patch
Comment on attachment 280840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280840&action=review > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:175 > - RefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper = m_workerClientWrapper; > + RefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper = m_workerClientWrapper.copyRef(); > m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper = WTFMove(workerClientWrapper), sendRequestResult] (ScriptExecutionContext&) { This can be written as 1 line. m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper = Ref<ThreadableWebSocketChannelClientWrapper>(*m_workerClientWrapper), sendRequestResult] (ScriptExecutionContext&) mutable { All the other ones like it, too!
(In reply to comment #5) > Comment on attachment 280840 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=280840&action=review > > > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:175 > > - RefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper = m_workerClientWrapper; > > + RefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper = m_workerClientWrapper.copyRef(); > > m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper = WTFMove(workerClientWrapper), sendRequestResult] (ScriptExecutionContext&) { > > This can be written as 1 line. > > m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper = > Ref<ThreadableWebSocketChannelClientWrapper>(*m_workerClientWrapper), > sendRequestResult] (ScriptExecutionContext&) mutable { > > All the other ones like it, too! Also, there should be no space between the capture list and the open paren: m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper = > Ref<ThreadableWebSocketChannelClientWrapper>(*m_workerClientWrapper), > sendRequestResult](ScriptExecutionContext&) mutable {
Created attachment 280845 [details] Patch
Comment on attachment 280845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280845&action=review > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:355 > + auto peer = std::make_unique<Peer>(clientWrapper.copyRef(), loaderProxy, context, taskMode); > + bool sent = loaderProxy.postTaskForModeToWorkerGlobalScope({ > ScriptExecutionContext::Task::CleanupTask, > - [clientWrapper, loaderProxy, peer = WTFMove(peer)] (ScriptExecutionContext& context) mutable { > + [clientWrapper = clientWrapper.copyRef(), &loaderProxy, peer = WTFMove(peer)] (ScriptExecutionContext& context) mutable { You could move the peer creation into the capture list.
http://trac.webkit.org/changeset/201837