[Webkit-unassigned] [Bug 72155] Replace Qt QThread threading back-end with pthread/Win32 threading back-ends

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=72155


Simon Hausmann <hausmann at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #116639|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #18 from Simon Hausmann <hausmann at webkit.org>  2011-11-25 23:35:35 PST ---
(From update of attachment 116639)
View in context: https://bugs.webkit.org/attachment.cgi?id=116639&action=review

r- because I believe the removal of the moveToThread() code will cause layout test failures (like it did on Thursday).

> Source/JavaScriptCore/wtf/qt/MainThreadQt.cpp:73
> +    QCoreApplication::postEvent(webkit_main_thread_invoker(), new QEvent(static_cast<QEvent::Type>(s_mainThreadInvokerEventType)));

There's a problem with the removal of the moveToThread code. There is currently no guarantee that initializeMainThreadPlatform is called before scheduleDispatchFunctionsOnMainThread() is called the first time from a thread. As a result it may happen that the webkit_main_thread_invoker object
will live on one of the threads other than the gui thread and as a result the functionality of scheduling the dispatch of functions on the main thread
won't work. We had exactly this problem on Thursday morning and if you run the layout tests on Linux with your patch applied, you'll see that all the appcache
tests will break and time out.

There are two solutions AFAICS:

    1) Keep the moveToThread code that I added. That's least intrusive and probably most robust, but it's so "clean".

    2) Ensure initializeMainThreadPlatform() is called before any thread other than the gui thread is started.

The modification you're going to need for (2) is around the static functions in QWebSettings, in particular the icon database related ones. If you
call QWebSettings::setIconDatabasePath() before any other function in QtWebKit (like QWebSettings::globalSettings() or creating a QWebPage), then you
have the situation that the WebCore IconDatabase will spawn its thread before initializeMainThreadPlatform() is called, and that thread will call
scheduleDispatchFunctionsOnMainThread(), which in turn will cause the creation of the invoker object in the wrong thread with the wrong thread affinity.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.