554008 - QApplication init must be initialized in ContentProcessChild, not in TabChild

Status: RESOLVED FIXED

Description

[Oleg Romashin (:romaxa)]

Created attachment 433909 [details] [diff] [review]
Qapplication -> ContentProcessChild

TabChilds can be created and destroyed in the same process, and we should not create and destroy QApplication every time when it happens.

Comment 1

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Why not create QApplication already in ContentProcessChild ctor?
And actually, since ContentProcessChild is a singleton, it could have QApplication
as a member variable. That would look a bit nicer.

Comment 2

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

I wonder if the member variable could be even
nsAutoPtr<QApplication> mQApp;
Then ContentProcessChild dtor should automatically delete the object.

Comment 3

[Oleg Romashin (:romaxa)]

Created attachment 433916 [details] [diff] [review]
Updated version with autoptr

Comment 4

[Oleg Romashin (:romaxa)]

Created attachment 433917 [details] [diff] [review]
QApp init moved to ctor

Comment 5

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Comment on attachment 433917 [details] [diff] [review]
QApp init moved to ctor

>diff -r 12cce72d1dd7 dom/ipc/ContentProcessChild.cpp
>--- a/dom/ipc/ContentProcessChild.cpp	Mon Mar 22 15:36:54 2010 +0200
>+++ b/dom/ipc/ContentProcessChild.cpp	Mon Mar 22 16:00:30 2010 +0200
> ContentProcessChild::ContentProcessChild()
>     : mQuit(PR_FALSE)
> {
>+#ifdef MOZ_WIDGET_QT
>+    NS_ASSERTION(!qApp, "QApplication created too early?");
>+    mQApp = new QApplication(gArgc, (char**)gArgv);
Not sure why you need (char**)

Comment 6

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Ah, perhaps QApplication has overloaded ctors.

Comment 7

[Oleg Romashin (:romaxa)]

Pushed
http://hg.mozilla.org/projects/electrolysis/rev/c4e431fb18d0

Comment 8

[Oleg Romashin (:romaxa)]

I think we should redo this patch, because ContentProcessChild ctor created not in main thread, and as result we have problems like:
***********************
QPixmap: It is not safe to use pixmaps outside the GUI thread                                           
QObject: Cannot create children for a parent that is in a different thread.                             
(Parent is QApplication(0x14220), parent's thread is QThread(0x1b3a0), current thread is QThread(0x499f2
8)                                                                                                      
QObject::startTimer: QTimer can only be used with threads started with QThread                          
QObject::setParent: Cannot set parent, new parent is in a different thread                              
QObject::startTimer: QTimer can only be used with threads started with QThread                          
[TabChild] MOVE to (x,y)=(0d, 0d), (w,h)= (0d, 0d)                                                      
###################################### frame-content loaded                                             
loading about:blank, 1                                                                                  
QObject::startTimer: QTimer can only be used with threads started with QThread                          
QObject::startTimer: QTimer can only be used with threads started with QThread     
***********************

Comment 9

[Oleg Romashin (:romaxa)]

Created attachment 434480 [details] [diff] [review]
QApp init must be in ::Init function, that is called from main thread

Comment 10

[Oleg Romashin (:romaxa)]

Additional fix pushed in:
http://hg.mozilla.org/projects/electrolysis/rev/c732df81ad85

Comment 11

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Uh, I didn't know the ctor is run in different thread than init.