Opened 6 years ago

Closed 5 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:
Has a Patch: yes 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)

patch (3.6 KB ) - added by akshay1994 5 years ago.
patch2 (1.1 KB ) - added by akshay1994 5 years ago.
0001-WebSocketsImplemented.patch (6.3 KB ) - added by akshay1994 5 years ago.
Web-Sockets Implementation (20/03/14)
0002-Commiting-changes.patch (11.7 KB ) - added by akshay1994 5 years ago.
Web-Sockets Implementation (Modifications) (20/03/14)
0001-WebSocketsImplemented.2.patch (9.8 KB ) - added by akshay1994 5 years ago.
WebSocketsImplemented

Download all attachments as: .zip

Change History (20)

comment:1 by pulkomandy, 6 years ago

Keywords: easy removed
Summary: Support Web SocketsSupport Web Sockets (easy)

comment:2 by kallisti5, 6 years ago

Milestone: UnscheduledR1/beta1

placing at beta1 as it is a good gsoc task.

in reply to:  2 comment:3 by azizulhakim, 5 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 pulkomandy, 5 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 akshay1994, 5 years ago

Attachment: patch added

comment:5 by akshay1994, 5 years ago

Has a Patch: set

by akshay1994, 5 years ago

Attachment: patch2 added

comment:6 by akshay1994, 5 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 pulkomandy, 5 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 akshay1994, 5 years ago

Web-Sockets Implementation (20/03/14)

by akshay1994, 5 years ago

Web-Sockets Implementation (Modifications) (20/03/14)

comment:8 by akshay1994, 5 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 pulkomandy, 5 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.

by akshay1994, 5 years ago

WebSocketsImplemented

comment:10 by akshay1994, 5 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 hamish, 5 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 akshay1994, 5 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 pulkomandy, 5 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 pulkomandy, 5 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 pulkomandy, 5 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.