Bug 167832

Summary: Fix misleading comment in RunLoop.h
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web Template FrameworkAssignee: Joseph Pecoraro <joepeck>
Status: REOPENED    
Severity: Normal CC: andersca, ap, benjamin, cdumez, cmarcelo, commit-queue, dbates, ddkilzer, joepeck, linnalandau, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 168152    
Bug Blocks:    
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[PATCH] Proposed Fix none

Joseph Pecoraro
Reported 2017-02-03 21:36:25 PST
Fix misleading comment in RunLoop.h. Mac initialization used to force using CFRunLoopGetMain(). Now however it just uses RunLoop::current which uses CFRunLoopGetCurrent(). So this comment that it can be done on any thread is misleading and can lead to incorrect behavior if it is actually done on a non-main thread on Mac.
Attachments
[PATCH] Proposed Fix (1.43 KB, patch)
2017-02-03 21:37 PST, Joseph Pecoraro 		no flags
DetailsFormatted DiffDiff
[PATCH] Proposed Fix (1.81 KB, patch)
2017-02-04 15:55 PST, Joseph Pecoraro 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Joseph Pecoraro
Comment 1 2017-02-03 21:37:33 PST
Created attachment 300604 [details] [PATCH] Proposed Fix
Alexey Proskuryakov
Comment 2 2017-02-04 00:07:57 PST
Comment on attachment 300604 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=300604&action=review > Source/WTF/wtf/RunLoop.h:53 > + // Must be called from the main thread. On the Mac, this can be checked with an assertion (using pthread_main_np).
Joseph Pecoraro
Comment 3 2017-02-04 15:55:50 PST
Created attachment 300643 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 4 2017-02-04 17:04:49 PST
Comment on attachment 300643 [details] [PATCH] Proposed Fix Oh, that makes sense. WebKit1 will need some kind of "wait a beat".
Joseph Pecoraro
Comment 5 2017-02-04 17:05:38 PST
Comment on attachment 300643 [details] [PATCH] Proposed Fix That comment was meant for another bug.
WebKit Commit Bot
Comment 6 2017-02-10 13:10:06 PST
Comment on attachment 300643 [details] [PATCH] Proposed Fix Clearing flags on attachment: 300643 Committed r212139: <http://trac.webkit.org/changeset/212139>
WebKit Commit Bot
Comment 7 2017-02-10 13:10:12 PST
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 8 2017-02-10 16:17:10 PST
Re-opened since this is blocked by bug 168152
Joseph Pecoraro
Comment 9 2017-02-10 16:23:02 PST
This revealed a few issues where non-main thread code triggers WebKit2Initialize and so could potentially be initializing the main run loop on a non-main thread (because WebKit2Initialize is not dispatch_once and always calls down into RunLoop::initializeMainThread). For example, this case in DumpRenderTree: (Bug 168149) https://bugs.webkit.org/show_bug.cgi?id=168149
Note You need to log in before you can comment on or make changes to this bug.