Bug 65637

Summary: Crash beneath PlatformCALayerWinInternal::updateTiles when zooming on Google Maps
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: Layout and RenderingAssignee: Adam Roben (:aroben) <aroben>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarrin, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar, PlatformOnly
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
URL: http://maps.google.com/
Attachments:
Description Flags
Detect and handle overflow in PlatformCALayerWinInternal::constrainedSize none

Adam Roben (:aroben)
Reported 2011-08-03 13:04:28 PDT
To reproduce: 1. Go to Google Maps 2. Select Satellite view 3. Zoom in and out using the scroll wheel Eventually, you'll crash beneath PlatformCALayerWinInternal::updateTiles due to a null CFArrayRef being passed to CFArrayGetValueAtIndex. Here's the backtrace: CoreFoundation.dll!CF_IS_OBJC() C++ CoreFoundation.dll!CFArrayGetValueAtIndex() + 0xe bytes C++ WebKit.dll!WebCore::PlatformCALayerWinInternal::updateTiles() Line 444 + 0x17 bytes C++ WebKit.dll!WebCore::PlatformCALayerWinInternal::setBounds(const WebCore::FloatRect & rect={...}) Line 329 C++ WebKit.dll!WebCore::PlatformCALayer::setBounds(const WebCore::FloatRect & value={...}) Line 364 C++ > WebKit.dll!WebCore::GraphicsLayerCA::updateGeometry(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}) Line 1058 C++ WebKit.dll!WebCore::GraphicsLayerCA::swapFromOrToTiledLayer(bool useTiledLayer=true, float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}) Line 2087 C++ WebKit.dll!WebCore::GraphicsLayerCA::updateGeometry(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}) Line 1018 C++ WebKit.dll!WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}) Line 894 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=true) Line 842 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=true) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=true) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=true) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=true) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=true) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=false) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=false) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=false) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::recursiveCommitChanges(float pageScaleFactor=1.0000000, const WebCore::FloatPoint & positionRelativeToBase={...}, bool affectedByPageScale=false) Line 850 C++ WebKit.dll!WebCore::GraphicsLayerCA::syncCompositingState() Line 816 C++ WebKit.dll!WebCore::RenderLayerCompositor::flushPendingLayerChanges(bool isFlushRoot=true) Line 207 + 0x12 bytes C++ WebKit.dll!WebCore::FrameView::syncCompositingStateForThisFrame(WebCore::Frame * rootFrameForSync=0x03d7b580) Line 700 C++ WebKit.dll!WebCore::FrameView::syncCompositingStateIncludingSubframes() Line 787 + 0x17 bytes C++ WebKit.dll!WebKit::LayerTreeHostCA::flushPendingLayerChanges() Line 247 C++ WebKit.dll!WebKit::LayerTreeHostCA::performScheduledLayerFlush() Line 224 + 0x8 bytes C++ WebKit.dll!WebKit::LayerTreeHostCAWin::flushPendingLayerChangesNow() Line 240 C++ WebKit.dll!WebCore::LayerChangesFlusher::hookFired(int code=0, unsigned int wParam=1, long lParam=1244088) Line 89 + 0x20 bytes C++ WebKit.dll!WebCore::LayerChangesFlusher::hookCallback(int code=0, unsigned int wParam=1, long lParam=1244088) Line 75 C++ user32.dll!_DispatchHookW@16() + 0x31 bytes user32.dll!_CallHookWithSEH@16() + 0x21 bytes user32.dll!___fnHkINLPMSG@4() + 0x25 bytes ntdll.dll!_KiUserCallbackDispatcher@12() + 0x13 bytes user32.dll!_NtUserGetMessage@16() + 0xc bytes WebKit.dll!RunLoop::run() Line 74 + 0x12 bytes C++ WebKit.dll!WebKit::WebProcessMain(const WebKit::CommandLine & commandLine={...}) Line 82 C++ WebKit.dll!WebKitMain(const WebKit::CommandLine & commandLine={...}) Line 50 + 0x9 bytes C++ WebKit.dll!WebKitMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x00021118, int nCmdShow=10) Line 187 + 0x9 bytes C++ WebKit2WebProcess.exe!wWinMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x00021118, int nCmdShow=10) Line 66 + 0x18 bytes C++ WebKit2WebProcess.exe!__tmainCRTStartup() Line 589 + 0x1c bytes C kernel32.dll!_BaseProcessStart@4() + 0x23 bytes
Attachments
Detect and handle overflow in PlatformCALayerWinInternal::constrainedSize (5.26 KB, patch)
2011-08-03 15:03 PDT, Adam Roben (:aroben) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Adam Roben (:aroben)
Comment 1 2011-08-03 13:04:52 PDT
<rdar://problem/9784849>
Adam Roben (:aroben)
Comment 2 2011-08-03 13:07:29 PDT
PlatformCALayer::setBounds is being passed a rect with size 33554432x33554432. This comes from GraphicsLayerCA::m_size. It looks like both updateTiles and constrainedSize have the same vulnerability to overflow. The overflow is breaking our calculations that try to limit the number of tiles a layer can have.
Adam Roben (:aroben)
Comment 3 2011-08-03 13:08:40 PDT
Specifically, the overflow happens in a calculation like this: int numTilesHorizontal = ceil(m_constrainedSize.width / m_tileSize.width); int numTilesVertical = ceil(m_constrainedSize.height / m_tileSize.height); int numTilesTotal = numTilesHorizontal * numTilesVertical; numTilesHorizontal and numTilesVertical are both 65536. The calculation of numTilesTotal results in overflow.
Adam Roben (:aroben)
Comment 4 2011-08-03 13:21:02 PDT
I set a breakpoint on GraphicsLayer::setSize to try to figure out where the huge size is coming from. Here's where it got hit, when passed a rect of size 67108864x67108864: > WebKit.dll!WebCore::GraphicsLayer::setSize(const WebCore::FloatSize & size={...}) Line 246 C++ WebKit.dll!WebCore::GraphicsLayerCA::setSize(const WebCore::FloatSize & size={...}) Line 402 C++ WebKit.dll!WebCore::RenderLayerBacking::updateGraphicsLayerGeometry() Line 422 + 0x2d bytes C++ WebKit.dll!WebCore::RenderLayerCompositor::rebuildCompositingLayerTree(WebCore::RenderLayer * layer=0x22bbc6bc, const WebCore::CompositingState & compositingState={...}, WTF::Vector<WebCore::GraphicsLayer *,0> & childLayersOfEnclosingLayer=[0]()) Line 850 C++ WebKit.dll!WebCore::RenderLayerCompositor::rebuildCompositingLayerTree(WebCore::RenderLayer * layer=0x0c8ac69c, const WebCore::CompositingState & compositingState={...}, WTF::Vector<WebCore::GraphicsLayer *,0> & childLayersOfEnclosingLayer=[0]()) Line 902 C++ WebKit.dll!WebCore::RenderLayerCompositor::rebuildCompositingLayerTree(WebCore::RenderLayer * layer=0x0aa51024, const WebCore::CompositingState & compositingState={...}, WTF::Vector<WebCore::GraphicsLayer *,0> & childLayersOfEnclosingLayer=[0]()) Line 902 C++ WebKit.dll!WebCore::RenderLayerCompositor::rebuildCompositingLayerTree(WebCore::RenderLayer * layer=0x064ad8e4, const WebCore::CompositingState & compositingState={...}, WTF::Vector<WebCore::GraphicsLayer *,0> & childLayersOfEnclosingLayer=[0]()) Line 902 C++ WebKit.dll!WebCore::RenderLayerCompositor::rebuildCompositingLayerTree(WebCore::RenderLayer * layer=0x064da904, const WebCore::CompositingState & compositingState={...}, WTF::Vector<WebCore::GraphicsLayer *,0> & childLayersOfEnclosingLayer=[0]()) Line 902 C++ WebKit.dll!WebCore::RenderLayerCompositor::updateCompositingLayers(WebCore::CompositingUpdateType updateType=CompositingUpdateAfterLayoutOrStyleChange, WebCore::RenderLayer * updateRoot=0x064da904) Line 307 C++ WebKit.dll!WebCore::FrameView::updateCompositingLayers() Line 629 C++ WebKit.dll!WebCore::FrameView::layout(bool allowSubtree=true) Line 1042 C++ WebKit.dll!WebCore::Document::updateLayout() Line 1621 C++ WebKit.dll!WebCore::RenderLayer::hitTest(const WebCore::HitTestRequest & request={...}, WebCore::HitTestResult & result={...}) Line 2860 C++ WebKit.dll!WebCore::Document::prepareMouseEvent(const WebCore::HitTestRequest & request={...}, const WebCore::IntPoint & documentPoint={...}, const WebCore::PlatformMouseEvent & event={...}) Line 2649 C++ WebKit.dll!WebCore::EventHandler::prepareMouseEvent(const WebCore::HitTestRequest & request={...}, const WebCore::PlatformMouseEvent & mev={...}) Line 1924 + 0x39 bytes C++ WebKit.dll!WebCore::EventHandler::handleMouseMoveEvent(const WebCore::PlatformMouseEvent & mouseEvent={...}, WebCore::HitTestResult * hoveredNode=0x0012f5a4) Line 1605 C++ WebKit.dll!WebCore::EventHandler::mouseMoved(const WebCore::PlatformMouseEvent & event={...}) Line 1536 + 0x10 bytes C++ WebKit.dll!WebKit::handleMouseEvent(const WebKit::WebMouseEvent & mouseEvent={...}, WebCore::Page * page=0x03dc8f20) Line 1053 + 0x13 bytes C++ WebKit.dll!WebKit::WebPage::mouseEvent(const WebKit::WebMouseEvent & mouseEvent={...}) Line 1079 + 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=0x03dc8b48, void (const WebKit::WebMouseEvent &)* function=0x10008f67) Line 19 + 0xf bytes C++ WebKit.dll!CoreIPC::handleMessage<Messages::WebPage::MouseEvent,WebKit::WebPage,void (__thiscall WebKit::WebPage::*)(WebKit::WebMouseEvent const &)>(CoreIPC::ArgumentDecoder * argumentDecoder=0x223acbd0, WebKit::WebPage * object=0x03dc8b48, void (const WebKit::WebMouseEvent &)* function=0x10008f67) Line 277 + 0x15 bytes C++ WebKit.dll!WebKit::WebPage::didReceiveWebPageMessage(CoreIPC::Connection * __formal=0x03da4cf0, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x223acbd0) Line 104 + 0x23 bytes C++ WebKit.dll!WebKit::WebPage::didReceiveMessage(CoreIPC::Connection * connection=0x03da4cf0, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x223acbd0) Line 2087 C++ WebKit.dll!WebKit::WebProcess::didReceiveMessage(CoreIPC::Connection * connection=0x03da4cf0, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x223acbd0) Line 642 C++ WebKit.dll!CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::ArgumentDecoder> & message={...}) Line 689 + 0x30 bytes C++ WebKit.dll!CoreIPC::Connection::dispatchMessages() Line 717 C++ WebKit.dll!MemberFunctionWorkItem0<CoreIPC::Connection>::execute() Line 79 + 0x10 bytes C++ WebKit.dll!RunLoop::performWork() Line 63 + 0x1a bytes C++ WebKit.dll!RunLoop::wndProc(HWND__ * hWnd=0x001805c8, unsigned int message=1025, unsigned int wParam=64637248, long lParam=0) Line 62 C++ WebKit.dll!RunLoop::RunLoopWndProc(HWND__ * hWnd=0x001805c8, unsigned int message=1025, unsigned int wParam=64637248, long lParam=0) Line 44 + 0x18 bytes C++ user32.dll!_InternalCallWinProc@20() + 0x28 bytes user32.dll!_UserCallWinProcCheckWow@32() + 0xb7 bytes user32.dll!_DispatchMessageWorker@8() + 0xdc 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 50 + 0x9 bytes C++ WebKit.dll!WebKitMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x00021118, int nCmdShow=10) Line 187 + 0x9 bytes C++ WebKit2WebProcess.exe!wWinMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x00021118, int nCmdShow=10) Line 66 + 0x18 bytes C++ WebKit2WebProcess.exe!__tmainCRTStartup() Line 589 + 0x1c bytes C kernel32.dll!_BaseProcessStart@4() + 0x23 bytes
Adam Roben (:aroben)
Comment 5 2011-08-03 14:20:45 PDT
Checking for overflow in PlatformCALayerWinInternal::constrainedSize makes the crash go away. Chris Marrin says we will end up just rendering the upper-left portion of the layer, which matches Mac's behavior.
Adam Roben (:aroben)
Comment 6 2011-08-03 15:03:44 PDT
Created attachment 102834 [details] Detect and handle overflow in PlatformCALayerWinInternal::constrainedSize
Adam Roben (:aroben)
Comment 7 2011-08-03 15:37:58 PDT
Thanks for reviewing, Sam. I'll wait until tomorrow to commit this so that I have time to watch the bots.
Simon Fraser (smfr)
Comment 8 2011-08-03 15:57:14 PDT
Comment on attachment 102834 [details] Detect and handle overflow in PlatformCALayerWinInternal::constrainedSize GraphicsLayerCA::constrainedSize() already does some size limiting; why does PlatformCALayerWinInternal need it too? FWIW, that code uses floats to get around the overflow issues.
Simon Fraser (smfr)
Comment 9 2011-08-03 15:57:45 PDT
Sorry i clobbered the review status, but perhaps Adam would like to reconsider.
Adam Roben (:aroben)
Comment 10 2011-08-04 07:53:28 PDT
(In reply to comment #8) > GraphicsLayerCA::constrainedSize() already does some size limiting; why does PlatformCALayerWinInternal need it too? FWIW, that code uses floats to get around the overflow issues. We in fact have three constrainedSize functions: FloatSize GraphicsLayerCA::constrainedSize() const CGSize PlatformCALayerWinInternal::constrainedSize(const CGSize& size) const CGSize WebTiledLayer::constrainedSize(const CGSize& size) const That seems like two too many to me. (I suspect that the entire WebTiledLayer class isn't even used anymore, but I haven't confirmed this yet.) I'm not sure why we have so many. Chris would probably know, since I think he wrote all but the first.
Simon Fraser (smfr)
Comment 11 2011-08-04 08:40:09 PDT
I'm just surprised that PlatformCALayerWinInternal ever saw the huge layer, since I would have expected GraphicsLayerCA to constrain first. Is that not the case?
Adam Roben (:aroben)
Comment 12 2011-08-04 08:44:55 PDT
(In reply to comment #11) > I'm just surprised that PlatformCALayerWinInternal ever saw the huge layer, since I would have expected GraphicsLayerCA to constrain first. Is that not the case? GraphicsLayerCA only constrains on Leopard and SnowLeopard. We should change it to constrain on Windows, too.
Adam Roben (:aroben)
Comment 13 2011-08-04 09:56:58 PDT
(In reply to comment #12) > (In reply to comment #11) > > I'm just surprised that PlatformCALayerWinInternal ever saw the huge layer, since I would have expected GraphicsLayerCA to constrain first. Is that not the case? > > GraphicsLayerCA only constrains on Leopard and SnowLeopard. We should change it to constrain on Windows, too. I have a patch that gets rid of PlatformCALayerWinInteral's constraining code and uses GraphicsLayerCA's instead. But I noticed a difference between the two implementations: when both width and height are too large, GraphicsLayerCA constrains whichever is larger down to just a single tile, while PlatformCALayerWinInternal constrains both proportionally until we have a small enough number of tiles. PlatformCALayerWinInternal's behavior seems slightly better, since you end up with a proportional rectangle of rendered content in the upper-left rather than just a one-tile-wide strip; maybe we should change GraphicsLayerCA to do the same thing?
Simon Fraser (smfr)
Comment 14 2011-08-04 10:25:44 PDT
Agreed.
Adam Roben (:aroben)
Comment 15 2011-08-04 10:58:37 PDT
I think it might make the most sense to do this in two parts: 1) Fix the overflow bug that caused this crash using the already-attached patch 2) Unify GraphicsLayerCA::constrainedSize and PlatformCALayerWinInternal::constrainedSize I filed bug 65705 to cover (2).
Adam Roben (:aroben)
Comment 16 2011-08-04 11:15:14 PDT
Committed r92389: <http://trac.webkit.org/changeset/92389>
Note You need to log in before you can comment on or make changes to this bug.