[webkit-reviews] review canceled: [Bug 61353] WebSocket: Use fail() when WebSocketChannel has failed : [Attachment 94590] Patch

[bugzilla-daemon at webkit.org]

Yuta Kitamura <yutak at chromium.org> has canceled Yuta Kitamura
<yutak at chromium.org>'s request for review:
Bug 61353: WebSocket: Use fail() when WebSocketChannel has failed
https://bugs.webkit.org/show_bug.cgi?id=61353

Attachment 94590: Patch
https://bugs.webkit.org/attachment.cgi?id=94590&action=review

------- Additional Comments from Yuta Kitamura <yutak at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=94590&action=review

>> Source/WebCore/ChangeLog:8
>> +	    No change in behavior, thus no new tests.
> 
> Is it true?
> To me, the patch looks to change console messages in some cases.

You are right. However, the messages changed (or added) in this patch are not
covered in the existing LayoutTests, because they depend on fastMalloc's
failure and, as far as I know, it's pretty hard to make it fail reliably.

So I think I need to rephrase the above text to mention these facts.

>> Source/WebCore/ChangeLog:12
>> +	    We should not close the socket stream if m_closed is true.
> 
> This is a behavior change, isn't it?

Actually, no, becuase SocketStreamHandle::close() does nothing and returns
immediately if that socket stream is already closed. So, this change is just
adding a shortcut, and does not change the behavior.

I will update the ChangeLog on this, too.