Summary: | [Qt] QtWebKit crash with Popups | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tor Arne Vestbø <vestbo> | ||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Critical | CC: | commit-queue, hausmann, jturcotte, tonikitoo, zecke | ||||||
Priority: | P2 | Keywords: | Qt | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Tor Arne Vestbø
2009-09-21 07:35:02 PDT
Created attachment 41034 [details]
Debug backtrace
What seems to happen is that QNetworkReplyImplPrivate::outgoingData is deleted when the backend has finished dealing with the replay.
However, the network reply is trying to access this object later after the javascript alert display during the response handling.
Created attachment 41035 [details]
Patch
This patch disable the deletion of the FormDataIODevide object when the http backend has finished handling the reply.
The buffers will be deleted a bit later, when the QNetworkReplyImpl is deleted.
The disadvantages I see from this patch is that we are keeping this memory allocated for a slightly longer time.
Is there any other reason that I may have missed that this object was deleted on the finish() signal?
Other solutions would be to connect the QNetworkReplyImpl on the destroyed() signal of the outgoingData member to prevent the dangling pointer.
Jocelyn, the patch looks good to me. Do you think it's possible to write a layout test or unit tests for this? Okay, a unit test would be nice. *** Bug 29592 has been marked as a duplicate of this bug. *** (In reply to comment #3) > Jocelyn, the patch looks good to me. Do you think it's possible to write a > layout test or unit tests for this? I'm not sure, the test case is as follow: - Client sends a POST request to the server with some data. - The server respond with a text/html which displays a JavaScript alert() while loading. I tried with a local apache and the same response as the other server, can't reproduce. I have to send the request to a remote server (maybe the latency plays a role). I don't see a way of having a reliable and maintainable unit test for this test case. Tell me if you have an idea. Comment on attachment 41035 [details]
Patch
I'm impressed that you can fix a crash only by removing code. Based on the discussion, this patch appears fine. I'm sad you can't make a test. You know you can use PHP sleep to simulate long load times...
Comment on attachment 41035 [details]
Patch
Let commit bot land it
Comment on attachment 41035 [details] Patch Clearing flags on attachment: 41035 Committed r49789: <http://trac.webkit.org/changeset/49789> All reviewed patches have been landed. Closing bug. |