Bug 35247 – [Qt] QtTestBrowser crashes when enabling QGraphicsView mode after first loading page without it enabled

• Component: New Bugs
• Status: RESOLVED FIXED

[Jesus Sanchez-Palencia]

Created attachment 49225 [details]
Backtrace

QtLauncher crashes on Mac OS on QGraphicsView mode:

- if you try to enable QGraphicsView mode after loading a website, QtLauncher will crash. Please test with several pages, since it does not occur always (not on http://www.google.com, for instance). I did several tests with www.uol.com.br and it always crashed.

Backtrace is attached.

[Tor Arne Vestbø]

Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should only be used for bugs/features in the
    public QtWebKit API layer, not to signify that the bug is specific to
    the Qt port of WebKit

      http://trac.webkit.org/wiki/QtWebKitBugs#Component

  - Add the keyword 'Qt' to signal that it's a Qt-related bug

      http://trac.webkit.org/wiki/QtWebKitBugs#Keywords

[Kent Hansen]

Reproduced with r55986, Qt 4.7, same backtrace.

[Simon Hausmann]

Jesus, does this problem still exist? Can this bug be closed?

Could you attach the backtrace as a plain text file?

[Jesus Sanchez-Palencia]

(In reply to comment #3)
> Jesus, does this problem still exist? Can this bug be closed?

Right now I'm having problems with plugins here so I'm not sure if this still exist. Last week, during my last tests on Leopard it was there. Maybe someone else with Snow Leopard (and Leopard) and QtWebKit working full with plugins support can test this?

> 
> Could you attach the backtrace as a plain text file?
Sure, and I'm pasting it below:


The following backtraces were generated after opening QtLauncher, entering a website e then activating the QGraphicsView mode.

0   QtGui                         	0x03314cd9 QWidget::window() const + 9
1   QtWebKit                      	0x02525e9e WebCore::PluginView::updatePluginWidget() + 318
2   QtWebKit                      	0x02379b3e WebCore::ScrollView::frameRectsChanged() + 78
3   QtWebKit                      	0x0237b19e WebCore::ScrollView::setFrameRect(WebCore::IntRect const&) + 110
4   QtWebKit                      	0x025030de QWebPage::setViewportSize(QSize const&) const + 142
5   QtWebKit                      	0x024faed4 QGraphicsWebView::setPage(QWebPage*) + 308
6   QtLauncher                    	0x0000de4b WebViewGraphicsBased::setPage(QWebPage*) + 29 (webview.h:74)

(...)

This one was generated by torarne:

#0  0x05391ff5 in QPixmapData::classId (this=0x0) at qpixmapdata_p.h:110
#1  0x054f8127 in qt_mac_cg_context (pdev=0x7155c04) at /Users/torarne/dev/qt/4.6/src/gui/painting/qpaintdevice_mac.cpp:125
#2  0x00d925ab in WebCore::PluginView::updatePluginWidget (this=0x7155a00) at /Users/torarne/dev/webkit/land/WebCore/plugins/mac/PluginViewMac.cpp:466

[Kent Hansen]

(In reply to comment #4)
> (In reply to comment #3)
> > Jesus, does this problem still exist? Can this bug be closed?
> 
> Right now I'm having problems with plugins here so I'm not sure if this still
> exist. Last week, during my last tests on Leopard it was there. Maybe someone
> else with Snow Leopard (and Leopard) and QtWebKit working full with plugins
> support can test this?
> 

Yes, I can still reproduce it on Leopard.

[Kent Hansen]

printf to the rescue:

PluginView::setPlatformPluginWidget(0xd95a70)
~QWidget(0xd95a70)
~QWidget(0xd95b90)
~QWidget(0x1a2bd510)
0x4b46000 PluginViewMac::updatePluginWidget()
platformPluginWidget: 0xd95a70 <-- OOPS!

So topLevelOffsetFor() in PluginViewMac.cpp is calling window() on a junk pointer.

Now just to figure out who deleted it.

[Tor Arne Vestbø]

Looking at this

[Tor Arne Vestbø]

The crash is caused by re-using the same page for the QGraphicsWebView as the one for QWidget.

The problem is that we currently have no tear-down logic in QWebPage when the view changes to invalidate platform widgets and other things (WebGL) based on the change to the view, so we use a destroyed view when trying to draw the NPAPI plugin.

Long term we want to fix this since this is a valid use-case, but it's not release critical. Removing from 2.0 blocker bug.

I'll land a workaround for the QtLauncher for now that tears down the QWebPage as well, just so that we don't crash.

[Tor Arne Vestbø]

(In reply to comment #8)
> I'll land a workaround for the QtLauncher for now that tears down the QWebPage
> as well, just so that we don't crash.

Turned out to be more work than anticipated due to toolbars etc, deferring for now.

[Kenneth Rohde Christiansen]

Jeez, how does this work with your new PageClient patch? Any improvement?

[Tor Arne Vestbø]

*** Bug 44688 has been marked as a duplicate of this bug. ***

[Simon Hausmann]

The bug still needs to be present, as the duplicate report shows.

[Suresh Voruganti]

Adding the error to QtWebkit 2.1 release bug 39121. This error blocks QtWebkit 2.1 release.

[Antonio Gomes]

Some questions:

1) Is that Mac specific? (asking because it says ALL in PLATFORM field). I could try helping if it is reproducible on Linux.
2) Tor Arne, are you currently looking at it (since you are the assignee)?

[Tor Arne Vestbø]

(In reply to comment #14)
> Some questions:
> 
> 1) Is that Mac specific? (asking because it says ALL in PLATFORM field). I could try helping if it is reproducible on Linux.

It was found on mac, but the underlying problem of not tearing down plugin etc when switching views is cross platform. Also, the workaround by creating a new page in the QtLauncher would also be cross-platform.

> 2) Tor Arne, are you currently looking at it (since you are the assignee)?

Nope, feel free to take over.

[Antonio Gomes]

(In reply to comment #15)
> (In reply to comment #14)
> > Some questions:
> > 
> > 1) Is that Mac specific? (asking because it says ALL in PLATFORM field). I could try helping if it is reproducible on Linux.
> 
> It was found on mac, but the underlying problem of not tearing down plugin etc when switching views is cross platform.

I'd love to help, but I can not reproduce it so far :(

> Also, the workaround by creating a new page in the QtLauncher would also be cross-platform.

I would like to avoid that workaround if possible. One of the coolest things of our launcher is that we can keep using the same QWebPage while switching views (qwidget and qgraphicsview based). It'd be a shame if we come to remove that feature.

[Alexandra Santos]

For Decoded crash log please take a look at: https://qtrequirements.europe.nokia.com/browse/QT-3849

[Suresh Voruganti]

Any progress on the issue?

[Antonio Gomes]

Simon, did you manage do reproduce that?

[Simon Hausmann]

Created attachment 67669 [details]
Patch

[Simon Hausmann]

Committed r67554: <http://trac.webkit.org/changeset/67554>

[Simon Hausmann]

Revision r67554 cherry-picked into qtwebkit-2.1 with commit 951126b8cbe4bb5b17eed0f0b0ababc555c4177b

[Antonio Gomes]

> > Also, the workaround by creating a new page in the QtLauncher would also be cross-platform.
> 
> I would like to avoid that workaround if possible. One of the coolest things of our launcher is that we can keep using the same QWebPage while switching views (qwidget and qgraphicsview based). It'd be a shame if we come to remove that feature.

It was bad that this commit landed as is: it's been cause so many regressions:
bug 46828
bug 47613
bug 48141

I asked a couple of times how to reproduce this crash since I was doing most of the current qttestlauncher work on trunk, and would know of possible regressions, but was ignored :(

[Antonio Gomes]

(In reply to comment #23)
> > > Also, the workaround by creating a new page in the QtLauncher would also be cross-platform.
> > 
> > I would like to avoid that workaround if possible. One of the coolest things of our launcher is that we can keep using the same QWebPage while switching views (qwidget and qgraphicsview based). It'd be a shame if we come to remove that feature.
> 
> It was bad that this commit landed as is: it's been cause so many regressions:
> bug 46828
> bug 47613
> bug 48141
> 
> I asked a couple of times how to reproduce this crash since I was doing most of the current qttestlauncher work on trunk, and would know of possible regressions, but was ignored :(

Sorry if my ton was overreacted here. Maybe I was not in a good night.

I just got a bit disappointed about the regressions it caused and about that it made it harder to fix other two bugs I was working on: bug 45871 and bug 43768.

Anyway, did not mean to point fingers or blame anyone. We just needed to communicate better possibly.

[Simon Hausmann]

(In reply to comment #16)
> > Also, the workaround by creating a new page in the QtLauncher would also be cross-platform.
> 
> I would like to avoid that workaround if possible. One of the coolest things of our launcher is that we can keep using the same QWebPage while switching views (qwidget and qgraphicsview based). It'd be a shame if we come to remove that feature.

I'm sorry about the regressions the workaround caused :( but realistically I don't see how to make the view switching work reliably, if you take more advanced features into account the require bigger re-initializations. WebKit doesn't seem to be built for that. think of changing from a GL-enabled QGraphicsWebView to a QWebView, with plugins, etc. - I don't think it's worth trying to make that work but instead in the long term get rid of QWebView and based it on QGraphicsWebView.

[Alexandra Santos]

This crash is not reproducible as described above on symbian but there is an abnormal behavior going on after enabling QgraphicsView.

Here are the steps to reproduce:

1.- Flash the device with symbian 3
2.- Install Qt 4.7, Webkit 2.1 and QtTestBrowser.
3.- Launch QtTestBrowser.
4.- Go to "Options"-->"Develop"-->"QGraphicsView"-->"Toggle use of QGraphisView" 

Actual Results:

QtTestBrowser's content view goes black and the "Option" button disappears from the screen for several minutes.

Expected Results:

QtTestBrowser would not change its look after enabling QGraphicsView.

[Alexandra Santos]

Created attachment 71879 [details]
Qgraphicsview side effect...

[Alexandra Santos]

(In reply to comment #27)
> Created an attachment (id=71879) [details]
> Qgraphicsview side effect...

Screen shot attached for issue depiction.

[Suresh Voruganti]

(In reply to comment #26)
> This crash is not reproducible as described above on symbian but there is an abnormal behavior going on after enabling QgraphicsView.
> Here are the steps to reproduce:
> 1.- Flash the device with symbian 3
> 2.- Install Qt 4.7, Webkit 2.1 and QtTestBrowser.
> 3.- Launch QtTestBrowser.
> 4.- Go to "Options"-->"Develop"-->"QGraphicsView"-->"Toggle use of QGraphisView" 
> Actual Results:
> QtTestBrowser's content view goes black and the "Option" button disappears from the screen for several minutes.
> Expected Results:
> QtTestBrowser would not change its look after enabling QGraphicsView.

Do we need to re open this error?

[Antonio Gomes]

> > It was bad that this commit landed as is: it's been cause so many regressions:
> > bug 46828
> > bug 47613
> > bug 48141

For the record, bug 48440 also regressed.