Closed Bug 1153724 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Graphics, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: jruderman, Assigned: milan)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: gfx-noted)

Attachments

(3 files)

Attached file 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.
Attached file stack
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?
Whiteboard: gfx-noted
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.
Assignee: nobody → milan
This should do it, but I can't get past the white screen with these options until bug 1151644 is sorted out.
Attachment #8591902 - Attachment description: Only assert on reasonable sizes. → Only assert on reasonable sizes. r=mchang
Attachment #8591902 - Flags: review?(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?
Attachment #8591902 - Flags: review?(mchang) → review+
(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.
Let me add a test case for this before landing.
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.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/78ad86e96087
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Will land the crashtest in bug 1156896.
You need to log in before you can comment on or make changes to this bug.