1153724 - An assert from the graphics logger - Failed 2 buffer db=0x0 dw=0x0 for 0, 0, 16283868, 1056

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jesse Ruderman]

Attachment: testcase

With:
  user_pref("layers.acceleration.disabled", true);
  user_pref("layers.force-active", true);

> [GFX3-]: Surface size too large (would overflow)!
> [GFX3-]: Surface size too large (exceeds caller's limit)!
> [GFX1-]: Failed to allocate a surface due to invalid size Size(16283868,1056)
> [GFX1]: Failed 2 buffer db=0x0 dw=0x0 for 0, 0, 16283868, 1056
> Assertion failure: false (An assert from the graphics logger), at gfx/2d/Logging.h:487

Bug 1101685 added the ability for graphics logging to assert.

Variations on the testcase cause the entire window to paint white (including browser controls) rather than asserting.

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Could be related or a duplicate of bug 1151644.  Any chance you can get a regression range, perhaps aiming around April 1st as a potential day a patch that broke it landed?

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

Never mind the previous comment, I looked closer at this, we should just not assert when the size request is "unreasonable" as we do in other places.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Only assert on reasonable sizes. r=mchang

This should do it, but I can't get past the white screen with these options until bug 1151644 is sorted out.

Comment 5

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8591902 [details] [diff] [review]
Only assert on reasonable sizes. r=mchang

Review of attachment 8591902 [details] [diff] [review]:
-----------------------------------------------------------------

Now that bug 1151644 landed, can we get past the white screen?

::: gfx/layers/RotatedBuffer.cpp
@@ +753,5 @@
>    if (aPaintState.mMode == SurfaceMode::SURFACE_COMPONENT_ALPHA) {
>      if (!mDTBuffer || !mDTBufferOnWhite) {
>        // This can happen in release builds if allocating one of the two buffers
> +      // failed. This in turn can happen if unreasonably large textures are
> +      // requested.

Can we assert that we got an unreasonably large size here then?

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Mason Chang [:mchang] from comment #5)
> Comment on attachment 8591902 [details] [diff] [review]
> Only assert on reasonable sizes. r=mchang
> 
> Review of attachment 8591902 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Now that bug 1151644 landed, can we get past the white screen?

Yes, I should have been explicit here (rather than just in bug 1151644) that I can get past the white screen and actually test this fix.

> 
> ::: gfx/layers/RotatedBuffer.cpp
> @@ +753,5 @@
> >    if (aPaintState.mMode == SurfaceMode::SURFACE_COMPONENT_ALPHA) {
> >      if (!mDTBuffer || !mDTBufferOnWhite) {
> >        // This can happen in release builds if allocating one of the two buffers
> > +      // failed. This in turn can happen if unreasonably large textures are
> > +      // requested.
> 
> Can we assert that we got an unreasonably large size here then?

Don't think we can get this far without having gone through the code above to get the buffers.  If the size was reasonable, we wouldn't get this far, because we would have crashed (in debug) above; if the size wasn't reasonable, then we don't want to crash anyway, so I think we're covered.

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

Let me add a test case for this before landing.

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a1c40541de0

The automated test is going to wait a bit, blocked on the similar issue as bug 971126.  When specified in crashtests, the preferences get set "too late" for the "Once" gfxPrefs, which means that we never pick them up.  So, we'd have a test that doesn't fail even without the bugfix.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/78ad86e96087

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/78ad86e96087

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

Will land the crashtest in bug 1156896.