32670 2009-12-17 10:08:32 -0800 QGraphicsWebView crash 2010-03-23 15:53:21 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) PC All RESOLVED FIXED Qt P2 Critical --- 1 anders.bakken webkit-qt-unassigned commit-queue hausmann jesus kenneth kent.hansen laszlo.gombos oldest_to_newest 172835 0 anders.bakken 2009-12-17 10:08:32 -0800 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. 172837 1 45082 anders.bakken 2009-12-17 10:09:07 -0800 Created attachment 45082 Patch to fix bug 174754 2 laszlo.gombos 2009-12-24 03:50:39 -0800 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. 198034 3 vestbo 2010-03-10 06:27:36 -0800 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 201612 4 hausmann 2010-03-18 15:47:54 -0700 Anders, any update on this? It seems only the changelog is missing :) 201614 5 hausmann 2010-03-18 15:49:23 -0700 Changing severity to critical, it's a crasher. 203226 6 51455 jesus 2010-03-23 14:48:06 -0700 Created attachment 51455 Patch 203228 7 jesus 2010-03-23 14:52:38 -0700 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. 203261 8 51455 commit-queue 2010-03-23 15:40:23 -0700 Comment on attachment 51455 Patch Clearing flags on attachment: 51455 Committed r56423: <http://trac.webkit.org/changeset/56423> 203262 9 commit-queue 2010-03-23 15:40:28 -0700 All reviewed patches have been landed. Closing bug. 203264 10 kenneth 2010-03-23 15:43:41 -0700 Not solved properly yet, so reopening. 203265 11 kenneth 2010-03-23 15:44:32 -0700 Wrong bug :-( 203270 12 jesus 2010-03-23 15:53:21 -0700 (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. 45082 2009-12-17 10:09:07 -0800 2010-03-23 14:47:57 -0700 Patch to fix bug patch text/plain 1234 anders.bakken ZGlmZiAtLWdpdCBhL3NyYy8zcmRwYXJ0eS93ZWJraXQvV2ViS2l0L3F0L0FwaS9xZ3JhcGhpY3N3 ZWJ2aWV3LmNwcCBiL3NyYy8zcmRwYXJ0eS93ZWJraXQvV2ViS2l0L3F0L0FwaS9xZ3JhcGhpY3N3 ZWJ2aWV3LmNwcAppbmRleCBjZWI1ZWUxLi44NWExNmNmIDEwMDY0NAotLS0gYS9zcmMvM3JkcGFy dHkvd2Via2l0L1dlYktpdC9xdC9BcGkvcWdyYXBoaWNzd2Vidmlldy5jcHAKKysrIGIvc3JjLzNy ZHBhcnR5L3dlYmtpdC9XZWJLaXQvcXQvQXBpL3FncmFwaGljc3dlYnZpZXcuY3BwCkBAIC0xMzYs MTAgKzEzNiwxMiBAQCBRUGFsZXR0ZSBRR3JhcGhpY3NXZWJWaWV3UHJpdmF0ZTo6cGFsZXR0ZSgp IGNvbnN0CiBpbnQgUUdyYXBoaWNzV2ViVmlld1ByaXZhdGU6OnNjcmVlbk51bWJlcigpIGNvbnN0 CiB7CiAjaWYgZGVmaW5lZChRX1dTX1gxMSkKLSAgICBjb25zdCBRTGlzdDxRR3JhcGhpY3NWaWV3 Kj4gdmlld3MgPSBxLT5zY2VuZSgpLT52aWV3cygpOworICAgIGlmIChRR3JhcGhpY3NTY2VuZSAq c2NlbmUgPSBxLT5zY2VuZSgpKSB7CisgICAgICAgIGNvbnN0IFFMaXN0PFFHcmFwaGljc1ZpZXcq PiB2aWV3cyA9IHNjZW5lLT52aWV3cygpOwogCi0gICAgaWYgKCF2aWV3cy5pc0VtcHR5KCkpCi0g ICAgICAgIHJldHVybiB2aWV3cy5hdCgwKS0+eDExSW5mbygpLnNjcmVlbigpOworICAgICAgICBp ZiAoIXZpZXdzLmlzRW1wdHkoKSkKKyAgICAgICAgICAgIHJldHVybiB2aWV3cy5hdCgwKS0+eDEx SW5mbygpLnNjcmVlbigpOworICAgIH0KICNlbmRpZgogCiAgICAgcmV0dXJuIDA7CkBAIC0xNDcs OCArMTQ5LDExIEBAIGludCBRR3JhcGhpY3NXZWJWaWV3UHJpdmF0ZTo6c2NyZWVuTnVtYmVyKCkg Y29uc3QKIAogUVdpZGdldCogUUdyYXBoaWNzV2ViVmlld1ByaXZhdGU6Om93bmVyV2lkZ2V0KCkg Y29uc3QKIHsKLSAgICBjb25zdCBRTGlzdDxRR3JhcGhpY3NWaWV3Kj4gdmlld3MgPSBxLT5zY2Vu ZSgpLT52aWV3cygpOwotICAgIHJldHVybiB2aWV3cy52YWx1ZSgwKTsKKyAgICBpZiAoUUdyYXBo aWNzU2NlbmUgKnNjZW5lID0gcS0+c2NlbmUoKSkgeworICAgICAgICBjb25zdCBRTGlzdDxRR3Jh cGhpY3NWaWV3Kj4gdmlld3MgPSBzY2VuZS0+dmlld3MoKTsKKyAgICAgICAgcmV0dXJuIHZpZXdz LnZhbHVlKDApOworICAgIH0KKyAgICByZXR1cm4gMDsKIH0KIAogUU9iamVjdCogUUdyYXBoaWNz V2ViVmlld1ByaXZhdGU6OnBsdWdpblBhcmVudCgpIGNvbnN0Cg== 51455 2010-03-23 14:48:06 -0700 2010-03-23 15:40:23 -0700 Patch bug-32670-20100323185002.patch text/plain 1878 jesus ZGlmZiAtLWdpdCBhL1dlYktpdC9xdC9BcGkvcWdyYXBoaWNzd2Vidmlldy5jcHAgYi9XZWJLaXQv cXQvQXBpL3FncmFwaGljc3dlYnZpZXcuY3BwCmluZGV4IDEwN2EzNzYuLmQ5MjhiOGQgMTAwNjQ0 Ci0tLSBhL1dlYktpdC9xdC9BcGkvcWdyYXBoaWNzd2Vidmlldy5jcHAKKysrIGIvV2ViS2l0L3F0 L0FwaS9xZ3JhcGhpY3N3ZWJ2aWV3LmNwcApAQCAtMzI3LDEwICszMjcsMTIgQEAgUVBhbGV0dGUg UUdyYXBoaWNzV2ViVmlld1ByaXZhdGU6OnBhbGV0dGUoKSBjb25zdAogaW50IFFHcmFwaGljc1dl YlZpZXdQcml2YXRlOjpzY3JlZW5OdW1iZXIoKSBjb25zdAogewogI2lmIGRlZmluZWQoUV9XU19Y MTEpCi0gICAgY29uc3QgUUxpc3Q8UUdyYXBoaWNzVmlldyo+IHZpZXdzID0gcS0+c2NlbmUoKS0+ dmlld3MoKTsKKyAgICBpZiAoUUdyYXBoaWNzU2NlbmUqIHNjZW5lID0gcS0+c2NlbmUoKSkgewor ICAgICAgICBjb25zdCBRTGlzdDxRR3JhcGhpY3NWaWV3Kj4gdmlld3MgPSBzY2VuZS0+dmlld3Mo KTsKIAotICAgIGlmICghdmlld3MuaXNFbXB0eSgpKQotICAgICAgICByZXR1cm4gdmlld3MuYXQo MCktPngxMUluZm8oKS5zY3JlZW4oKTsKKyAgICAgICAgaWYgKCF2aWV3cy5pc0VtcHR5KCkpCisg ICAgICAgICAgICByZXR1cm4gdmlld3MuYXQoMCktPngxMUluZm8oKS5zY3JlZW4oKTsKKyAgICB9 CiAjZW5kaWYKIAogICAgIHJldHVybiAwOwpAQCAtMzM4LDggKzM0MCwxMSBAQCBpbnQgUUdyYXBo aWNzV2ViVmlld1ByaXZhdGU6OnNjcmVlbk51bWJlcigpIGNvbnN0CiAKIFFXaWRnZXQqIFFHcmFw aGljc1dlYlZpZXdQcml2YXRlOjpvd25lcldpZGdldCgpIGNvbnN0CiB7Ci0gICAgY29uc3QgUUxp c3Q8UUdyYXBoaWNzVmlldyo+IHZpZXdzID0gcS0+c2NlbmUoKS0+dmlld3MoKTsKLSAgICByZXR1 cm4gdmlld3MudmFsdWUoMCk7CisgICAgaWYgKFFHcmFwaGljc1NjZW5lKiBzY2VuZSA9IHEtPnNj ZW5lKCkpIHsKKyAgICAgICAgY29uc3QgUUxpc3Q8UUdyYXBoaWNzVmlldyo+IHZpZXdzID0gc2Nl bmUtPnZpZXdzKCk7CisgICAgICAgIHJldHVybiB2aWV3cy52YWx1ZSgwKTsKKyAgICB9CisgICAg cmV0dXJuIDA7CiB9CiAKIFFPYmplY3QqIFFHcmFwaGljc1dlYlZpZXdQcml2YXRlOjpwbHVnaW5Q YXJlbnQoKSBjb25zdApkaWZmIC0tZ2l0IGEvV2ViS2l0L3F0L0NoYW5nZUxvZyBiL1dlYktpdC9x dC9DaGFuZ2VMb2cKaW5kZXggZDZhNDZiOS4uNjBiNjVhNyAxMDA2NDQKLS0tIGEvV2ViS2l0L3F0 L0NoYW5nZUxvZworKysgYi9XZWJLaXQvcXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIw MTAtMDMtMjMgIEFuZGVycyBCYWtrZW4gPGFuZGVycy5iYWtrZW5Abm9raWEuY29tPiwgSmVzdXMg U2FuY2hlei1QYWxlbmNpYSA8amVzdXMucGFsZW5jaWFAb3BlbmJvc3NhLm9yZz4KKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDaGVjayBpZiBxLT5zY2Vu ZSgpIGlzIGF2YWlsYWJsZSBiZWZvcmUgYWN0dWFsbHkgdXNpbmcgaXQKKyAgICAgICAgb24gUUdy YXBoaWNzV2ViVmlld1ByaXZhdGUuCisKKyAgICAgICAgUUdyYXBoaWNzV2ViVmlldyBjcmFzaAor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzI2NzAKKwor ICAgICAgICAqIEFwaS9xZ3JhcGhpY3N3ZWJ2aWV3LmNwcDoKKyAgICAgICAgKFFHcmFwaGljc1dl YlZpZXdQcml2YXRlOjpzY3JlZW5OdW1iZXIpOgorICAgICAgICAoUUdyYXBoaWNzV2ViVmlld1By aXZhdGU6Om93bmVyV2lkZ2V0KToKKwogMjAxMC0wMy0yMyAgRGF2aWQgTGVvbmcgIDxkYXZpZC5s ZW9uZ0Bub2tpYS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgTGFzemxvIEdvbWJvcy4K