We should disentangle the initialization of WebCores notion of the main thread from the rest of the initialization done in JSC::initializeThreading and WTF::initializeThreading.
Created attachment 54246 [details] Patch
Comment on attachment 54246 [details] Patch Forgot something. Obsoleting.
Created attachment 54247 [details] Patch
Attachment 54247 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/win/WebKitClassFactory.cpp:61: Alphabetical sorting problem. [build/include_order] [4] WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp" WebKit/efl/ewk/ewk_main.cpp:29: wtf includes should be <wtf/file.h> instead of "wtf/file.h". [build/include] [4] Total errors found: 2 in 52 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 54247 [details] Patch Some minor comments below. Overall this seems reasonable other than the "deprecated" naming. ALso I think it would be good to log or assert or throw an exception or something in the cases where deprecatedInitializeMainThread is called from a thread other than the main thread (since it implies misuse of the original WebKit API). r- to consider these issues, please upload a fresh version. JavaScriptCore/ChangeLog:15 + (WTF::deprecatedInitializeMainThread): The difference between initializeMainThread and deprecatedInitializeMainThread is not clear from the names. Also from offline discussion it seems like the "deprecated" version is not going away soon or possibly ever, so that doesn't seem like a very good name. JavaScriptCore/wtf/MainThread.cpp:101 + MutexLocker locker(mainThreadInitializationMutex()); Why a mutex here instead of a "once" type mechanism like pthread_once? JavaScriptCore/wtf/mac/MainThreadMac.mm:75 + void deprecatedInitializeMainThreadPlatform() it looks to me like mainThreadPthread will never be initialized if WebKit is called off the main thread the first time, even on platforms where it would otherwise be initialized to pthread_self(). Is that ok?
(In reply to comment #5) > (From update of attachment 54247 [details]) > Some minor comments below. Overall this seems reasonable other than the > "deprecated" naming. ALso I think it would be good to log or assert or throw an > exception or something in the cases where deprecatedInitializeMainThread is > called from a thread other than the main thread (since it implies misuse of the > original WebKit API). r- to consider these issues, please upload a fresh > version. > > JavaScriptCore/ChangeLog:15 > + (WTF::deprecatedInitializeMainThread): > The difference between initializeMainThread and deprecatedInitializeMainThread > is not clear from the names. Also from offline discussion it seems like the > "deprecated" version is not going away soon or possibly ever, so that doesn't > seem like a very good name. Which name seems better, initializeMainThreadToSystemMainThread() or legacyInitializeMainThread()? > > JavaScriptCore/wtf/MainThread.cpp:101 > + MutexLocker locker(mainThreadInitializationMutex()); > Why a mutex here instead of a "once" type mechanism like pthread_once? Will fix. > JavaScriptCore/wtf/mac/MainThreadMac.mm:75 > + void deprecatedInitializeMainThreadPlatform() > it looks to me like mainThreadPthread will never be initialized if WebKit is > called off the main thread the first time, even on platforms where it would > otherwise be initialized to pthread_self(). Is that ok? That is fine. If deprecatedInitializeMainThreadPlatform() is ever called, it is being used from an environment which expects WebCore's main thread to be the system's main thread.
I didn't note this anywhere, and I feel silly for not having done so, but this is the first step in getting rid of the WEB_THREAD #define and paving the way to allowing mac both WebKit and WebKit2 to use the same WebCore. I will put this information in the next versions changelog.
Created attachment 54338 [details] Patch 3
Attachment 54338 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebKit/win/WebKitClassFactory.cpp:61: Alphabetical sorting problem. [build/include_order] [4] WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp" WebKit/efl/ewk/ewk_main.cpp:29: wtf includes should be <wtf/file.h> instead of "wtf/file.h". [build/include] [4] Total errors found: 2 in 47 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 54338 [details] Patch 3 r=me
Landed in r58266.
Fix tiger build in r58268.
http://trac.webkit.org/changeset/58266 might have broken Chromium Win Release