Bug 116178

Summary:

Remove WebSocketHandshakeRequest class

Product:

WebKit

Reporter:

Anders Carlsson <andersca>

Component:

New Bugs

Assignee:

Anders Carlsson <andersca>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, eflews.bot, gyuyoung.kim, kling, sam, webkit-ews

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	kling: review+

Anders Carlsson

Reported 2013-05-15 12:44:17 PDT

Remove WebSocketHandshakeRequest class

Attachments
Patch (30.25 KB, patch) 2013-05-15 12:45 PDT, Anders Carlsson	no flags	Details Formatted Diff Diff
Patch (29.47 KB, patch) 2013-05-15 13:06 PDT, Anders Carlsson	kling: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Anders Carlsson

Comment 1 2013-05-15 12:45:50 PDT

Created attachment 201867 [details] Patch

Early Warning System Bot

Comment 2 2013-05-15 12:58:26 PDT

Comment on attachment 201867 [details] Patch Attachment 201867 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/476631

EFL EWS Bot

Comment 3 2013-05-15 13:02:42 PDT

Comment on attachment 201867 [details] Patch Attachment 201867 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/473621

Anders Carlsson

Comment 4 2013-05-15 13:06:38 PDT

Created attachment 201871 [details] Patch

Andreas Kling

Comment 5 2013-05-15 13:20:24 PDT

Comment on attachment 201871 [details] Patch I'ma go ahead and trust you on this one, Carlsson.

Anders Carlsson

Comment 6 2013-05-15 13:21:53 PDT

Committed r150142: <http://trac.webkit.org/changeset/150142>

Alexey Proskuryakov

Comment 7 2013-05-15 19:59:55 PDT

> Turns out WebSocketHandshakeRequest is just used by the web inspector, and there's no reason why we can't just use a ResourceRequest instead. This is not accurate. WebSocket has different semantics than HTTP, in particular, order of header fields matters. I'm puzzled that you are making such changes without any attempt of talking to me. Andreas, I suggest waiting at least 24 hours before reviewing patches in areas you are not sure that you are an expert in.

Andreas Kling

Comment 8 2013-05-16 09:58:30 PDT

(In reply to comment #7) > > Turns out WebSocketHandshakeRequest is just used by the web inspector, and there's no reason > why we can't just use a ResourceRequest instead. > > This is not accurate. WebSocket has different semantics than HTTP, in particular, order of header fields matters. > > I'm puzzled that you are making such changes without any attempt of talking to me. Andreas, I suggest waiting at least 24 hours before reviewing patches in areas you are not sure that you are an expert in. Alexey, I trusted Anders's judgment on this change and it looked like a straightforward removal of an unnecessary abstraction. I set my r+ without fully understanding the implications, and for that I apologize.

Alexey Proskuryakov

Comment 9 2013-05-17 12:06:08 PDT

> order of header fields matters This is no longer the case, the spec no longer requires that. I could not quickly find any other differences that would require using a distinct class.

Note You need to log in before you can comment on or make changes to this bug.