Bug 28121 – [Haiku] Modifications on JavaScriptCore.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Maxime Simon]

Some modifications on JavaScriptCore files to allow the the port.
It also adds a missing header on wtf/haiku/MainThreadHaiku.cpp

Regards,
Maxime

[Maxime Simon]

Created attachment 34406 [details]
Modifications on JavaScriptCore to allow Haiku port.

[Eric Seidel (no email)]

Comment on attachment 34406 [details]
Modifications on JavaScriptCore to allow Haiku port.

Bah!  We need to abstract the Collector to use some sort of Threading* abstraction!

This is the 3rd patch I've reviewed this week to add #ifdefs there. :(

+#include "platform/NotImplemented.h"

should be "NotImplemented.h" no need for "platform/"

r- for the wrong include path.  If you'd be kind enough to fix the threading abstraction in some way (e.g. to at least use a static inline function for the Haiku case that would be nice.)

[Eric Seidel (no email)]

(Btw, I'm not mad about the collector thing.  I'm just annoyed that no one has abstracted it yet, and that we've had so may contributions recently to that #ifdef madness file.)

[Ryan Leavengood]

Eric,

We are definitely interested in improving the general WebKit code as well as having our port code done "better". In this case some more details on what exactly you are looking for would help.

It seems like even if we added a ThreadingHaiku.cpp and a new static inline method to get the stack base there would still need to be an #elif PLATFORM(HAIKU) in currentThreadStackBase in Collector.cpp.

I suppose the best solution is to move all the associated thread stack base code for all platforms in their respective Threading* classes and then just call that function from Collector.cpp. I suppose that would be your "ideal" solution, right?

Of course there are no files ThreadingSolaris, ThreadingSymbian, ThreadingBSD, ThreadingWinCE, or ThreadingHaiku yet. It seems like the stack base needs to be abstracted out at another level maybe. Or ThreadingPthreads can just have #ifdefs for the various pthread platforms to get the stack base in all those ways. Maybe WinCE could be stuck in with ThreadingWin, since ThreadingWin would need at least 3 #ifdefs as it is for the stack base.

It is definitely a tricky problem.

[Eric Seidel (no email)]

I think asking you to fix this #ifdef battleground is unecessary.  Your #ifdefs are OK.  the r- was mostly for the include name being wrong.

Maybe in the end we'll move all this #ifdeffed code out into a .cpp file which can be included by Collector.cpp.  Then at least Collector.cpp could have some semblance of x-platform-ness, and all the #ifdef crud can be elsewhere.

[Maxime Simon]

Created attachment 34424 [details]
Modifications on JavaScriptCore to allow Haiku port.

This patch doesn't change anything else but the bad include.

[Eric Seidel (no email)]

Comment on attachment 34424 [details]
Modifications on JavaScriptCore to allow Haiku port.

LGTM.

[Eric Seidel (no email)]

Comment on attachment 34424 [details]
Modifications on JavaScriptCore to allow Haiku port.

Clearing review flag on attachment: 34424

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/runtime/Collector.cpp
	M	JavaScriptCore/wtf/Platform.h
	M	JavaScriptCore/wtf/haiku/MainThreadHaiku.cpp
Committed r47050
	M	JavaScriptCore/runtime/Collector.cpp
	M	JavaScriptCore/wtf/haiku/MainThreadHaiku.cpp
	M	JavaScriptCore/wtf/Platform.h
	M	JavaScriptCore/ChangeLog
r47050 = b11f14b641a8c5357b1ee13ba14846834ae2311e (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/47050

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.