Bug 33373

Created attachment 46127 [details] Proposed patch This patch enables optimized scrolling and makes sure that (Qt) PluginViews are invalidated when scrolling. (This is needed for them to move themselves properly.) No regressions in the existing auto and manual tests on Qt/X11. Comments very welcome!

style-queue ran check-webkit-style on attachment 46127 [details] without any errors.

Comment on attachment 46127 [details] Proposed patch This looks sane to me. It seems whoever put in the previous Qt scrolling code might want to look at this though.

Holger, Kenneth: Could you take a look at this one? It appears that the workaround for full repaints on pages with widgets isn't needed anymore.

First thing. Have you encountered a crash or why are you adding a null check? If you have encountered a crash please create a LayoutTest for this if not please remove it.

The second thing starts with an apology. The commit message I used when adding this it total crap, it lacks a reference to a testcase, a bug report... I'm really sorry about this. IIRC I added the code before we even supported Netscape Plugins in QtWebKIt. This means we will need to check this patach not only against Netscape Plugins support but also the QWidget embedding. Now to the second apology. We currently do not have any (automatic/manual) test for scrolling of embedded Qt plugins. Could you help out on this? The manual test should be a testshell that embeds a QLabel as "alien" and non-alien widget and then asks the user to scroll it.

Yes, I agree that we need to make sure that there are no regressions with this patch with regard to Qt plugins. I already wanted to remove this earlier as it is not really required for Netscape plugins, so the patch is heading in the right direction.

I added the null pointer check due to a crash that occurs when loading WebCore/manual-tests/qt/qtplugin.html: WebCore::PluginView::invalidateRect(WebCore::IntRect const&) (PluginViewQt.cpp:653) WebCore::Widget::invalidate() (Widget.h:147) WebCore::PluginView::updatePluginWidget() (PluginViewQt.cpp:133) WebCore::PluginView::frameRectsChanged() (PluginView.cpp:140) WebCore::Widget::setFrameRect(WebCore::IntRect const&) (WidgetQt.cpp:75) WebCore::PluginView::setFrameRect(WebCore::IntRect const&) (PluginView.cpp:124) WebCore::RenderWidget::setWidgetGeometry(WebCore::IntRect const&) (RenderWidget.cpp:159) WebCore::RenderWidget::updateWidgetPosition() (RenderWidget.cpp:308) WebCore::RenderView::updateWidgetPositions() (RenderView.cpp:543) WebCore::FrameView::performPostLayoutTasks() (FrameView.cpp:1380) WebCore::FrameView::layout(bool) (FrameView.cpp:732) WebCore::FrameView::visibleContentsResized() (FrameView.cpp:1010) WebCore::ScrollView::updateScrollbars(WebCore::IntSize const&) (ScrollView.cpp:403) WebCore::ScrollView::setContentsSize(WebCore::IntSize const&) (ScrollView.cpp:239) WebCore::FrameView::setContentsSize(WebCore::IntSize const&) (FrameView.cpp:372) WebCore::FrameView::adjustViewSize() (FrameView.cpp:392) WebCore::FrameView::layout(bool) (FrameView.cpp:698) WebCore::FrameView::layoutTimerFired(WebCore::Timer<WebCore::FrameView>*) (FrameView.cpp:1119) WebCore::Timer<WebCore::FrameView>::fired() (Timer.h:98) WebCore::ThreadTimers::sharedTimerFiredInternal() (ThreadTimers.cpp:112) WebCore::ThreadTimers::sharedTimerFired() (ThreadTimers.cpp:90) WebCore::SharedTimerQt::timerEvent(QTimerEvent*) (SharedTimerQt.cpp:105) QObject::event(QEvent*) (qobject.cpp:1204) ... QApplication::exec() (qapplication.cpp:3576) main (main.cpp:37)

Created attachment 46284 [details] Same patch, with a manual test added Added a simple manual test with one alien and one non-alien QLabel as per Holger's suggestion.

Attachment 46284 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKitTools/QtLauncher/main.cpp:656: Declaration has space between type name and * in QLabel *l [whitespace/declaration] [3] Total errors found: 1

Created attachment 46285 [details] Same patch, style complaints addressed

Comment on attachment 46285 [details] Same patch, style complaints addressed Cool. The patch looks ready. I wish we could automate the crasher too. :)

Comment on attachment 46285 [details] Same patch, style complaints addressed Clearing flags on attachment: 46285 Committed r53259: <http://trac.webkit.org/changeset/53259>

All reviewed patches have been landed. Closing bug.

(In reply to comment #13) > (From update of attachment 46285 [details]) > Clearing flags on attachment: 46285 > > Committed r53259: <http://trac.webkit.org/changeset/53259> Cherry-picked into qtwebkit-4.6 with commit ca7b2e1e1ca558050cf49dd8f7c9b35e4b9d4df5