Summary: | REGRESSION (r79167): Windowed plugins in Google Reader don't move when the article list is scrolled | ||
---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> |
Component: | Plug-ins | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | ademar, andersca, jamesr, jhoneycutt, mitz |
Priority: | P2 | Keywords: | InRadar, PlatformOnly, Regression |
Version: | 528+ (Nightly build) | ||
Hardware: | PC | ||
OS: | Windows 7 | ||
URL: | data:text/html,<div style="overflow:scroll;width:300px"><iframe width=500 height=314 src="http://www.youtube.com/embed/mYP4MgxDV2U"> | ||
Bug Depends on: | 60776 | ||
Bug Blocks: | |||
Attachments: |
Description
Adam Roben (:aroben)
2011-05-04 11:53:19 PDT
The bug does not occur in Safari 5. Reduction: data:text/html,<div style="overflow:scroll;width:300px"><iframe width=500 height=314 src="http://www.youtube.com/embed/mYP4MgxDV2U" frameborder=0 allowfullscreen> Further reduction: data:text/html,<div style="overflow:scroll;width:300px"><iframe width=500 height=314 src="http://www.youtube.com/embed/mYP4MgxDV2U"> r79167 is starting to look suspicious: <http://trac.webkit.org/changeset/79167> Here's the backtrace when the plugin is repositioned due to scrolling with r79167 rolled out: > WebKit.dll!WebKit::NetscapePlugin::scheduleWindowedGeometryUpdate() Line 174 C++ WebKit.dll!WebKit::NetscapePlugin::platformGeometryDidChange() Line 149 C++ WebKit.dll!WebKit::NetscapePlugin::geometryDidChange(const WebCore::IntRect & frameRect={...}, const WebCore::IntRect & clipRect={...}) Line 549 C++ WebKit.dll!WebKit::PluginView::viewGeometryDidChange() Line 640 + 0x2e bytes C++ WebKit.dll!WebKit::PluginView::frameRectsChanged() Line 547 C++ WebKit.dll!WebCore::ScrollView::frameRectsChanged() Line 841 + 0x21 bytes C++ WebKit.dll!WebCore::ScrollView::setFrameRect(const WebCore::IntRect & newRect={...}) Line 831 + 0xf bytes C++ WebKit.dll!WebCore::FrameView::setFrameRect(const WebCore::IntRect & newRect={...}) Line 374 C++ WebKit.dll!WebCore::RenderWidget::setWidgetGeometry(const WebCore::IntRect & frame={...}, const WebCore::IntSize & boundsSize={...}) Line 177 + 0x21 bytes C++ WebKit.dll!WebCore::RenderWidget::updateWidgetPosition() Line 347 + 0x1c bytes C++ WebKit.dll!WebCore::RenderView::updateWidgetPositions() Line 618 + 0x13 bytes C++ WebKit.dll!WebCore::RenderLayer::scrollTo(int x=6, int y=0) Line 1368 C++ WebKit.dll!WebCore::RenderLayer::setScrollOffset(const WebCore::IntPoint & offset={...}) Line 1656 C++ WebKit.dll!WebCore::ScrollableArea::setScrollOffsetFromAnimation(const WebCore::IntPoint & offset={...}) Line 140 + 0x13 bytes C++ WebKit.dll!WebCore::ScrollAnimator::notityPositionChanged() Line 130 C++ WebKit.dll!WebCore::ScrollAnimator::scrollToOffsetWithoutAnimation(const WebCore::FloatPoint & offset={...}) Line 80 + 0xf bytes C++ WebKit.dll!WebCore::ScrollableArea::scrollToOffsetWithoutAnimation(const WebCore::FloatPoint & offset={...}) Line 104 + 0x1e bytes C++ WebKit.dll!WebCore::ScrollableArea::scrollToXOffsetWithoutAnimation(float x=6.0272727) Line 118 C++ WebKit.dll!WebCore::ScrollableArea::scrollToOffsetWithoutAnimation(WebCore::ScrollbarOrientation orientation=HorizontalScrollbar, float offset=6.0272727) Line 111 C++ WebKit.dll!WebCore::Scrollbar::moveThumb(int pos=124, bool draggingDocument=false) Line 302 C++ WebKit.dll!WebCore::Scrollbar::mouseMoved(const WebCore::PlatformMouseEvent & evt={...}) Line 340 C++ WebKit.dll!WebCore::EventHandler::handleMouseMoveEvent(const WebCore::PlatformMouseEvent & mouseEvent={...}, WebCore::HitTestResult * hoveredNode=0x0034f240) Line 1574 + 0x19 bytes C++ WebKit.dll!WebCore::EventHandler::mouseMoved(const WebCore::PlatformMouseEvent & event={...}) Line 1524 + 0x10 bytes C++ WebKit.dll!WebKit::handleMouseEvent(const WebKit::WebMouseEvent & mouseEvent={...}, WebCore::Page * page=0x00abef00) Line 1011 + 0x13 bytes C++ WebKit.dll!WebKit::WebPage::mouseEvent(const WebKit::WebMouseEvent & mouseEvent={...}) Line 1037 + 0x15 bytes C++ WebKit.dll!CoreIPC::callMemberFunction<WebKit::WebPage,void (__thiscall WebKit::WebPage::*)(WebKit::WebMouseEvent const &),WebKit::WebMouseEvent>(const CoreIPC::Arguments1<WebKit::WebMouseEvent> & args={...}, WebKit::WebPage * object=0x00aefd70, void (const WebKit::WebMouseEvent &)* function=0x53298e36) Line 19 + 0xf bytes C++ WebKit.dll!CoreIPC::handleMessage<Messages::WebPage::MouseEvent,WebKit::WebPage,void (__thiscall WebKit::WebPage::*)(WebKit::WebMouseEvent const &)>(CoreIPC::ArgumentDecoder * argumentDecoder=0x053cf0f8, WebKit::WebPage * object=0x00aefd70, void (const WebKit::WebMouseEvent &)* function=0x53298e36) Line 265 + 0x15 bytes C++ WebKit.dll!WebKit::WebPage::didReceiveWebPageMessage(CoreIPC::Connection * __formal=0x00a5aca8, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x053cf0f8) Line 88 + 0x23 bytes C++ WebKit.dll!WebKit::WebPage::didReceiveMessage(CoreIPC::Connection * connection=0x00a5aca8, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x053cf0f8) Line 1974 C++ WebKit.dll!WebKit::WebProcess::didReceiveMessage(CoreIPC::Connection * connection=0x00a5aca8, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x053cf0f8) Line 644 C++ WebKit.dll!CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::ArgumentDecoder> & message={...}) Line 690 + 0x30 bytes C++ WebKit.dll!CoreIPC::Connection::dispatchMessages() Line 714 + 0x15 bytes C++ WebKit.dll!MemberFunctionWorkItem0<CoreIPC::Connection>::execute() Line 76 + 0x10 bytes C++ WebKit.dll!RunLoop::performWork() Line 63 + 0x1a bytes C++ WebKit.dll!RunLoop::wndProc(HWND__ * hWnd=0x000f01f2, unsigned int message=1025, unsigned int wParam=10789288, long lParam=0) Line 62 C++ WebKit.dll!RunLoop::RunLoopWndProc(HWND__ * hWnd=0x000f01f2, unsigned int message=1025, unsigned int wParam=10789288, long lParam=0) Line 44 + 0x18 bytes C++ user32.dll!_InternalCallWinProc@20() + 0x23 bytes user32.dll!_UserCallWinProcCheckWow@32() + 0xb7 bytes user32.dll!_DispatchMessageWorker@8() + 0xed bytes user32.dll!_DispatchMessageW@4() + 0xf bytes WebKit.dll!RunLoop::run() Line 78 + 0xc bytes C++ WebKit.dll!WebKit::WebProcessMain(const WebKit::CommandLine & commandLine={...}) Line 82 C++ WebKit.dll!WebKitMain(const WebKit::CommandLine & commandLine={...}) Line 48 + 0x9 bytes C++ WebKit.dll!WebKitMain(HINSTANCE__ * hInstance=0x01160000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x0077463a, int nCmdShow=10) Line 172 + 0x9 bytes C++ WebKit2WebProcess.exe!wWinMain(HINSTANCE__ * hInstance=0x01160000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x0077463a, int nCmdShow=10) Line 66 + 0x18 bytes C++ WebKit2WebProcess.exe!__tmainCRTStartup() Line 589 + 0x1c bytes C kernel32.dll!@BaseThreadInitThunk@12() + 0x12 bytes ntdll.dll!___RtlUserThreadStart@8() + 0x27 bytes ntdll.dll!__RtlUserThreadStart@8() + 0x1b bytes Presumably the bug is that we're not calling frameRectsChanged from ScrollView::setFrameRect anymore during scrolling. Moving the frameRectsChanged call from setBoundsSize to setFrameRect seems to fix the bug. Now I need to write a test. Created attachment 92656 [details]
Tell ScrollView's children their frameRects have changed when its own frameRect changes
Comment on attachment 92656 [details]
Tell ScrollView's children their frameRects have changed when its own frameRect changes
I'm a little worried that I may have broken some things with this patch. I'm going to do some more testing on Monday to try to find out.
Comment on attachment 92656 [details]
Tell ScrollView's children their frameRects have changed when its own frameRect changes
Gonna do this a liiiiiittle differently.
I think I might be able to make a non-pixel test, too, by dumping the position of the plugin HWND. I talked to James Robinson and came up with a plan. Here are some snippets of our conversation in IRC: <aroben> jamesr: including the call to frameRectsChanged <aroben> jamesr: this was done on the assumption that scrollbars didn't need to be respositioned when the ScrollView is repositioned <aroben> jamesr: (I guess that means that scrollbars are positioned relative to the ScrollView, and move along with it automatically) <jamesr> they are, yeah <aroben> jamesr: that change caused https://bugs.webkit.org/show_bug.cgi?id=60194 because frameRectsChanged did more than just reposition scrollbars <aroben> jamesr: so I'm trying to figure out how to fix it <jamesr> my vague recollection is that setFrameRects did a whole lot and it wasn't clear to me what it should or shouldn't be doing <aroben> jamesr: I could move the positionScrollbarLayers call out of frameRectsChanged <jamesr> the call is there to try to catch adding/removing scrollbars, i think <jamesr> aroben: the other difficulty for you is that scrollbar layers are only created on the mac port when overlay scrollbars are in use, and last i checked DumpRenderTree did not use overlay scrollbars on any platform <aroben> jamesr: frameRectsChanged is called in four places: 1) updateScrollbars (when scrollbars are added/removed), 2) scrollContents, 3) setBoundsSize, 4) recursively from frameRectsChanged <jamesr> yeah, you should only need the positionScrollbarLayers() call in (1) and (3) <aroben> jamesr: ok, so maybe I should call positionScrollbarLayers() explicitly in those two places and move it out of frameRectsChanged <aroben> jamesr: and then move the frameRectsChanged call back from setBoundsSize to setFrameRect <jamesr> aroben: that sounds like it should work! I have a new (dumpAsText) test that should show this bug. We put a plugin inside an iframe inside a scrolled overflow area and dump the plugin window's rect. But for some reason it's giving me the same behavior both before and after fixing this bug! I now have a working test. Time to fix the bug! Created attachment 93470 [details]
Tell ScrollView's child Widgets that their frame rects have changed when its own frame rect changes
Committed r86442: <http://trac.webkit.org/changeset/86442> Revision r86442 cherry-picked into qtwebkit-2.2 with commit 457a0f0 <http://gitorious.org/webkit/qtwebkit/commit/457a0f0> |