Bug 33842 – [Qt] media tests crash

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jakub Wieczorek]

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).

[Jakub Wieczorek]

Created attachment 46908 [details]
backtrace

Here is a mysterious backtrace I managed to get. Not sure what is happening here.

[Jakub Wieczorek]

Created attachment 47016 [details]
backtrace

This one seems to make more sense.

[Jakub Wieczorek]

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.

[Jakub Wieczorek]

Tor Arne: do you remember why that change has been guarded with the version #if?

[Csaba Osztrogonác]

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.

[Tor Arne Vestbø]

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

[Jakub Wieczorek]

(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.

[Tor Arne Vestbø]

(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.

[WebKit Commit Bot]

Comment on attachment 47047 [details]
patch

Clearing flags on attachment: 47047

Committed r53670: <http://trac.webkit.org/changeset/53670>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.