Bug 65637 - Crash beneath PlatformCALayerWinInternal::updateTiles when zooming on Google Maps
Summary: Crash beneath PlatformCALayerWinInternal::updateTiles when zooming on Google ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Adam Roben (:aroben)
URL: http://maps.google.com/
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2011-08-03 13:04 PDT by Adam Roben (:aroben)
Modified: 2011-08-04 11:15 PDT (History)
3 users (show)

See Also:


Attachments
Detect and handle overflow in PlatformCALayerWinInternal::constrainedSize (5.26 KB, patch)
2011-08-03 15:03 PDT, Adam Roben (:aroben)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 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
Comment 1 Adam Roben (:aroben) 2011-08-03 13:04:52 PDT
<rdar://problem/9784849>
Comment 2 Adam Roben (:aroben) 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.
Comment 3 Adam Roben (:aroben) 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.
Comment 4 Adam Roben (:aroben) 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
Comment 5 Adam Roben (:aroben) 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.
Comment 6 Adam Roben (:aroben) 2011-08-03 15:03:44 PDT
Created attachment 102834 [details]
Detect and handle overflow in PlatformCALayerWinInternal::constrainedSize
Comment 7 Adam Roben (:aroben) 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.
Comment 8 Simon Fraser (smfr) 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.
Comment 9 Simon Fraser (smfr) 2011-08-03 15:57:45 PDT
Sorry i clobbered the review status, but perhaps Adam would like to reconsider.
Comment 10 Adam Roben (:aroben) 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.
Comment 11 Simon Fraser (smfr) 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?
Comment 12 Adam Roben (:aroben) 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.
Comment 13 Adam Roben (:aroben) 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?
Comment 14 Simon Fraser (smfr) 2011-08-04 10:25:44 PDT
Agreed.
Comment 15 Adam Roben (:aroben) 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).
Comment 16 Adam Roben (:aroben) 2011-08-04 11:15:14 PDT
Committed r92389: <http://trac.webkit.org/changeset/92389>