Bug 52629 – [Qt] Crash when calling QWebFrame::render() in response to QWebPage::repaintRequested()

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Andreas Oberritter]

Created attachment 79280 [details]
Example code to reproduce the crash

Using QWebPage to render a website into a QImage crashes frequently, if Javascript is enabled. See attached sample code and stack trace from Ubuntu natty (Qt 4.7.1) on amd64. The crash depends on the visited website. The sample code uses http://www.heise.de/. http://www.google.com/ does not seem to be affected.

I've been able to reproduce this crash with Qt 4.7.0, 4.7.1 and 4.7.1+QtWebKit-2.2 in several Linux-based environments (Qt/X11 on Ubuntu maverick and natty on i386 and amd64, Qt/Embedded on Linux mipsel 32-bit).

The crash does not occur every time, but very frequently.

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff70bfa0b in WebCore::ScrollView::repaintContentRectangle(WebCore::IntRect const&, bool) () from /usr/lib/libQtWebKit.so.4
#0  0x00007ffff70bfa0b in WebCore::ScrollView::repaintContentRectangle(WebCore::IntRect const&, bool) () from /usr/lib/libQtWebKit.so.4
#1  0x00007ffff7055199 in WebCore::FrameView::doDeferredRepaints() () from /usr/lib/libQtWebKit.so.4
#2  0x00007ffff7057be9 in WebCore::FrameView::layout(bool) () from /usr/lib/libQtWebKit.so.4
#3  0x00007ffff70cb3c2 in WebCore::ThreadTimers::sharedTimerFiredInternal() () from /usr/lib/libQtWebKit.so.4
#4  0x00007ffff5507779 in QObject::event (this=0x7ffa20, e=<value optimized out>) at kernel/qobject.cpp:1183
#5  0x00007ffff59e1024 in QApplicationPrivate::notify_helper (this=0x607000, receiver=0x7ffa20, e=0x7fffffffde80) at kernel/qapplication.cpp:4445
#6  0x00007ffff59e595a in QApplication::notify (this=<value optimized out>, receiver=0x7ffa20, e=0x7fffffffde80) at kernel/qapplication.cpp:4324
#7  0x00007ffff54f3aec in QCoreApplication::notifyInternal (this=0x7fffffffe190, receiver=0x7ffa20, event=0x7fffffffde80) at kernel/qcoreapplication.cpp:732
#8  0x00007ffff55214c2 in sendEvent (this=0x60cd50) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:215
#9  QTimerInfoList::activateTimers (this=0x60cd50) at kernel/qeventdispatcher_unix.cpp:603
#10 0x00007ffff551e2e4 in timerSourceDispatch (source=<value optimized out>) at kernel/qeventdispatcher_glib.cpp:184
#11 0x00007ffff2ea3d0d in g_main_dispatch (context=0x60bc80) at /build/buildd/glib2.0-2.27.91/glib/gmain.c:2440
#12 g_main_context_dispatch (context=0x60bc80) at /build/buildd/glib2.0-2.27.91/glib/gmain.c:3013
#13 0x00007ffff2ea44f8 in g_main_context_iterate (context=0x60bc80, block=<value optimized out>, dispatch=1, self=<value optimized out>) at /build/buildd/glib2.0-2.27.91/glib/gmain.c:3091
#14 0x00007ffff2ea4789 in g_main_context_iteration (context=0x60bc80, may_block=1) at /build/buildd/glib2.0-2.27.91/glib/gmain.c:3154
#15 0x00007ffff551e9cf in QEventDispatcherGlib::processEvents (this=0x609160, flags=<value optimized out>) at kernel/qeventdispatcher_glib.cpp:415
#16 0x00007ffff5a86f9e in QGuiEventDispatcherGlib::processEvents (this=<value optimized out>, flags=<value optimized out>) at kernel/qguieventdispatcher_glib.cpp:204
#17 0x00007ffff54f2ed2 in QEventLoop::processEvents (this=<value optimized out>, flags=...) at kernel/qeventloop.cpp:149
#18 0x00007ffff54f310c in QEventLoop::exec (this=0x7fffffffe0f0, flags=...) at kernel/qeventloop.cpp:201
#19 0x00007ffff54f751b in QCoreApplication::exec () at kernel/qcoreapplication.cpp:1009
#20 0x0000000000401c8e in main (argc=1, argv=<value optimized out>) at main.cpp:11

The crash occurs in this method:

void FrameView::doDeferredRepaints()
{
    ASSERT(!m_deferringRepaints);
    if (isOffscreen() && !shouldUpdateWhileOffscreen()) {
        m_repaintRects.clear();
        m_repaintCount = 0;
        return;
    }
    unsigned size = m_repaintRects.size();
    for (unsigned i = 0; i < size; i++) {
#if ENABLE(TILED_BACKING_STORE)
        if (frame()->tiledBackingStore()) {
            frame()->tiledBackingStore()->invalidate(m_repaintRects[i]);
            continue;
        }
#endif
        ScrollView::repaintContentRectangle(m_repaintRects[i], false);
    }
    m_repaintRects.clear();
    m_repaintCount = 0;

    updateDeferredRepaintDelay();
}

It seems as if ScrollView::repaintContentRectangle() triggered a modification of m_repaintRects, so that m_repaintRects[i] became invalid inside the loop.

Steps to reproduce:
tar -xzf ewebview-0.0.1.tar.gz && cd ewebview-0.0.1 && ./configure && make && ./src/eWebView

I'm unsure which component this bug belongs to (WebCore JavaScript or WebCore Misc.), so I left it at New Bugs.

[Andreas Oberritter]

Created attachment 79378 [details]
Quick fix or workaround.

This patch seems to fix the issue, but as I'm new to WebKit, I'm unsure whether it just hides the real problem or not.

[Benjamin Poulain]

Untested by me but I still set P1 since it is a crash in common path.

> This patch seems to fix the issue, but as I'm new to WebKit, I'm unsure whether it just hides the real problem or not.

This is not good enough. If the element removed from the array is <= current index, a rect will be ignored. You need to find what is modifying the data structure, and add a test case.

[Benjamin Poulain]

(In reply to comment #2)
> This is not good enough. If the element removed from the array is <= current index, a rect will be ignored. You need to find what is modifying the data structure, and add a test case.

I meant an autotest/unittest/layouttest, you already provided a test case ;)

[Simon Fraser (smfr)]

Did this regress at some point?

[Andreas Oberritter]

I don't know any version that doesn't crash. Apparently, Qt 4.6.3 has the same problem.

[Alexis Menard (darktears)]

I can't reproduce with the "exotic" test case.

No complain from valgrind.

Trunk revision : r79433

Need more infos.

[Andreas Kling]

Taking. I'm working on a patch for this, will post tomorrow.

[Andreas Kling]

Created attachment 87177 [details]
Proposed patch

[WebKit Review Bot]

Attachment 87177 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/ChangeLog', u'Source/WebK..." exit_code: 1

Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:2803:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Benjamin Poulain]

Comment on attachment 87177 [details]
Proposed patch

Yep, look sane.

[Andreas Kling]

Committed r82142: <http://trac.webkit.org/changeset/82142>