Opened 11 years ago
Closed 11 years ago
#10510 closed enhancement (fixed)
Support Web Sockets (easy)
Reported by: | pulkomandy | Owned by: | pulkomandy |
---|---|---|---|
Priority: | normal | Milestone: | R1/beta1 |
Component: | Applications/WebPositive | Version: | R1/Development |
Keywords: | gsoc2014 | Cc: | |
Blocked By: | Blocking: | ||
Platform: | All |
Description
Test case: http://websocketstest.com
This is reasonably easy to implement. Most of the protocol handling is already done on WebKit side, so all we need is an implementation of Source/WebCore/platform/network/haiku/SocketStreamHandleHaiku.cpp.
Have a look at Source/WebCore/platform/network/soup/SocketStreamHandleSoup.cpp for a reference implementation.
Basically, this is just a socket I/O class with a non-blocking connect/read/write. It should not be a problem to implement this over BSocket/BSecureSocket.
Attachments (5)
Change History (20)
comment:1 by , 11 years ago
Keywords: | easy removed |
---|---|
Summary: | Support Web Sockets → Support Web Sockets (easy) |
follow-up: 3 comment:2 by , 11 years ago
Milestone: | Unscheduled → R1/beta1 |
---|
comment:3 by , 11 years ago
Replying to kallisti5:
placing at beta1 as it is a good gsoc task.
I've been trying to work on this issue. I am using virtual box to test Haiku. But I don't see the WebPositive application running. Is it somehow disabled in the source code? If so, how can I enable it so that I can test?
comment:4 by , 11 years ago
Currently, WebPositive is only available in x86 nightlies (not x86_64). You need the gcc2hybrid one, or gcc4 or gcc4hybrid. Get those from http://www.haiku-files.org.
The sourcecode you need to modify is at http://github.com/haiku/webkit. I recommend cloning with git clone --depth=1 (the repo is big), or downloading the archive for version 1.2.4 from the "releases" tab (in that case, keep a copy of the original sources, so you can generate a patch from your modified ones).
@kallisti5: I don't the link between GSoC and beta1? The task is marked "easy" so GSoC students can find it, already.
by , 11 years ago
comment:5 by , 11 years ago
patch: | 0 → 1 |
---|
by , 11 years ago
comment:6 by , 11 years ago
I've added two patches. Both need to be applied. I am not at all confident about these, as I wasn't able to test them at all ( and I am just beginning with haiku ), due to build failures on gcc4hybrid. If anyone can test them and tell me, it would be great :)
comment:7 by , 11 years ago
Keywords: | gsoc2014 added |
---|
Quick review:
- You can remove the notImplemented() calls and "# not implemented" comment in the CMakeFile.
- Make sure you follow the coding guidelines, in this case you have at least missing space around "=" and "==" operators (we write "stuff() == B_OK", not "stuff()==B_OK": https://www.haiku-os.org/development/coding-guidelines
- I'll see if the patches compile, later today.
To make this easier to track, it would be nice if you used git to extract the patches. You can use git format-patch: http://git-scm.com/docs/git-format-patch .
(GSoC note: there seem to be a problem with the CMake in our gcc4 repo, leading to strange build failures. Working with student to fix this so he can test his patch next time).
by , 11 years ago
Attachment: | 0001-WebSocketsImplemented.patch added |
---|
Web-Sockets Implementation (20/03/14)
by , 11 years ago
Attachment: | 0002-Commiting-changes.patch added |
---|
Web-Sockets Implementation (Modifications) (20/03/14)
comment:8 by , 11 years ago
Web sockets implementation.
Passed all tests. (https://www.dropbox.com/s/o0q1tgzy71pxuik/Screenshot%202014-03-20%2020.11.02.png) Please review.
comment:9 by , 11 years ago
- Could you merge the two patches into one? (use git rebase -i)
- Please change the copyright in SocketStreamHandleHaiku. You rewrote all of it, so assign copyright to yourself or to Haiku, Inc.
- You added a blank line after void receivedCancellation(const AuthenticationChallenge&); , before the end of the class declaration. This is not needed.
- #define READ_BUFFER_SIZE 1024 : a "static const int kReadBufferSize" would be better. Maybe a BNetBuffer could be used instead (or a BMallocIO), for dynamic sizing?
- The change to Source/JavaScriptCore/runtime/JSLock.cpp should not be in the patch. Please remove it.
- Instead of using suspendThread, you should notify the thread using a variable (used in the thread while loop, for example), to make the thread exit the loop and properly terminate
- There are debug calls (to std::cout) left. Please remove those.
comment:10 by , 11 years ago
- MIT License and copyrights updated accordingly.
- Made #define READ_BUFFER_SIZE 1024 -> static const int kReadBufferSize (probably not much use of dynamic sizing)
- Used messages to terminate thread, instead of killing.
- Couldn't find std::cout calls. They were handled in the second patch. -> Rebased.
comment:11 by , 11 years ago
Instead of having three different threads to handle asynchronous connect, read and write, you could have one thread that selects/polls the socket for read/writability and calls the appropriate callbacks.
You might also want to look at using C++11 threads instead of Be threads, as the rest of the Webkit codebase uses these.
comment:12 by , 11 years ago
There is this reason I had for keeping separate read and write threads. Both of them are now being operated in blocking-mode (It waits indefinitely, till the socket is available for writability or readability), and thus are put to sleep by the OS. If both are merged into a single thread, then for independent operation of read and write (i.e., we can write, even if nothing is available to be read and vice-versa), we'll have to shift to non-blocking operation. Continuously polling a socket, hence, will give rise to high CPU usage, by this thread. I agree separate connect and read threads is surely an overkill. I'll merge them into one, and use C++11 threads, and submit the patch in a day or two. :)
comment:13 by , 11 years ago
This wouldn't need a busy-loop style polling. You can use the poll() or select() functions to have the thread sleep, and be woken up when the socket becomes either readable, writable, or closed.
If this isn't conveniently done with the current BSocket API, it could be extended to allow for it.
comment:14 by , 11 years ago
Hi, I have tested the latest version of the patch. Unfortunately there still is a problem.
http://www.websocket.org/echo.html
Use the "connect" button and then try sending some messages. You can see that it works.
Now press "back" to leave the page (or enter another URL in the address bar). The socket is destroyed when leaving the page, but your read thread crashes. It should instead notice that the socket is gone and cleanly shutdown.
comment:15 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I have fixed the remaining issues myself in https://github.com/haiku/webkit/commit/48041f5a839c32916b27a66d6cac5a28d5a14c8f .
placing at beta1 as it is a good gsoc task.