Bug 29185

Summary: [Qt] QWebGraphicsItem should check for null QWebPage
Product: WebKit Reporter: Jakub Wieczorek <jwieczorek>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, kenneth, tonikitoo
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
patch to fix the segfaults
none
patch to fix the segfaults none

Jakub Wieczorek
Reported 2009-09-11 08:45:43 PDT
QWebGraphicsItem segfaults when the associated page is null.
Attachments
patch to fix the segfaults (4.43 KB, patch)
2009-09-11 08:48 PDT, Jakub Wieczorek
no flags
patch to fix the segfaults (6.07 KB, patch)
2009-09-11 10:14 PDT, Jakub Wieczorek
no flags
Jakub Wieczorek
Comment 1 2009-09-11 08:48:56 PDT
Created attachment 39436 [details] patch to fix the segfaults
Antonio Gomes
Comment 2 2009-09-11 09:08:34 PDT
jakub, looks good overall ... just one question: in comment https://bugs.webkit.org/show_bug.cgi?id=28862#c14 simon suggested to not use page() (maybe to avoid a unnecessary method call overhead), but d->page ... maybe we should respect that ? ps: although i agree w/ your idea since page() willalways return a non-null pointer
Jakub Wieczorek
Comment 3 2009-09-11 09:28:50 PDT
(In reply to comment #2) > jakub, looks good overall ... just one question: in comment > https://bugs.webkit.org/show_bug.cgi?id=28862#c14 simon suggested to not use > page() (maybe to avoid a unnecessary method call overhead), but d->page ... > maybe we should respect that ? > > ps: although i agree w/ your idea since page() willalways return a non-null > pointer I agree with the comment as far as it concerns functions that should silently return when the page is null. But most of them should just create a default page when they are called. That's also how QWebView works and I believe that QWebGraphicsItem should be symmetrical to that. Otherwise, you would never be able to load an url because the page would always be null.
Jakub Wieczorek
Comment 4 2009-09-11 10:14:25 PDT
Created attachment 39446 [details] patch to fix the segfaults This one fixes one more crash.
Kenneth Rohde Christiansen
Comment 5 2009-09-11 12:02:15 PDT
but then again I guess setGeometry etc should also create a page, so that I can do ->setGeometry(...) ->load(...) and it will actually respect the geometry. > I agree with the comment as far as it concerns functions that should silently > return when the page is null. But most of them should just create a default > page when they are called. That's also how QWebView works and I believe that > QWebGraphicsItem should be symmetrical to that. Otherwise, you would never be > able to load an url because the page would always be null.
Jakub Wieczorek
Comment 6 2009-09-11 12:14:17 PDT
(In reply to comment #5) > but then again I guess setGeometry etc should also create a page, so that I can > do > > ->setGeometry(...) > ->load(...) > > and it will actually respect the geometry. It does. QWebGraphicsItem::setPage(), that would be triggered by the load() call, sets the viewport size.
Kenneth Rohde Christiansen
Comment 7 2009-09-11 12:27:09 PDT
ah great! maybe add a unit test for this?
Simon Hausmann
Comment 8 2009-09-11 23:16:16 PDT
Comment on attachment 39446 [details] patch to fix the segfaults r=me. Thanks Jakub!
WebKit Commit Bot
Comment 9 2009-09-11 23:29:50 PDT
Comment on attachment 39446 [details] patch to fix the segfaults Rejecting patch 39446 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
Eric Seidel (no email)
Comment 10 2009-09-14 09:51:15 PDT
Comment on attachment 39446 [details] patch to fix the segfaults media/video-source-error.html -> timed out You were bitten by bug 28845.
WebKit Commit Bot
Comment 11 2009-09-14 10:08:02 PDT
Comment on attachment 39446 [details] patch to fix the segfaults Rejecting patch 39446 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
Eric Seidel (no email)
Comment 12 2009-09-14 10:12:04 PDT
Comment on attachment 39446 [details] patch to fix the segfaults svg/custom/group-opacity.svg -> failed It would appear we have a flakey SVG test.
WebKit Commit Bot
Comment 13 2009-09-14 10:26:53 PDT
Comment on attachment 39446 [details] patch to fix the segfaults Clearing flags on attachment: 39446 Committed r48357: <http://trac.webkit.org/changeset/48357>
WebKit Commit Bot
Comment 14 2009-09-14 10:26:57 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.