65637 – Crash beneath PlatformCALayerWinInternal::updateTiles when zooming on Google Maps

RESOLVED FIXED 65637

Crash beneath PlatformCALayerWinInternal::updateTiles when zooming on Google Maps

https://bugs.webkit.org/show_bug.cgi?id=65637

Summary Crash beneath PlatformCALayerWinInternal::updateTiles when zooming on Google ...

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	Details Formatted Diff Diff
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.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware PC

OS Windows XP

Product WebKit

Component Layout and Rendering

Assignee

Adam Roben (:aroben)

Reported

2011-08-03 13:04 PDT

Modified

2011-08-04 11:15 PDT History

CC List

3 users Show

URL

http://maps.google.com/

Keywords InRadar, PlatformOnly

Depends on

Blocks