Bug 65637

<rdar://problem/9784849>

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.

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.

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

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.

Created attachment 102834 [details] Detect and handle overflow in PlatformCALayerWinInternal::constrainedSize

Thanks for reviewing, Sam. I'll wait until tomorrow to commit this so that I have time to watch the bots.

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.

Sorry i clobbered the review status, but perhaps Adam would like to reconsider.

(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.

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?

(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.

(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?

Agreed.

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).

Committed r92389: <http://trac.webkit.org/changeset/92389>