Bug 33842 - [Qt] media tests crash
Summary: [Qt] media tests crash
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P3 Normal
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks: 33453
  Show dependency treegraph
 
Reported: 2010-01-19 07:49 PST by Jakub Wieczorek
Modified: 2010-01-21 20:05 PST (History)
5 users (show)

See Also:


Attachments
backtrace (11.04 KB, text/plain)
2010-01-19 07:50 PST, Jakub Wieczorek
no flags Details
backtrace (4.97 KB, text/plain)
2010-01-20 06:20 PST, Jakub Wieczorek
no flags Details
patch (1.84 KB, patch)
2010-01-20 10:28 PST, Jakub Wieczorek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Wieczorek 2010-01-19 07:49:29 PST
When running /media tests, every second or third one would crash. This does not happen when they are run separately. 
It can be also reproduced by running the same test more than once (--repeat n).
Comment 1 Jakub Wieczorek 2010-01-19 07:50:44 PST
Created attachment 46908 [details]
backtrace

Here is a mysterious backtrace I managed to get. Not sure what is happening here.
Comment 2 Jakub Wieczorek 2010-01-20 06:20:56 PST
Created attachment 47016 [details]
backtrace

This one seems to make more sense.
Comment 3 Jakub Wieczorek 2010-01-20 10:28:36 PST
Created attachment 47047 [details]
patch

OK, these two stack traces are definitely worth investigating but it seems that they are more or less unrelated.

What is strange in these traces is that the application gets destroyed. It turns out that in fact these tests are not crashing (except these two random cases). They are passing but the DRT quits after they end. The run-webkit-tests gets confused and clasifies that as a crash.

The question is why DRT quits: it looks related to http://trac.webkit.org/changeset/38223.

The video widget has no parent and therefore is considered to be a top window. Thus, its lifetime has influence on the application's lifetime (see http://qt.nokia.com/doc/4.6/qapplication.html#lastWindowClosed). However, I still don't get why the deletion of the video widget triggers the application quit. There is still the main QWebView, which should keep it running.

I wonder why the above change was limited to Qt < 4.5 only? In case it is safe to make that change version-independent, I'm attaching a patch.
Comment 4 Jakub Wieczorek 2010-01-20 10:33:28 PST
Tor Arne: do you remember why that change has been guarded with the version #if?
Comment 5 Csaba Osztrogonác 2010-01-20 13:09:25 PST
I tested your patch together with last patch in https://bugs.webkit.org/show_bug.cgi?id=33453 and it works. It's great work! ;)

$ WebKitTools/Scripts/run-webkit-tests media --skipped=only
...
56 test cases (53%) succeeded
32 test cases (30%) had incorrect layout
17 test cases (16%) were new
27 test cases (25%) had stderr output

But we should wait for review of Tor Arne not to cause strange errors with removing guard.
Comment 6 Tor Arne Vestbø 2010-01-21 04:00:36 PST
From http://qt.nokia.com/doc/4.6/qapplication.html#lastWindowClosed:

"This signal is emitted from QApplication::exec() when the last visible primary window (i.e. window with no parent) with the Qt::WA_QuitOnClose attribute set is closed."

The video widget has the Qt::WA_DontShowOnScreen flag set. Pre Qt 4.5 this flag was not considered when deciding "is this window visible" (it has no parent, so it's a primary window), so the video widget would keep the QtLauncher running even after the main window was closed. 

The fix was to also set Qt::WA_QuitOnClose, so that closing the main application would be considered the "last visible primary window with the Qt::WA_QuitOnClose attribute set".

What's happening here seems to be that we receive the lastWindowClosed signal since we're running the DRT headless, and the video widget is not considered visible due to the flag, so we end up quitting the DRT since the signal is implicitly connected to QApplication::quit().

I think the solution might be to set quitOnLastWindowClosed to false, could you try that without removing the ifdef and see if it works?  

http://qt.nokia.com/doc/4.6/qapplication.html#quitOnLastWindowClosed-prop
Comment 7 Jakub Wieczorek 2010-01-21 05:29:39 PST
(In reply to comment #6)
> From http://qt.nokia.com/doc/4.6/qapplication.html#lastWindowClosed:
> 
> "This signal is emitted from QApplication::exec() when the last visible primary
> window (i.e. window with no parent) with the Qt::WA_QuitOnClose attribute set
> is closed."
> 
> The video widget has the Qt::WA_DontShowOnScreen flag set. Pre Qt 4.5 this flag
> was not considered when deciding "is this window visible" (it has no parent, so
> it's a primary window), so the video widget would keep the QtLauncher running
> even after the main window was closed. 
> 
> The fix was to also set Qt::WA_QuitOnClose, so that closing the main
> application would be considered the "last visible primary window with the
> Qt::WA_QuitOnClose attribute set".
> 
> What's happening here seems to be that we receive the lastWindowClosed signal
> since we're running the DRT headless, and the video widget is not considered
> visible due to the flag, so we end up quitting the DRT since the signal is
> implicitly connected to QApplication::quit().
> 
> I think the solution might be to set quitOnLastWindowClosed to false, could you
> try that without removing the ifdef and see if it works?  
> 
> http://qt.nokia.com/doc/4.6/qapplication.html#quitOnLastWindowClosed-prop

I tried that before but it looks more like a work around to me. It would solve the problem for DRT - OK, but it seems that this sort of behavior would be present in any application that runs headless? Consider the following example:

class WebView : public QWebView
{
    Q_OBJECT

    public:
        WebView() { }

    public slots:
        void loadMedia()
        {
            load(QUrl("site-with-audio-or-video"));
        }
            
        void loadGoogle()
        {
            load(QUrl("http://google.com"));
        }
};

QApplication app(argc, argv);
  
WebView view;
QTimer::singleShot(1000, &view, SLOT(loadMedia()));
QObject::connect(&view, SIGNAL(loadFinished(bool)), &view, SLOT(loadGoogle()));

return app.exec();

This example quits right after it starts loading google.com.

It is discussable whether applications that do not show any widgets on the screen should set quitOnLastWindowClosed to false. The problem we're talking about is kinda expectable for normal Qt applications though.

But here, a developer would have no clue about what's going on because the widget that triggers this sort of thing is hidden under the hood and therefore the behavior is hard to diagnose. A developer shouldn't even care about the underlying video widget. And as it is embedded withing a web page, it should be safe to exclude it from deciding when the application quits.
Comment 8 Tor Arne Vestbø 2010-01-21 06:16:54 PST
(In reply to comment #7)
> I tried that before but it looks more like a work around to me. It would solve
> the problem for DRT - OK, but it seems that this sort of behavior would be
> present in any application that runs headless?

You're right, there seems to be a bug in Qt's quitOnClose-handling code. I'm looking into it now.
Comment 9 WebKit Commit Bot 2010-01-21 20:04:56 PST
Comment on attachment 47047 [details]
patch

Clearing flags on attachment: 47047

Committed r53670: <http://trac.webkit.org/changeset/53670>
Comment 10 WebKit Commit Bot 2010-01-21 20:05:02 PST
All reviewed patches have been landed.  Closing bug.