RESOLVED FIXED 116178
Remove WebSocketHandshakeRequest class 
https://bugs.webkit.org/show_bug.cgi?id=116178
Summary Remove WebSocketHandshakeRequest class
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
DetailsFormatted DiffDiff
Patch (29.47 KB, patch)
2013-05-15 13:06 PDT, Anders Carlsson 		kling: review+
DetailsFormatted DiffDiff
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.