Bug 58818 – [GTK] fast/block/float/overhanging-tall-block.html crashes in the bots

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alejandro G. Castro]

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.

[Alejandro G. Castro]

Skipped http://trac.webkit.org/changeset/84175

[Simon Fraser (smfr)]

Why would this just crash GTK? Got a crash log?

[Alejandro G. Castro]

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

[Martin Robinson]

Created attachment 90219 [details]
Patch

[Alejandro G. Castro]

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?

[Martin Robinson]

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

[Martin Robinson]

Created attachment 90385 [details]
Patch fixing typo in the ChangeLog

[Eric Seidel (no email)]

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

[Martin Robinson]

Committed r84978: <http://trac.webkit.org/changeset/84978>

[WebKit Review Bot]

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