My favorites | Sign in
Logo
             
New issue | Search
for
| Advanced search | Search tips
Issue 2044: [CRASH]Renderer crashes when zoomed into street level on maps.yahoo.com
3 people starred this issue and may be notified of changes. Back to list
 
Reported by sunandt@chromium.org, Sep 10, 2008
What steps will reproduce the problem?
1. Go to maps.yahoo.com
2. Get the directions from one place to another
3. Now Zoom in to Street level

What is the expected output? 
Map should be zoomed into street level

What do you see instead?
Renderer crashes. 

Please use labels and text to provide additional information.
Report Id's: 
64f8d5ec314aa1a0
87f0fb3a1eefc3ba
fd0684fb38b605cd

Call Stack
-----------
Thread 1 *CRASHED* (EXCEPTION_BREAKPOINT @0x0100c414)
0x0100c414 	[chrome.dll 	+ 0x0000c414] 	
0x01163ec1 	[chrome.dll 	+ 0x00163ec1] 	
0x011648e1 	[chrome.dll 	+ 0x001648e1] 	
0x01153bea 	[chrome.dll 	+ 0x00153bea] 	

Comment 1 by sunandt@chromium.org, Sep 10, 2008
In version 149.29, the tab hangs
Comment 2 by niranjan@chromium.org, Sep 10, 2008
(No comment was entered for this change.)
Labels: -Version152.0_r1977 v-152.0
Comment 3 by sunandt@chromium.org, Sep 10, 2008
(No comment was entered for this change.)
Labels: -Pri-2 Pri-1
Comment 4 by mal.chromium, Sep 10, 2008
This is clearly a regression.

Looks like there is a symbol upload problem with this build :( (I've asked someone to 
look into it.)

Not knowing anything about this crash makes it hard to assign... so I've picked Ojan 
semi-randomly.
Status: Assigned
Owner: o...@chromium.org
Labels: dev-release-block
Comment 5 by nsylvain@chromium.org, Sep 11, 2008
FAULTING_IP: 
chrome_1000000!logging::LogMessage::~LogMessage+25f [c:\b\slave\chrome-
official\build\src\base\logging.cc @ 497]
0100c414 cc              int     3

EXCEPTION_RECORD:  ffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 0100c414 
(chrome_1000000!logging::LogMessage::~LogMessage+0x0000025f)
   ExceptionCode: 80000003 (Break instruction exception)
  ExceptionFlags: 00000000
NumberParameters: 3
   Parameter[0]: 00000000
   Parameter[1]: 00d6ee78
   Parameter[2]: 07910001

DEFAULT_BUCKET_ID:  STATUS_BREAKPOINT

PROCESS_NAME:  chrome.exe

ERROR_CODE: (NTSTATUS) 0x80000003 - {EXCEPTION}  Breakpoint  A breakpoint has been 
reached.

FAULTING_THREAD:  000017b8

PRIMARY_PROBLEM_CLASS:  STATUS_BREAKPOINT

BUGCHECK_STR:  APPLICATION_FAULT_STATUS_BREAKPOINT

LAST_CONTROL_TRANSFER:  from 01163ec2 to 0100c414

STACK_TEXT:  
00d6ef38 01163ec2 02ad28f0 8001118c 00000000 
chrome_1000000!logging::LogMessage::~LogMessage+0x25f [c:\b\slave\chrome-
official\build\src\base\logging.cc @ 497]
00d6f030 011648e2 8001118c 00004758 00002228 
chrome_1000000!gfx::BitmapPlatformDeviceWin::create+0x9c [c:\b\slave\chrome-
official\build\src\base\gfx\bitmap_platform_device_win.cc @ 275]
00d6f050 011647f6 00004758 00002228 00000000 
chrome_1000000!gfx::PlatformCanvasWin::createPlatformDevice+0x22 [c:\b\slave\chrome-
official\build\src\base\gfx\platform_canvas_win.cc @ 75]
00d6f070 011647c4 00004758 00002228 00000000 
chrome_1000000!gfx::PlatformCanvasWin::initialize+0x1d [c:\b\slave\chrome-
official\build\src\base\gfx\platform_canvas_win.cc @ 41]
00d6f08c 01321770 00004758 00002228 00000000 
chrome_1000000!gfx::PlatformCanvasWin::PlatformCanvasWin+0x24 [c:\b\slave\chrome-
official\build\src\base\gfx\platform_canvas_win.cc @ 23]
00d6f0a8 01329790 00004758 00002228 02ac7838 
chrome_1000000!WebCore::GraphicsContext::createOffscreenContext+0x5f 
[c:\b\slave\chrome-
official\build\src\webkit\port\platform\graphics\graphicscontextskia.cpp @ 722]
00d6f0c0 0111dcb0 02dbea58 00b7bc60 4608a000 
chrome_1000000!WebCore::ImageBuffer::create+0x33 [c:\b\slave\chrome-
official\build\src\webkit\port\platform\graphics\imagebufferskia.cpp @ 47]
00d6f0e0 01153beb 01b3c5d5 01152ba4 00b7bc60 
chrome_1000000!WebCore::HTMLCanvasElement::createDrawingContext+0xb2 
[c:\b\slave\chrome-official\build\src\webkit\pending\htmlcanvaselement.cpp @ 313]
00d6f0e8 01152ba4 00b7bc60 011a7155 0579dfa8 
chrome_1000000!WebCore::CanvasRenderingContext2D::drawingContext+0x17 
[c:\b\slave\chrome-official\build\src\webkit\pending\canvasrenderingcontext2d.cpp @ 
1175]
00d6f0f0 011a7155 0579dfa8 00b7bc6c 015b805a 
chrome_1000000!WebCore::CanvasRenderingContext2D::setStrokeStyle+0x2a 
[c:\b\slave\chrome-official\build\src\webkit\pending\canvasrenderingcontext2d.cpp @ 
143]
00d6f0fc 015b805a 00b7bc68 00b7bc5c 00d6f128 
chrome_1000000!WebCore::V8Custom::v8CanvasRenderingContext2DStrokeStyleAccessorSetter
+0x27 [c:\b\slave\chrome-official\build\src\webkit\port\bindings\v8\v8_custom.cpp @ 
467]
00d6f140 015c2b5c 011a712e 019243b5 00b7bc68 
chrome_1000000!v8::internal::JSObject::SetPropertyWithCallback+0x18a 
[c:\b\slave\chrome-official\build\src\v8\src\objects.cc @ 1328]
00d6f178 015c1f5c 00d6f198 019243b5 0193e989 
chrome_1000000!v8::internal::JSObject::SetProperty+0x23c [c:\b\slave\chrome-
official\build\src\v8\src\objects.cc @ 1555]
00d6f1a8 015d8bd2 019243b5 0193e989 00000000 
chrome_1000000!v8::internal::JSObject::SetProperty+0x3c [c:\b\slave\chrome-
official\build\src\v8\src\objects.cc @ 1282]
00d6f1c8 0160aa71 00d6f1f8 00d6f25c 00d6f258 
chrome_1000000!v8::internal::SetProperty+0x22 [c:\b\slave\chrome-
official\build\src\v8\src\handles.cc @ 185]
00d6f20c 0160abfe 00d6f25c 00d6f258 00d6f254 
chrome_1000000!v8::internal::Runtime::SetObjectProperty+0x141 [c:\b\slave\chrome-
official\build\src\v8\src\runtime.cc @ 1310]
00d6f228 01e00224 00000003 00d6f25c 01b344b9 
chrome_1000000!v8::internal::Runtime_SetProperty+0x5e [c:\b\slave\chrome-
official\build\src\v8\src\runtime.cc @ 1365]
WARNING: Frame IP not in any known module. Following frames may be wrong.
00d6f24c 020d4e7b 0193e989 019243b5 01b3c5d5 0x1e00224
00d6f288 01e008c1 01e00135 01b4a30d 01b3c5d5 0x20d4e7b
00d6f2ac 0200f49b 01b4a30d 01b3c5d5 01b446fd 0x1e008c1
00d6f2d4 01e008c1 01e00135 01e00161 01e00101 0x200f49b
00d6f318 02030407 01e00161 01e00101 01e00179 0x1e008c1
00d6f388 01e008c1 01e00135 01e00161 01e00101 0x2030407
00d6f3d0 02007f12 01e00135 01e00101 01e00179 0x1e008c1
00d6f414 01e008c1 01e00135 01e00101 01e00179 0x2007f12
00d6f458 01f83cad 01e00101 01e00179 01e00161 0x1e008c1
00d6f4a8 0190cb4f 01b61c25 01e18d0d 01b12dc9 0x1f83cad
00d6f4c0 01e008c1 01b61c25 00000006 01b12dc9 0x190cb4f
00d6f4d8 01fc7195 00000002 01c9fe39 00000024 0x1e008c1
00d6f518 01fc2320 00000002 01a8c431 01b31c71 0x1fc7195
00d6f534 01fc5ab1 01a8c431 01b31a45 01e00161 0x1fc2320
00d6f550 01e008c1 01e00135 00000024 02894771 0x1fc5ab1
00d6f574 01fbd3fb 00000024 02894771 01a8c431 0x1e008c1
00d6f594 020f1b9e 00000024 01b54555 01f956e9 0x1fbd3fb
00d6f5b4 01e008c1 01bc49bd 00000002 01b88511 0x20f1b9e
00d6f6bc 015c9c3b 01e31a84 01aa4f89 01aca419 0x1e008c1
00d6f6e4 011a2f71 00b7bc50 00000000 00b8baa0 chrome_1000000!v8::internal::Invoke+0xab 
[c:\b\slave\chrome-official\build\src\v8\src\execution.cc @ 88]
00d6f700 015c9d15 0163a464 00b7bc48 00b7bc4c 
chrome_1000000!WebCore::DOMPeerableWrapperMap<WebCore::Node>::get+0xb 
[c:\b\slave\chrome-official\build\src\webkit\port\bindings\v8\v8_proxy.cpp @ 239]
00d6f720 015acc52 00d6f768 00b7bc48 00b7bc4c 
chrome_1000000!v8::internal::Execution::Call+0x25 [c:\b\slave\chrome-
official\build\src\v8\src\execution.cc @ 117]
00d6f754 0119fc46 00d6f778 00b7bc4c 00000001 chrome_1000000!v8::Function::Call+0x92 
[c:\b\slave\chrome-official\build\src\v8\src\api.cc @ 1902]
00d6f770 011ae917 00b452f8 00b7bc48 00b7bc4c 
chrome_1000000!WebCore::V8Proxy::CallFunction+0x2b [c:\b\slave\chrome-
official\build\src\webkit\port\bindings\v8\v8_proxy.cpp @ 1057]
00d6f7a0 011ae6cb 00d6f7e0 04048d30 04195680 
chrome_1000000!WebCore::V8EventListener::CallListenerFunction+0x54 
[c:\b\slave\chrome-official\build\src\webkit\port\bindings\v8\v8_events.cpp @ 201]
00d6f804 010b34be 04195680 00000000 00b27160 
chrome_1000000!WebCore::V8AbstractEventListener::handleEvent+0xf1 [c:\b\slave\chrome-
official\build\src\webkit\port\bindings\v8\v8_events.cpp @ 106]
00d6f820 0108b65d 00000000 04195680 00000000 
chrome_1000000!WebCore::EventTarget::handleLocalEvents+0x56 [c:\b\slave\chrome-
official\build\src\webkit\pending\eventtarget.cpp @ 316]
00d6f830 010b32c7 04195680 00000000 00b26f10 
chrome_1000000!WebCore::EventTargetNode::handleLocalEvents+0x30 [c:\b\slave\chrome-
official\build\src\webkit\pending\eventtargetnode.cpp @ 111]
00d6f884 0108b6c0 00ecc85c 00ecc828 04195680 
chrome_1000000!WebCore::EventTarget::dispatchGenericEvent+0x1d7 [c:\b\slave\chrome-
official\build\src\webkit\pending\eventtarget.cpp @ 228]
00d6f8a4 0108bc74 04195680 00d6f8c4 00000001 
chrome_1000000!WebCore::EventTargetNode::dispatchEvent+0x5e [c:\b\slave\chrome-
official\build\src\webkit\pending\eventtargetnode.cpp @ 127]
00d6f8d8 0108ba44 01802df8 00000000 00000001 
chrome_1000000!WebCore::EventTargetNode::dispatchMouseEvent+0xde [c:\b\slave\chrome-
official\build\src\webkit\pending\eventtargetnode.cpp @ 304]
00d6f938 0105766b 00ecc828 01802df8 00000001 
chrome_1000000!WebCore::EventTargetNode::dispatchMouseEvent+0x7e [c:\b\slave\chrome-
official\build\src\webkit\pending\eventtargetnode.cpp @ 218]
00d6f964 010570d8 00000001 00d6fa00 00000000 
chrome_1000000!WebCore::EventHandler::dispatchMouseEvent+0x34 [c:\b\slave\chrome-
official\build\src\webkit\pending\eventhandler.cpp @ 1300]
00d6f9ec 01024e30 00d6fa00 003ed8a8 06365e68 
chrome_1000000!WebCore::EventHandler::handleMouseReleaseEvent+0x152 
[c:\b\slave\chrome-official\build\src\webkit\pending\eventhandler.cpp @ 1102]
00d6fa34 01025802 06365e68 003ed8a8 003ed8a8 chrome_1000000!WebViewImpl::MouseUp+0x4d 
[c:\b\slave\chrome-official\build\src\webkit\glue\webview_impl.cc @ 262]
00d6fa84 01175d08 06365e68 05a0f3b8 003ed8a8 
chrome_1000000!WebViewImpl::HandleInputEvent+0x8d [c:\b\slave\chrome-
official\build\src\webkit\glue\webview_impl.cc @ 750]
00d6fab4 011756f0 00a0f3b8 05a0f3b8 00000000 
chrome_1000000!RenderWidget::OnHandleInputEvent+0x41 [c:\b\slave\chrome-
official\build\src\chrome\renderer\render_widget.cc @ 309]
00d6fb6c 0116cd96 05a0f3b8 05a0f3b8 00b10a30 
chrome_1000000!RenderWidget::OnMessageReceived+0x135 [c:\b\slave\chrome-
official\build\src\chrome\renderer\render_widget.cc @ 155]
00d6fbbc 01186de6 05a0f3b8 05a0f3b8 00b10a30 
chrome_1000000!RenderView::OnMessageReceived+0x64e [c:\b\slave\chrome-
official\build\src\chrome\renderer\render_view.cc @ 342]
00d6fbcc 01186db9 05a0f3b8 05a0f3b8 00b109f4 
chrome_1000000!MessageRouter::RouteMessage+0x28 [c:\b\slave\chrome-
official\build\src\chrome\common\message_router.cc @ 40]
00d6fbdc 01169d0d 05a0f3b8 00d6feb0 01188b5c 
chrome_1000000!MessageRouter::OnMessageReceived+0x23 [c:\b\slave\chrome-
official\build\src\chrome\common\message_router.cc @ 30]
00d6fc04 01236350 05a0f3b8 003ed3b0 01006086 
chrome_1000000!RenderThread::OnMessageReceived+0xcc [c:\b\slave\chrome-
official\build\src\chrome\renderer\render_thread.cc @ 164]
00d6fc10 01006086 003ed3b0 00000000 003e87a0 
chrome_1000000!RunnableMethod<CancelableRequest<CallbackRunner<Tuple0> >,void 
(__thiscall CancelableRequest<CallbackRunner<Tuple0> >::*)(Tuple0 const 
&),Tuple1<Tuple0> >::Run+0x17 [c:\b\slave\chrome-official\build\src\base\task.h @ 
312]
00d6fcac 010060c0 00d6fce8 00d6feb0 010062c3 chrome_1000000!MessageLoop::RunTask+0x75 
[c:\b\slave\chrome-official\build\src\base\message_loop.cc @ 276]
00d6fcb8 010062c3 003e87a0 003e87b0 00d6feb0 
chrome_1000000!MessageLoop::DeferOrRunPendingTask+0x29 [c:\b\slave\chrome-
official\build\src\base\message_loop.cc @ 286]
00d6fce8 0100d33c 00d6feb0 00b109fc 00d6feb0 chrome_1000000!MessageLoop::DoWork+0x7b 
[c:\b\slave\chrome-official\build\src\base\message_loop.cc @ 375]
00d6fd9c 01005ded 00d6feb0 00d6feb0 00b109fc 
chrome_1000000!base::MessagePumpDefault::Run+0x119 [c:\b\slave\chrome-
official\build\src\base\message_pump_default.cc @ 47]
00d6fe40 01005d5c b4752630 00d6feb0 00b109fc 
chrome_1000000!MessageLoop::RunInternal+0x8b [c:\b\slave\chrome-
official\build\src\base\message_loop.cc @ 165]
00d6fe78 01005cff 00d6fa10 00000001 7c80a000 
chrome_1000000!MessageLoop::RunHandler+0x4f [c:\b\slave\chrome-
official\build\src\base\message_loop.cc @ 148]
00d6fe98 01475d64 00000000 017525ae 806e4427 chrome_1000000!MessageLoop::Run+0x15 
[c:\b\slave\chrome-official\build\src\base\message_loop.cc @ 122]
00d6ff70 01017de9 0158ae7c 00b109fc b47527e4 
chrome_1000000!base::Thread::ThreadMain+0x7e [c:\b\slave\chrome-
official\build\src\base\thread.cc @ 163]
00d6ff74 0158ae7c 00b109fc b47527e4 00000000 chrome_1000000!`anonymous 
namespace'::ThreadFunc+0x9 [c:\b\slave\chrome-
official\build\src\base\platform_thread_win.cc @ 29]
00d6ffac 0158af21 017525ae 7c80b683 00b10ad0 chrome_1000000!_callthreadstartex+0x1b 
[f:\sp\vctools\crt_bld\self_x86\crt\src\threadex.c @ 348]
00d6ffb4 7c80b683 00b10ad0 00000000 017525ae chrome_1000000!_threadstartex+0x7f 
[f:\sp\vctools\crt_bld\self_x86\crt\src\threadex.c @ 326]
00d6ffec 00000000 0158aea2 00b10ad0 00000000 kernel32!BaseThreadStart+0x37


STACK_COMMAND:  ~1s; .ecxr ; kb

FOLLOWUP_IP: 
chrome_1000000!logging::LogMessage::~LogMessage+25f [c:\b\slave\chrome-
official\build\src\base\logging.cc @ 497]
0100c414 cc              int     3

SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  chrome_1000000!logging::LogMessage::~LogMessage+25f

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: chrome_1000000

IMAGE_NAME:  chrome.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  48c77043

FAILURE_BUCKET_ID:  
STATUS_BREAKPOINT_80000003_chrome.dll!logging::LogMessage::~LogMessage

BUCKET_ID:  
APPLICATION_FAULT_STATUS_BREAKPOINT_chrome_1000000!logging::LogMessage::_LogMessage+2
5f

Followup: MachineOwner
---------


Comment 6 by mal.chromium, Sep 11, 2008
Marc-Antoine, please have a look at this. It may be a GDI leak.
Owner: mar...@chromium.org
Comment 7 by nsylvain@chromium.org, Sep 11, 2008
CreateDIBSection fails because it runs out of memory.

It runs out of memory because the call above (CreateBitmapHeader) is creating a 
bitmap of 13224 x 9864 pixels.

This is coming the width()/height() of HTMLCanvasElement.
Comment 8 by nsylvain@chromium.org, Sep 11, 2008
This is coming directly from javascript, from heightAttrSetter.

Testing old build to see where the regression comes from seems like our best bet.
Comment 9 by nsylvain@chromium.org, Sep 11, 2008
I can replicate with 0.2.149.29 (1798). This is not a regression.

Comment 10 by nsylvain@chromium.org, Sep 11, 2008
Adding back ojan in cc because he might have a better idea.
Cc: o...@chromium.org
Comment 11 by maruelatchromium, Sep 12, 2008
This is definitely a bug in Chromium. Safari 3.1.2 Win does show a large increase of
memory usage when zooming, but in Chromium it's exponential. At the level just before
crashing, the renderer is using ~480 megs of RAM. So I don't know if it is because we
create too many internal bitmaps or if it is on the javascript side. It is
reproducible with JSC so that rules out being a v8 bug.
Comment 12 by ojan@chromium.org, Sep 12, 2008
looks to be introduced in this revision range 1618-1625
Comment 13 by ojan@chromium.org, Sep 12, 2008
It was the change that made our UA change Version/... to Chrome/...

Tried it in testshell on tip of tree with both UAs. Only the Chrome/... crashed.
Comment 14 by mal.chromium, Sep 12, 2008
Wow. Who knew the user agent string could cause OOM?

I'm trying to get in touch with Yahoo.
Owner: m...@chromium.org
Labels: -dev-release-block
Comment 15 by brianrakowski, Sep 12, 2008
I'm trying to reach Yahoo too but it seems like we shouldn't crash no matter what 
Yahoo sends us.
Comment 16 by jon@chromium.org, Feb 13, 2009
This no longer appears to happen in 2.0.163.0 (Developer Build 9751) so I am closing 
this as Fixed.
Status: Fixed
Owner: j...@chromium.org
Cc: m...@chromium.org
Comment 17 by sunandt@chromium.org, Mar 26, 2009
Jon, 
I can still see renderer crash in Chrome2.0.169.1. This has something to do with UA 
string. If FF3 UA string is used, it doesn't crash.

Here's the link to full stack trace
http://go/crash/reportdetail?
email=&clientid=&reportid=1c00009c549c4981&product=Chrome&version=&signature=&date=
Status: Untriaged
Cc: anan...@chromium.org
Labels: Channel-Beta
Comment 18 by jon@chromium.org, Apr 03, 2009
I zoomed in and out several times without a crash.  2.0.173.0 (Developer Build 13084)
Status: Fixed
Comment 19 by sunandt@chromium.org, Apr 03, 2009
I can still see the crash with 2.0.173.0 (Developer Build 13084).

Steps
-----
1. Go to maps.yahoo.com
2. Get the directions from one place to another
3. Now Zoom in to Street level by dragging the bar to the end towards the [+] sign

Crash!!
Status: Assigned
Comment 20 by mal.chromium, Apr 03, 2009
Looks like we are getting something wrong here:

    m_imageBuffer->context()->scale(FloatSize(size.width() / unscaledSize.width(), 
size.height() / unscaledSize.height()));

in this function

void HTMLCanvasElement::createImageBuffer() const
{
    ASSERT(!m_imageBuffer);

    m_createdImageBuffer = true;
    
    FloatSize unscaledSize(width(), height());
    IntSize size = convertLogicalToDevice(unscaledSize);
    if (!size.width() || !size.height())
        return;

    m_imageBuffer.set(ImageBuffer::create(size, false).release());
    m_imageBuffer->context()->scale(FloatSize(size.width() / unscaledSize.width(), 
size.height() / unscaledSize.height()));
    m_imageBuffer->context()->setShadowsIgnoreTransforms(true);
}

Owner: esei...@chromium.org
Labels: -Area-Compat -v-152.0 Area-WebKit Mstone-2.0
Comment 21 by esei...@chromium.org, Apr 06, 2009
Well, void HTMLCanvasElement::createImageBuffer() const has an obvious bug, in that
ImageBuffer::create() can return NULL, and it doesn't check for that.  But that
doesn't seem to be the bug we're hitting here.  (See WebKit  bug 23212  for discussion.)

I'll fix the obvious null return in WebKit (by reviewing and landing Mike's patch
https://bugs.webkit.org/show_bug.cgi?id=23212), but this Skia/Chromium crasher
someone else who works in the Chromium/Skia tree more regularly should go after.
Owner: m...@chromium.org
Comment 22 by esei...@chromium.org, Apr 06, 2009
Landed Mike Belshe's patch to fix the obvious WebKit bug in
http://trac.webkit.org/changeset/42236.  Not sure if that will fix the Chromium
crasher or not though.
Comment 23 by mal.chromium, Apr 06, 2009
(No comment was entered for this change.)
Owner: bre...@chromium.org
Comment 24 by mal.chromium, Apr 08, 2009
Stephen, are you interested in looking into this bug? 
Owner: senorbla...@chromium.org
Comment 25 by senorblanco@chromium.org, Apr 09, 2009
I'll take a look.  Looks like it has quite a pedigree..  :)
Comment 26 by senorblanco@chromium.org, Apr 15, 2009
See attached for reduced test case.  This crash seems to happen with any canvas of 
large size, say 15000x15000.  Oddly, we handle it more gracefully if you make it 
absurdly large (say, 100Kx100K), presumably because the malloc fails earlier and is 
caught.
canvas.html
521 bytes Download
Comment 27 by senorblanco@chromium.org, Apr 16, 2009
Here's what I've discovered so far:  if the canvas size is 15Kx15K, it meets 
HTMLCanvasElement's "MaxCanvasArea" criterion (32Kx8K), and the allocation of the 
ImageBufferSkia succeeds.  However, the actual malloc() is deferred until later (in 
ImageBufferSkia::image()), and if there's insufficient memory, the malloc() fails at 
draw time and blows up with an INT3, which kills the browser.

We could lower MaxCanvasArea, but there's still no guarantee that the malloc() will 
succeed on any given machine (and if we do a 64-bit port, would be an artificial 
limit).

There's no limit specified in the HTML5 spec for the width and height of the canvas 
tag.

In Safari, the allocation is done right away, and failure is gracefully handled (ie., 
you get a white canvas, and drawing on it does nothing).  I'm pretty sure that 
m_imageBuffer is NULL in this case.

Another possibility would be to force the ImageBuffer to do the malloc() right away, 
so we can check for failure the way Safari does.  This might require calling 
_set_new_handler temporarily, to prevent the INT3.
Comment 28 by brettw@chromium.org, Apr 20, 2009
It seems like we should allocate at the same time as Safari, and handle when it fails. 
I don't know about the int3, I'm surprised it's happening in release modes. Did we add 
that?
Comment 29 by senorblanco@chromium.org, Apr 20, 2009
In chrome_dll_main.cc:

int OnNoMemory(size_t memory_size) {
  __debugbreak();

  [...]
}

It's set as the new handler in RegisterInvalidParamHandler():

// Register the invalid param handler and pure call handler to be able to
// notify breakpad when it happens.
void RegisterInvalidParamHandler() {
#if defined(OS_WIN)
  _set_invalid_parameter_handler(InvalidParameter);
  _set_purecall_handler(PureCall);
  // Gather allocation failure.
  _set_new_handler(&OnNoMemory);
  // Make sure malloc() calls the new handler too.
  _set_new_mode(1);
#endif
}

This means it doesn't happen in the test_shell, since it doesn't set this handler, 
AFAICT.
Comment 30 by cpu@chromium.org, Apr 22, 2009
That __debugbreak() was added as a security feature. Its saves our bacon because we 
don't check for allocs not to be zero in all places.

Failure to crash can be leveraged into an attack. Specially if the alloc is meant to 
be used as an array. In fact a very powerful vector because it allows you to modify 
memory at arbitrary locations without having to trample over the previous memory thus 
defeating stack canaries.



Comment 31 by senorblanco@chromium.org, Apr 22, 2009
Fair enough -- like I said on the review thread, I'm an idiot with security.

I'm just not sure how to emulate Safari and Firefox behaviour without (at least 
temporarily) disabling the __debugbreak.
Comment 32 by cpu@chromium.org, Apr 22, 2009
I understand the desire of not crash. I am afraid the bug needs a _complicated_ fix at 
least from my POV.

That Safari does not crash gives me a bad feeling.

Adding Eric to the bug.

Cc: esei...@chromium.org
Comment 33 by r...@android.com, Apr 24, 2009
on a different tact:

Can we/webkit realize that the canvas size is larger than the screen, and create a clipped canvas buffer that is 
only as large as the screen (or some other reasonable size), rather than trying to allocated/erase/draw 
something that is so big? If doable, this should both address the OOM, and would improve performance.
Comment 34 by r...@android.com, Apr 24, 2009
(No comment was entered for this change.)
Cc: r...@android.com
Comment 35 by senorblanco@chromium.org, Apr 24, 2009
I don't think so, since the canvas can be scrolled.
Comment 36 by r...@android.com, Apr 24, 2009
That does complicate matters :) In that case, we'd have to recognize that, and inval-redraw the newly visible part. 
Just a thought.
Comment 37 by mal.chromium, Apr 28, 2009
Any updates on this bug? I'd really like to get it fixed for 2.0, which means a fix 
ASAP.
Comment 38 by mal.chromium, Apr 28, 2009
senorblanco emailed: "I have an idea for a fix, which seems to have reviewer 
approval, but I haven't tried it yet.  I became unblocked on one of my other tasks 
(moving skia to DEPS), so I switched to that.

One problem is, the fix requires a change to skia, and I'm in the middle of switching 
to use the upstream skia, so I had hoped to defer it until after that."



Is it possible to make the skia changes on the current code? I can't take a new drop 
of skia on the 172 branch for 2.0 at this point.
Comment 39 by senorblanco@chromium.org, Apr 28, 2009
Understood about the new drop.  Yes, we can make the changes in our local copy of 
skia, and I'll upstream it.

For reference, here's the plan:  when skia calls sk_malloc_flags() with SK_THROW
*not* set, the caller is indicating that it will robustly handle NULL returns (which
happens to be the case in SkBitmap::HeapAllocator::allocPixelRef(), where we're 
having our little problem).  So we'll overload this mechanism, and put in a callback
or global that disables our new_handler during those allocations, and re-enables it 
after.

Comment 40 by bugdroid1@chromium.org, Apr 29, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=14891 

------------------------------------------------------------------------
r14891 | senorblanco@chromium.org | 2009-04-29 14:31:13 -0700 (Wed, 29 Apr 2009) | 18 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/DEPS?r1=14891&r2=14890
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/chrome_dll_main.cc?r1=14891&r2=14890
   M http://src.chromium.org/viewvc/chrome/trunk/src/skia/corecg/SkMemory_stdlib.cpp?r1=14891&r2=14890
   M http://src.chromium.org/viewvc/chrome/trunk/src/skia/include/corecg/SkTypes.h?r1=14891&r2=14890

A better fix for http://www.crbug.com/2044:  crash
on large <canvas> elements.  We disable the __debugbreak
only when skia tells us it is prepared to correctly 
handle a failed (NULL) malloc().  It does this
by calling sk_malloc_flags() without SK_MALLOC_THROW.

Note that, since the switch to tcmalloc, the new_handler
was not getting called at all (since tcmalloc doesn't 
support it yet), so this crash is currently unreproducible
in trunk.  In order to test this change, I reverted the 
tcmalloc change in my client.  This is not the case in the 
stable branch, since it doesn't use tcmalloc, so this change 
is still needed there.  (It will also be needed in trunk 
again once mbelshe's re-implementation of the new_handler
is in).

BUG=http://www.crbug.com/2044
Review URL: http://codereview.chromium.org/100163
------------------------------------------------------------------------

Comment 41 by bugdroid1@chromium.org, Apr 29, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=14895 

------------------------------------------------------------------------
r14895 | laforge@chromium.org | 2009-04-29 14:51:23 -0700 (Wed, 29 Apr 2009) | 21 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/172/src/chrome/app/DEPS?r1=14895&r2=14894
   M http://src.chromium.org/viewvc/chrome/branches/172/src/chrome/app/chrome_dll_main.cc?r1=14895&r2=14894
   M http://src.chromium.org/viewvc/chrome/branches/172/src/skia/corecg/SkMemory_stdlib.cpp?r1=14895&r2=14894
   M http://src.chromium.org/viewvc/chrome/branches/172/src/skia/include/corecg/SkTypes.h?r1=14895&r2=14894

Merge 14891 - A better fix for http://www.crbug.com/2044:  crash
on large <canvas> elements.  We disable the __debugbreak
only when skia tells us it is prepared to correctly 
handle a failed (NULL) malloc().  It does this
by calling sk_malloc_flags() without SK_MALLOC_THROW.

Note that, since the switch to tcmalloc, the new_handler
was not getting called at all (since tcmalloc doesn't 
support it yet), so this crash is currently unreproducible
in trunk.  In order to test this change, I reverted the 
tcmalloc change in my client.  This is not the case in the 
stable branch, since it doesn't use tcmalloc, so this change 
is still needed there.  (It will also be needed in trunk 
again once mbelshe's reimplementation of the new_handler
is in).

BUG=http://www.crbug.com/2044
Review URL: http://codereview.chromium.org/100163

TBR=senorblanco@chromium.org
Review URL: http://codereview.chromium.org/100179
------------------------------------------------------------------------

Comment 42 by dglazkov@chromium.org, Apr 29, 2009
Is this fixed?
Comment 43 by mal.chromium, Apr 29, 2009
It is fixed on the 172 branch, but I'm not sure if senorblanco wants to make a 'better' 
fix once we're pulling a new version of Skia.

I'll push this off the 2.0 list for now, but close it if there's no more work to do 
here.
Labels: -Pri-1 -Mstone-2.0 Pri-2 Mstone-2.1
Comment 44 by senorblanco@chromium.org, Apr 30, 2009
No, there is no better fix coming.  The skia change was applied locally, and is pending
upstream as http://codereview.appspot.com/52066.

I'll mark this as Fixed.
Status: Fixed
Comment 45 by senorblanco@chromium.org, Jun 08, 2009
(No comment was entered for this change.)
Owner: anan...@chromium.org
Cc: -anan...@chromium.org senorbla...@chromium.org
Sign in to add a comment