Bug 29551

Summary: [Qt] QtWebKit crash with Popups
Product: WebKit Reporter: Tor Arne Vestbø <vestbo>
Component: PlatformAssignee: 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 Flags
Debug backtrace
none
Patch none

Description Tor Arne Vestbø 2009-09-21 07:35:02 PDT
This bug report originated from Nokia internal issue QT-1210

--- Description ---

Jesper reported this one:

To reproduce:
Using qt-45, visit www.2shared.com.
Upload a file.
A javascript popup will say that your upload is complete
click ok - crash!

(gdb) bt
receiver=0xa7e4af8, method=0x0)
    at /home/jesper/work/depot/qt-45/src/corelib/kernel/qobject.cpp:2618
/home/jesper/work/depot/qt-45/src/network/access/qnetworkreplyimpl.cpp:482
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
from /home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
WebCore::PageGroupLoadDeferrer::~PageGroupLoadDeferrer () from
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
---Type <return> to continue, or q <return> to quit---
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
object=0xa928698) at
/home/jesper/work/depot/qt-45/src/corelib/kernel/qobject.cpp:489
/home/jesper/work/depot/qt-45/src/corelib/kernel/qobject.cpp:1106
receiver=0xa928698, e=0xa584bc8)
    at /home/jesper/work/depot/qt-45/src/gui/kernel/qapplication.cpp:4084
receiver=0xa928698, e=0xa584bc8) at
/home/jesper/work/depot/qt-45/src/gui/kernel/qapplication.cpp:3631
receiver=0xa928698, event=0xa584bc8)
    at /home/jesper/work/depot/qt-45/src/corelib/kernel/qcoreapplication.cpp:598
/home/jesper/src/qt-copy/lib/libQtWebKit.so.4
(receiver=0x0, event_type=0, data=0x9f6c600)
    at /home/jesper/work/depot/qt-45/src/corelib/kernel/qcoreapplication.cpp:1236
event_type=0) at
/home/jesper/work/depot/qt-45/src/corelib/kernel/qcoreapplication.cpp:1132
../../include/QtCore/../../../qt-45/src/corelib/kernel/qcoreapplication.h:218
/home/jesper/work/depot/qt-45/src/corelib/kernel/qeventdispatcher_glib.cpp:209
flags={i = -1077205868})
    at /home/jesper/work/depot/qt-45/src/corelib/kernel/qeventdispatcher_glib.cpp:323
(this=0x9f719d0, flags={i = -1077205820})
    at /home/jesper/work/depot/qt-45/src/gui/kernel/qguieventdispatcher_glib.cpp:202
= -1077205744}) at
/home/jesper/work/depot/qt-45/src/corelib/kernel/qeventloop.cpp:149
-1077205672}) at
/home/jesper/work/depot/qt-45/src/corelib/kernel/qeventloop.cpp:196
/home/jesper/work/depot/qt-45/src/corelib/kernel/qcoreapplication.cpp:880
/home/jesper/work/depot/qt-45/src/gui/kernel/qapplication.cpp:3553
Comment 1 Jocelyn Turcotte 2009-10-12 05:54:32 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.
Comment 2 Jocelyn Turcotte 2009-10-12 06:29:43 PDT
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.
Comment 3 Simon Hausmann 2009-10-12 22:42:35 PDT
Jocelyn, the patch looks good to me. Do you think it's possible to write a layout test or unit tests for this?
Comment 4 Holger Freyther 2009-10-12 23:37:36 PDT
Okay, a unit test would be nice.
Comment 5 Jocelyn Turcotte 2009-10-13 07:57:51 PDT
*** Bug 29592 has been marked as a duplicate of this bug. ***
Comment 6 Jocelyn Turcotte 2009-10-13 10:56:35 PDT
(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 7 Adam Barth 2009-10-18 23:00:19 PDT
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 8 Yong Li 2009-10-19 08:52:10 PDT
Comment on attachment 41035 [details]
Patch

Let commit bot land it
Comment 9 WebKit Commit Bot 2009-10-19 09:26:10 PDT
Comment on attachment 41035 [details]
Patch

Clearing flags on attachment: 41035

Committed r49789: <http://trac.webkit.org/changeset/49789>
Comment 10 WebKit Commit Bot 2009-10-19 09:26:14 PDT
All reviewed patches have been landed.  Closing bug.