Bug 35247 - [Qt] QtTestBrowser crashes when enabling QGraphicsView mode after first loading page without it enabled
Summary: [Qt] QtTestBrowser crashes when enabling QGraphicsView mode after first loadi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P3 Normal
Assignee: Simon Hausmann
URL:
Keywords: Qt
: 44688 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-02-22 10:41 PST by Jesus Sanchez-Palencia
Modified: 2010-10-28 10:24 PDT (History)
10 users (show)

See Also:


Attachments
Backtrace (1.83 KB, text/rtf)
2010-02-22 10:41 PST, Jesus Sanchez-Palencia
no flags Details
Patch (2.41 KB, patch)
2010-09-15 06:26 PDT, Simon Hausmann
vestbo: review+
Details | Formatted Diff | Diff
Qgraphicsview side effect... (12.92 KB, image/png)
2010-10-26 07:31 PDT, Alexandra Santos
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jesus Sanchez-Palencia 2010-02-22 10:41:45 PST
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.
Comment 1 Tor Arne Vestbø (not active) 2010-03-10 06:42:32 PST
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
Comment 2 Kent Hansen 2010-03-16 04:08:34 PDT
Reproduced with r55986, Qt 4.7, same backtrace.
Comment 3 Simon Hausmann 2010-04-26 07:24:28 PDT
Jesus, does this problem still exist? Can this bug be closed?

Could you attach the backtrace as a plain text file?
Comment 4 Jesus Sanchez-Palencia 2010-04-26 11:15:50 PDT
(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
Comment 5 Kent Hansen 2010-04-27 05:36:58 PDT
(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.
Comment 6 Kent Hansen 2010-04-27 07:33:50 PDT
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.
Comment 7 Tor Arne Vestbø (not active) 2010-04-28 03:40:32 PDT
Looking at this
Comment 8 Tor Arne Vestbø (not active) 2010-04-28 04:11:39 PDT
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.
Comment 9 Tor Arne Vestbø (not active) 2010-04-28 04:35:50 PDT
(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.
Comment 10 Kenneth Rohde Christiansen 2010-04-28 05:52:32 PDT
Jeez, how does this work with your new PageClient patch? Any improvement?
Comment 11 Tor Arne Vestbø (not active) 2010-08-28 04:07:14 PDT
*** Bug 44688 has been marked as a duplicate of this bug. ***
Comment 12 Simon Hausmann 2010-08-30 05:34:44 PDT
The bug still needs to be present, as the duplicate report shows.
Comment 13 Suresh Voruganti 2010-08-30 13:14:48 PDT
Adding the error to QtWebkit 2.1 release bug 39121. This error blocks QtWebkit 2.1 release.
Comment 14 Antonio Gomes 2010-08-30 13:44:13 PDT
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)?
Comment 15 Tor Arne Vestbø (not active) 2010-08-30 14:34:44 PDT
(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.
Comment 16 Antonio Gomes 2010-08-30 14:49:26 PDT
(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.
Comment 17 Alexandra Santos 2010-09-01 13:27:24 PDT
For Decoded crash log please take a look at: https://qtrequirements.europe.nokia.com/browse/QT-3849
Comment 18 Suresh Voruganti 2010-09-07 06:48:31 PDT
Any progress on the issue?
Comment 19 Antonio Gomes 2010-09-14 20:55:39 PDT
Simon, did you manage do reproduce that?
Comment 20 Simon Hausmann 2010-09-15 06:26:34 PDT
Created attachment 67669 [details]
Patch
Comment 21 Simon Hausmann 2010-09-15 06:32:37 PDT
Committed r67554: <http://trac.webkit.org/changeset/67554>
Comment 22 Simon Hausmann 2010-09-16 00:31:47 PDT
Revision r67554 cherry-picked into qtwebkit-2.1 with commit 951126b8cbe4bb5b17eed0f0b0ababc555c4177b
Comment 23 Antonio Gomes 2010-10-24 05:56:28 PDT
> > 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 :(
Comment 24 Antonio Gomes 2010-10-25 06:52:51 PDT
(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.
Comment 25 Simon Hausmann 2010-10-25 08:31:51 PDT
(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.
Comment 26 Alexandra Santos 2010-10-26 07:30:32 PDT
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.
Comment 27 Alexandra Santos 2010-10-26 07:31:33 PDT
Created attachment 71879 [details]
Qgraphicsview side effect...
Comment 28 Alexandra Santos 2010-10-26 07:32:32 PDT
(In reply to comment #27)
> Created an attachment (id=71879) [details]
> Qgraphicsview side effect...

Screen shot attached for issue depiction.
Comment 29 Suresh Voruganti 2010-10-27 11:50:03 PDT
(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?
Comment 30 Antonio Gomes 2010-10-28 10:24:10 PDT
> > 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.