Created attachment 32280 [details] bt Running tst_qwebpage gives: ********* Start testing of tst_QWebPage ********* Config: Using QTest library 4.5.0, Qt 4.5.0 PASS : tst_QWebPage::initTestCase() PASS : tst_QWebPage::acceptNavigationRequest() PASS : tst_QWebPage::loadFinished() PASS : tst_QWebPage::acceptNavigationRequestWithNewWindow() ASSERTION FAILED: m_frame->view() == this (../../../WebCore/page/FrameView.cpp:493 void WebCore::FrameView::layout(bool)) The backtrace shows that the Frame is still being created at this point. (gdb) bt bt #0 0xb7522b58 in WebCore::FrameView::layout (this=0x9f28c28, allowSubtree=true) at ../../../WebCore/page/FrameView.cpp:493 #1 0xb75235b6 in WebCore::FrameView::visibleContentsResized (this=0x9f28c28) at ../../../WebCore/page/FrameView.h:215 #2 0xb759a306 in WebCore::ScrollView::updateScrollbars (this=0x9f28c28, desiredOffset=@0xbffc18c4) at ../../../WebCore/platform/ScrollView.cpp:353 #3 0xb759b4ad in WebCore::ScrollView::setFixedLayoutSize (this=0x9f28c28, newSize=@0xbffc1994) at ../../../WebCore/platform/ScrollView.cpp:199 #4 0xb7515923 in WebCore::Frame::createView (this=0x9e5c9d8, viewportSize=@0xbffc19a4, backgroundColor=@0xbffc199c, transparent=false, fixedLayoutSize=@0xbffc1994, useFixedLayout=false, horizontalScrollbarMode=WebCore::ScrollbarAuto, verticalScrollbarMode=WebCore::ScrollbarAuto) at ../../../WebCore/page/Frame.cpp:1757 In fact createView() calls setFixedLayoutSize() before it calls setView(frameView): setView(0); RefPtr<FrameView> frameView; if (isMainFrame) { frameView = FrameView::create(this, viewportSize); frameView->setFixedLayoutSize(fixedLayoutSize); frameView->setUseFixedLayout(useFixedLayout); } else frameView = FrameView::create(this); frameView->setScrollbarModes(horizontalScrollbarMode, verticalScrollbarMode); frameView->updateDefaultScrollbarState(); setView(frameView); Calling setView(frameView) before the layout calls causes numerous other cases in the qwebpage unit test to fail and times out the one that segfaults. So the answer seems to be to cater for this specific call chain in FrameView::layout(): /* The call to setFixedLayoutSize() in Frame::createView can bring us here, and at that point the frame's view is still 0. */ if (!m_frame->view()) return; ASSERT(m_frame->view() == this); My patch removes the comment and if statement from http://trac.webkit.org/changeset/29878 because it now appears to be redundant. The ASSERT above that it was intended to replace is currently un-commented in the code.
Created attachment 32281 [details] patch Decided against removing the comment and return statement from http://trac.webkit.org/changeset/29878. The assert makes the return redundant but maybe the rdar issue is still open.
I wonder why this doesn't affect other ports? Perhaps the Qt port is calling Frame methods differently, thus violating their assumptions.
(In reply to comment #2) > I wonder why this doesn't affect other ports? Perhaps the Qt port is calling > Frame methods differently, thus violating their assumptions. I'm not sure this is the case here. The sequence of events in WebCore::Frame::createView is: setView(0); frameView = FrameView::create(this, viewportSize); frameView->setFixedLayoutSize(fixedLayoutSize); frameView->setUseFixedLayout(useFixedLayout); ... setView(frameView); So the Frames m_view gets set to 0, a new view is created, and then various layout calls are made on it. It is during the call to setUseFixedLayout that WebCore::FrameView::layout fails on an ASSERT (the backtrace shows there are no intermediate calls to the qt port). This is because setView(frameView) has not yet been called on the frame. So as you can see, the assumptions all seem to be contained within WebCore::Frame::createView. I haven't done enough research to know why this situation doesn't crop up more often. I experimented with moving setView(frameView) to right after FrameView::create but it resulted in the segfaulting test case timing out, and introduced failures in others. So it seems to be in its current position for good reason.
This could be a quirk in the fixed layout size implementation. However I think if this turns out to be the correct fix, then it would be best to have it covered by a layout test for all platforms. After all the fixed layout size looks like a cross-platform feature. This may however require adding new interfaces to DumpRenderTree.
This behavior was modified in: https://bugs.webkit.org/show_bug.cgi?id=25125 Before this change, setFixedLayoutSize did not explicitly cause a layout. As for why this is hitting the Qt port and not others? Guessing: we're probably doing something different in our test case that causes a 'needsLayout.' Not sure what. However, I think for robustness this should be fixed as Robert's analysis looks correct. It is a happy coincidence that other ports aren't running into this.
I would add that this is not just a problem with setFixedLayout. As a little bit down in Frame::createView() you can see: frameView->setScrollbarModes(horizontalScrollbarMode, verticalScrollbarMode); ... setView(frameView); The call to 'setScrollbarModes' can also cause a layout now as it calls 'updateScrollbars' which in turn calls 'visibleContentsResized' which can call 'layout()'. All with the frame's current view set to 0.
This assertion is also triggered many times in the Qt DRT
as pointed out by simon, while running layout test in debug mode the following tests show the same bt: fast/dom/location-new-window-no-crash.html fast/dom/Document/early-document-access.html fast/dom/Window/closure-access-after-navigation-window.html fast/dom/Window/dom-access-from-closure-window.html fast/dom/Window/setting-properties-on-closed-window.html fast/dom/Window/window-early-properties.html fast/dom/Window/window-open-pending-url.html fast/history/history_reload.html storage/domstorage/localstorage/window-open.html storage/domstorage/sessionstorage/window-open.html ASSERTION FAILED: m_frame->view() == this (../../../../WebCore/page/FrameView.cpp:490 void WebCore::FrameView::layout(bool)) Segmentation fault 0: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/bin/DumpRenderTree [0x8053486] 1: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/bin/DumpRenderTree [0x8053899] 2: /lib/tls/i686/cmov/libc.so.6 [0xb4b97778] 3: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore9FrameView6layoutEb+0x1a2) [0xb70a1c0a] 4: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore9FrameView22visibleContentsResizedEv+0x34) [0xb70a2636] 5: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore10ScrollView16updateScrollbarsERKNS_7IntSizeE+0x140) [0xb711b95c] 6: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore10ScrollView18setFixedLayoutSizeERKNS_7IntSizeE+0x75) [0xb711cb1b] 7: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore5Frame10createViewERKNS_7IntSizeERKNS_5ColorEbS3_bNS_13ScrollbarModeES7_+0x1f3) [0xb70946fb] 8: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore19FrameLoaderClientQt31transitionToCommittedForNewPageEv+0x265) [0xb72e6999] 9: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore11FrameLoader21transitionToCommittedEN3WTF10PassRefPtrINS_10CachedPageEEE+0x3c8) [0xb70102ba] 10: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore11FrameLoader21commitProvisionalLoadEN3WTF10PassRefPtrINS_10CachedPageEEE+0x213) [0xb7011005] 11: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore14DocumentLoader13commitIfReadyEv+0x6c) [0xb6fe6f86] 12: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore14DocumentLoader10commitLoadEPKci+0x31) [0xb6fea455] 13: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore14DocumentLoader12receivedDataEPKci+0x58) [0xb6fea518] 14: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore11FrameLoader12receivedDataEPKci+0x35) [0xb6fff3d5] 15: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore18MainResourceLoader7addDataEPKcib+0x5c) [0xb702cc26] 16: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore14ResourceLoader14didReceiveDataEPKcixb+0x60) [0xb70345c0] 17: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore18MainResourceLoader14didReceiveDataEPKcixb+0x184) [0xb702b970] 18: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore14ResourceLoader14didReceiveDataEPNS_14ResourceHandleEPKcii+0x3e) [0xb70334cc] 19: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore20QNetworkReplyHandler11forwardDataEv+0x120) [0xb72ba154] 20: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/lib/libQtWebKit.so.4(_ZN7WebCore20QNetworkReplyHandler11qt_metacallEN11QMetaObject4CallEiPPv+0x92) [0xb72bbe7c] 21: /usr/lib/libQtCore.so.4(_ZN14QMetaCallEvent13placeMetaCallEP7QObject+0x34) [0xb4fb366e] 22: /usr/lib/libQtCore.so.4(_ZN7QObject5eventEP6QEvent+0x121) [0xb4fb8985] 23: /usr/lib/libQtGui.so.4(_ZN19QApplicationPrivate13notify_helperEP7QObjectP6QEvent+0x17f) [0xb5237f37] 24: /usr/lib/libQtGui.so.4(_ZN12QApplication6notifyEP7QObjectP6QEvent+0x352) [0xb52382ac] 25: /usr/lib/libQtCore.so.4(_ZN16QCoreApplication14notifyInternalEP7QObjectP6QEvent+0xa8) [0xb4fa0ad4] 26: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/bin/DumpRenderTree(_ZN16QCoreApplication9sendEventEP7QObjectP6QEvent+0x3e) [0x805277e] 27: /usr/lib/libQtCore.so.4(_ZN23QCoreApplicationPrivate16sendPostedEventsEP7QObjectiP11QThreadData+0x358) [0xb4fa105e] 28: /usr/lib/libQtCore.so.4(_ZN16QCoreApplication16sendPostedEventsEP7QObjecti+0x33) [0xb4fa12f3] 29: /usr/lib/libQtGui.so.4(_ZN16QCoreApplication16sendPostedEventsEv+0x26) [0xb5305cf6] 30: /usr/lib/libQtCore.so.4 [0xb4fd8b82] 31: /usr/lib/libglib-2.0.so.0(g_main_context_dispatch+0x1e8) [0xb475eb88] 32: /usr/lib/libglib-2.0.so.0 [0xb47620eb] 33: /usr/lib/libglib-2.0.so.0(g_main_context_iteration+0x68) [0xb4762268] 34: /usr/lib/libQtCore.so.4(_ZN20QEventDispatcherGlib13processEventsE6QFlagsIN10QEventLoop17ProcessEventsFlagEE+0xb4) [0xb4fd7d66] 35: /usr/lib/libQtGui.so.4 [0xb5304174] 36: /usr/lib/libQtCore.so.4(_ZN10QEventLoop13processEventsE6QFlagsINS_17ProcessEventsFlagEE+0xb0) [0xb4f9d0f0] 37: /usr/lib/libQtCore.so.4(_ZN10QEventLoop4execE6QFlagsINS_17ProcessEventsFlagEE+0x129) [0xb4f9d34b] 38: /usr/lib/libQtCore.so.4(_ZN16QCoreApplication4execEv+0x12f) [0xb4fa1429] 39: /usr/lib/libQtGui.so.4(_ZN12QApplication4execEv+0x24) [0xb5237c50] 40: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/bin/DumpRenderTree [0x8053237] 41: /lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe5) [0xb4b82775] 42: /home/agomes/webkit/staikos/webkit/WebKitBuild/Qt/Debug/bin/DumpRenderTree [0x804e711]
Ok, I know the problem. setFixedLayoutSize is called with a different value than the default because WebCore::IntSize's default constructed value is valid: 0x0. Qt's QSize default constructed value is invalid: -1x-1. Thus, a mismatch occurs and it setFixedLayoutSize tries to do a layout. This only happens with Qt port because other ports use IntSize(0x0) which is the same default constructed. However, it is important to note that if Frame::createView(...) was called with different values than the default ScrollView constructed values for scrollbarModes for instance, this same crash would happen. I'm producing a patch for the QSize/IntSize problem, but the larger issue should still be fixed.
Comment on attachment 32281 [details] patch Larger issues at work.
Created attachment 32717 [details] Fix the QSize/IntSize mismatch
Comment on attachment 32717 [details] Fix the QSize/IntSize mismatch Personally I'd prefer this in IntSizeQt.cpp but the issue is that it's impossible to detect whether it's QSize() or QSize(-1, -1). We'd like to convert the first one to QSize(0, 0) but not the latter (because it will mess up conversions after doing any kind of subtraction of IntSizes). So I think this will do for now.
Landed the Qt part and fixed with r45862.
Created attachment 32721 [details] Fixes the more general problem I made a check in the new FrameView::visibleContentsResized method to ensure the view is attached before layouting. I think this is a better place for it so we can still keep the ASSERT in layout() which proved to be valuable.
Comment on attachment 32721 [details] Fixes the more general problem Just make it a null check instead rather than checking for inequality with this. r=me with that change.
Landed with r45863. All better.
Created attachment 42827 [details] Fixes the test
Comment on attachment 42827 [details] Fixes the test wrong bug number