|Product:||WebKit||Reporter:||Maxime Simon <simon.maxime>|
|Version:||528+ (Nightly build)|
Description Maxime Simon 2009-08-09 04:41:31 PDT
Comment 1 Maxime Simon 2009-08-09 04:50:36 PDT
Comment 2 Eric Seidel (no email) 2009-08-09 07:39:28 PDT
Comment 3 Eric Seidel (no email) 2009-08-09 07:40:09 PDT
(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.)
Comment 4 Ryan Leavengood 2009-08-09 08:32:58 PDT
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.
Comment 5 Eric Seidel (no email) 2009-08-09 09:00:42 PDT
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.
Comment 6 Maxime Simon 2009-08-09 09:07:40 PDT
Comment 7 Eric Seidel (no email) 2009-08-09 09:43:21 PDT
Comment 8 Eric Seidel (no email) 2009-08-11 12:51:20 PDT
Comment 9 Eric Seidel (no email) 2009-08-11 12:51:24 PDT
All reviewed patches have been landed. Closing bug.