Bug 39702 – JavaScriptCore threading relies on QApplication existing

• Status: RESOLVED
• Resolution: INVALID
• Component: JavaScriptCore

[Anders Bakken]

This causes

Since initializeMainThread is guaranteed to be called from the main thread it's better to just make sure the MainThreadInvoker object is created in that thread and use the MainThreadInvoker's thread affinity to determine whether the current thread is the main thread.

[Anders Bakken]

Created attachment 57058 [details]
Patch that fixes the problem.

[WebKit Review Bot]

Attachment 57058 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Anders Bakken]

Created attachment 57061 [details]
Patch that fixes the problem.

[Eric Seidel (no email)]

Comment on attachment 57061 [details]
Patch that fixes the problem.

Why is it OK to still have this ASSERt use QCoreAplication now that the header include is removed?
     Q_ASSERT(!QCoreApplication::instance()
 51              || thread() == QCoreApplication::instance()->thread());

[Eric Seidel (no email)]

Comment on attachment 57061 [details]
Patch that fixes the problem.

r- based on my above comment.  Please reset r? after answering.

[Anders Bakken]

Created attachment 58663 [details]
Patch

[Anders Bakken]

@Eric: I am not sure why it compiled for me. Maybe it's implicitly included from somewhere (or maybe I only compiled in release). 

Either way. New patch without removing the include is added.

[Simon Hausmann]

Comment on attachment 58663 [details]
Patch

The patch/bug doesn't explain what the actual issue is that this fixes, beyond "it's better to make sure..."

The ChangeLog is also slightly malformed, the "Reviewed by" should be at the top.

Anders, I'm okay with the patch in principle, but it would really help to understand your motivation behind the change. Imagine a year down the line trying to figure out why this was changed ...

r- because of the missing explanation and malformed ChangeLog. The patch itself is okay, once we know what it really fixes :)

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.