Bug 39702 - JavaScriptCore threading relies on QApplication existing
Summary: JavaScriptCore threading relies on QApplication existing
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-05-25 17:47 PDT by Anders Bakken
Modified: 2014-02-03 03:50 PST (History)
2 users (show)

See Also:


Attachments
Patch that fixes the problem. (2.08 KB, patch)
2010-05-25 18:05 PDT, Anders Bakken
no flags Details | Formatted Diff | Diff
Patch that fixes the problem. (2.08 KB, patch)
2010-05-25 18:50 PDT, Anders Bakken
no flags Details | Formatted Diff | Diff
Patch (2.02 KB, patch)
2010-06-14 09:35 PDT, Anders Bakken
hausmann: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Bakken 2010-05-25 17:47:18 PDT
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.
Comment 1 Anders Bakken 2010-05-25 18:05:37 PDT
Created attachment 57058 [details]
Patch that fixes the problem.
Comment 2 WebKit Review Bot 2010-05-25 18:08:53 PDT
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.
Comment 3 Anders Bakken 2010-05-25 18:50:01 PDT
Created attachment 57061 [details]
Patch that fixes the problem.
Comment 4 Eric Seidel (no email) 2010-06-04 23:06:48 PDT
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());
Comment 5 Eric Seidel (no email) 2010-06-12 20:48:35 PDT
Comment on attachment 57061 [details]
Patch that fixes the problem.

r- based on my above comment.  Please reset r? after answering.
Comment 6 Anders Bakken 2010-06-14 09:35:12 PDT
Created attachment 58663 [details]
Patch
Comment 7 Anders Bakken 2010-06-14 09:36:50 PDT
@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.
Comment 8 Simon Hausmann 2010-07-08 23:46:28 PDT
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 :)
Comment 9 Jocelyn Turcotte 2014-02-03 03:50:45 PST
=== 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.