In bug #173738, we substituted RunLoop:Main() for WTF::isMainThread() only in file Source/WebKit2/UIProcess/GenericCallback.h with the explanation "We have verified locally that RunLoop::isMain() propertly (sic) return true in [in affected iOS apps] (while WTF::isMainThread() returns false)." It would be good to understand why WTF::isMainThread() can return the wrong result in iOS WebKit2 and fix it. If it is not feasible to fix it and we know that RunLoop:Main() always returns the correct answer (to the question whether the currently running thread is the main thread) then we should substitute RunLoop:Main() for all uses of WTF::isMainThread().
<rdar://problem/32936527>
The apps that were broken used both WK2 and WK1 I believe. I have not looked in details yet but my guess is that WTF::isMainThread() gets confused by the fact that there is a WebThread in the UIProcess?
Similar change made by Tim in: https://bugs.webkit.org/show_bug.cgi?id=134900
And by Enrica in: https://bugs.webkit.org/show_bug.cgi?id=129135
Created attachment 313683 [details] Patch
Can we rename isMainThread to avoid this confusion?
(In reply to Simon Fraser (smfr) from comment #6) > Can we rename isMainThread to avoid this confusion? Maybe. Although I'd rather do this in a separate patch. Any naming proposal? IsMainThreadOrWebThread? However, For non-Cocoa ports, isMainThread() does the right thing and is accurately named. The issue is only with iOS WebKit1 and the WebThread.
Created attachment 313684 [details] Patch
We recently had similar issues with using WebCore::Timer in the UIProcess. As a result, we'd want to add an assertion in the WebCore::Timer to make sure it is not called from the UIProcess. The issue is that we'd need a way to detect this accurately (and not break WebKit1 code running in the UIProcess). Ideally, we'd add a similar assertion in WTF::isMainThread() (or make WTF::isMainThread() work reliably).
(In reply to Chris Dumez from comment #9) > We recently had similar issues with using WebCore::Timer in the UIProcess. > As a result, we'd want to add an assertion in the WebCore::Timer to make > sure it is not called from the UIProcess. The issue is that we'd need a way > to detect this accurately (and not break WebKit1 code running in the > UIProcess). > > Ideally, we'd add a similar assertion in WTF::isMainThread() (or make > WTF::isMainThread() work reliably). Note that this patch helps but does not protect again people using WTF::isMainThread() is the UIProcess again. It also doesn't take care of code that is in WebCore and potentially called from the UIProcess.
Created attachment 313686 [details] Patch
Comment on attachment 313686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=313686&action=review Looks good. I was about to make this mistake in another patch! r=me. > Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:592 > + bool calledOnMainThread = WTF::RunLoop::isMain(); Do we need WTF:: here? We don't use it elsewhere.
Created attachment 313744 [details] Patch
Comment on attachment 313744 [details] Patch Clearing flags on attachment: 313744 Committed r218763: <http://trac.webkit.org/changeset/218763>
All reviewed patches have been landed. Closing bug.