Bug 58818 - [GTK] fast/block/float/overhanging-tall-block.html crashes in the bots
Summary: [GTK] fast/block/float/overhanging-tall-block.html crashes in the bots
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-18 13:25 PDT by Alejandro G. Castro
Modified: 2011-04-26 17:23 PDT (History)
7 users (show)

See Also:


Attachments
Patch (14.69 KB, patch)
2011-04-19 11:09 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch fixing typo in the ChangeLog (14.61 KB, patch)
2011-04-20 12:22 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alejandro G. Castro 2011-04-18 13:25:16 PDT
This is the error we get in the logs:

The program 'DumpRenderTree' received an X Window System error.
This probably reflects a bug in the program.
The error was 'BadAlloc (insufficient resources for operation)'.
  (Details: serial 102741 error_code 11 request_code 53 minor_code 0)
  (Note to programmers: normally, X errors are reported asynchronously;
   that is, you will receive the error a while after causing it.
   To debug your program, run it with the --sync command line
   option to change this behavior. You can then get a meaningful
   backtrace from your debugger if you break on the gdk_x_error() function.)
LEAK: 2494 WebCoreNode
LEAK: 1 Range
LEAK: 73 CachedResource
LEAK: 9 Frame
LEAK: 9 Page
LEAK: 1160 RenderObject

Simon, any clue about what could cause the problem? I'm going to skip the test for the moment.
Comment 1 Alejandro G. Castro 2011-04-18 13:29:52 PDT
Skipped http://trac.webkit.org/changeset/84175
Comment 2 Simon Fraser (smfr) 2011-04-18 13:47:08 PDT
Why would this just crash GTK? Got a crash log?
Comment 3 Alejandro G. Castro 2011-04-19 01:32:30 PDT
(In reply to comment #2)
> Why would this just crash GTK? Got a crash log?

No idea, it is really weird, amazari is checking the problem, unfortunately for some reason the crash does not produce the crash log, and it can not be reproduced locally. We are going to try to run the test in the bot manually to see what we get.
Comment 4 Martin Robinson 2011-04-19 11:09:05 PDT
Created attachment 90219 [details]
Patch
Comment 5 Alejandro G. Castro 2011-04-20 05:34:02 PDT
Comment on attachment 90219 [details]
Patch


Looks goods! Just a couple of comments.

View in context: https://bugs.webkit.org/attachment.cgi?id=90219&action=review

> Source/WebCore/ChangeLog:8
> +        Prevent allocating scratch buffers larger than the target GdkDarawable

GdkDrawable

> Source/WebCore/platform/gtk/WidgetRenderingContext.cpp:93
> +    // We never want to create a scratch buffer larger than the size of our target GdkDrawable.
> +    // This prevents giant pixmap allocations for very large widgets in smaller views.
> +    int maxWidth = 0, maxHeight = 0;
> +    gdk_drawable_get_size(graphicsContext->gdkWindow(), &maxWidth, &maxHeight);
> +    m_targetRect.setSize(m_targetRect.size().shrunkTo(IntSize(maxWidth, maxHeight)));
> +
>      // Widgets sometimes need to draw outside their boundaries for things such as
>      // exterior focus. We want to allocate a some extra pixels in our surface for this.
> -    m_extraSpace = IntSize(15, 15);
> +    static int extraSpace = 15;
> +    m_targetRect.inflate(extraSpace);

Should we inflate first and later shrunkTo?
Comment 6 Martin Robinson 2011-04-20 12:13:44 PDT
(In reply to comment #5)
> (From update of attachment 90219 [details])
> 
> Looks goods! Just a couple of comments.
> 
> View in context: https://bugs.webkit.org/attachment.cgi?id=90219&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Prevent allocating scratch buffers larger than the target GdkDarawable
> 
> GdkDrawable

Fixed!

> Should we inflate first and later shrunkTo?

inflate() modifies the offset of the target rectangle. If we inflate first and then shrink, there would have to be more code which detected if the offset needs to be corrected after shrunkTo. It seemed simpler to inflate later, since there isn't much harm in having a buffer that is 30 pixels bigger in each direction. We're really just trying to prevent having buffer that is much bigger than the target.
Comment 7 Martin Robinson 2011-04-20 12:22:43 PDT
Created attachment 90385 [details]
Patch fixing typo in the ChangeLog
Comment 8 Eric Seidel (no email) 2011-04-26 16:16:38 PDT
Comment on attachment 90385 [details]
Patch fixing typo in the ChangeLog

Seems reasonable to me.  if you need a more gtk-ish review, you'll have to ask someone else though. :)
Comment 9 Martin Robinson 2011-04-26 16:32:12 PDT
Committed r84978: <http://trac.webkit.org/changeset/84978>
Comment 10 WebKit Review Bot 2011-04-26 17:23:47 PDT
http://trac.webkit.org/changeset/84978 might have broken SnowLeopard Intel Release (Tests)
The following tests are not passing:
accessibility/anchor-linked-anonymous-block-crash.html
accessibility/aria-activedescendant-crash.html
accessibility/aria-checkbox-checked.html
accessibility/aria-checkbox-text.html
accessibility/aria-combobox.html
accessibility/aria-controls-with-tabs.html
accessibility/aria-describedby-on-input.html
accessibility/aria-disabled.html
accessibility/aria-help.html
accessibility/aria-hidden-update.html
accessibility/aria-hidden-with-elements.html
accessibility/aria-hidden.html
accessibility/aria-label.html
accessibility/aria-labelledby-on-input.html
accessibility/aria-labelledby-overrides-label.html
accessibility/aria-labelledby-stay-within.html
accessibility/aria-link-supports-press.html
accessibility/aria-list-and-listitem.html
accessibility/aria-option-role.html
accessibility/aria-presentational-role.html