Bug 32670 – QGraphicsWebView crash

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Anders Bakken]

QGraphicsWebViewPrivate assumes that it has been added to a scene and can crash when webkit calls QWebPageClient functions on it.

This attached example crashes on Qt/X11 (but likely on all platforms).
The attached patch takes care of the problem.

[Anders Bakken]

Created attachment 45082 [details]
Patch to fix bug

[Laszlo Gombos]

Anders, thanks for the patch. 

Every patch requires a ChangeLog.  See http://webkit.org/coding/contributing.html for how to create one. Next time you should also mark the patch for review to get some attentions from reviewers. 

The code changes looks good to me.

[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

[Simon Hausmann]

Anders, any update on this? It seems only the changelog is missing :)

[Simon Hausmann]

Changing severity to critical, it's a crasher.

[Jesus Sanchez-Palencia]

Created attachment 51455 [details]
Patch

[Jesus Sanchez-Palencia]

Since this is critical, I have applied the patch to trunk and added a Changelog to it. I kept Anders Bakken as the primary author of the patch.

[WebKit Commit Bot]

Comment on attachment 51455 [details]
Patch

Clearing flags on attachment: 51455

Committed r56423: <http://trac.webkit.org/changeset/56423>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Kenneth Rohde Christiansen]

Not solved properly yet, so reopening.

[Kenneth Rohde Christiansen]

Wrong bug :-(

[Jesus Sanchez-Palencia]

(In reply to comment #7)
> Since this is critical, I have applied the patch to trunk and added a Changelog
> to it. I kept Anders Bakken as the primary author of the patch.

What I mean is that I have updated the patch and not really landed it into trunk. It is now landed (thanks, Kenneth!).


I was taking another look at qgraphicswebview.cpp and noticed three other places that assume that q->scene() isn't null without checking:

- QGraphicsWebViewPrivate::~QGraphicsWebViewPrivate(), line 169.
- QGraphicsWebViewPrivate::setRootGraphicsLayer(QGraphicsItem* layer), line 199.
- QGraphicsWebViewPrivate::_q_updateMicroFocus(), line 254.

The first two we just have when ACCELERATED_COMPOSITING is enable. So I _guess_ that we could assume that we always have a QGScene on this situation. Maybe only a ASSERT is necessary?

The last one, at _q_updateMicroFocus(), would need a check as the ones from the previous patch, imho.

What do you think?


Also,
(In reply to comment #0)
> This attached example crashes on Qt/X11 (but likely on all platforms).

I think that the example is missing, Anders. It would be nice to have it as a test on QtWebKit, maybe.