914445 - "Assertion failure: mDeprecatedTextureClient (Failed to create texture client)"

Status: RESOLVED WORKSFORME

(Core :: Graphics: Layers, defect)

Description

[Jesse Ruderman]

Attachment: testcase

With:
  user_pref("layers.use-deprecated-textures", true);

Assertion failure: mDeprecatedTextureClient (Failed to create texture client), at ../../../gfx/layers/client/CanvasClient.cpp:144

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

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

Nical, thoughts?

Comment 3

[Nicolas Silva [:nical]]

{width = 21663, height = 27972}

First thought: That's a very big texture.
I wouldn't be surprised we just failed to allocate it. We need to pay attention to those assertions following allocation calls and make sure we provide with a fallback code path that maybe doesn't display the layer, but at least doesn't crash the browser.

I will give a closer look tomorrow.

Comment 4

[Nicolas Silva [:nical]]

needinfo'ing me again otherwise i'll forget

Comment 5

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

Now that you mention it - isn't CreateDeprecatedTextureClient(TEXTURE_FALLBACK) always going to return nullptr on OS X, so this assert will always trigger on anything but windows.

Comment 6

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

Attachment: WIP - doesn't fix the problem

This doesn't actually fix the crash for me, but it does allow for the Create... returning nullptr.

Comment 7

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

(In reply to Milan Sreckovic [:milan] from comment #6)
> Created attachment 802479 [details] [diff] [review]
> WIP - doesn't fix the problem
> 
> This doesn't actually fix the crash for me, but it does allow for the
> Create... returning nullptr.

As in - it crashes elsewhere now.

Comment 8

[Nicolas Silva [:nical]]

How much priority does this bug have? I don't think it is very hard to fix. Basically we need to go through all the places where we allocate or request memory, and make sure we have a fallback path where either we pick a different allocation strategy or we just return without rendering the element that is too big to be allocated (always better than crashing).

With all the B2G craze these days I don't know if I should work on this now.

Comment 9

[Nick Cameron [:nrc]]

(In reply to Nicolas Silva [:nical] from comment #8)
> How much priority does this bug have? I don't think it is very hard to fix.
> Basically we need to go through all the places where we allocate or request
> memory, and make sure we have a fallback path where either we pick a
> different allocation strategy or we just return without rendering the
> element that is too big to be allocated (always better than crashing).
> 
> With all the B2G craze these days I don't know if I should work on this now.

Should be even easier than that - you probably just need to return a shmem or memory image texture client from CreateDeprecatedTextureClient for the TEXTURE_FALLBACK case.

Comment 10

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

No more deprecated textures, this seems to work fine.  Jesse, please reopen if you see it, but that pref is now obsolete and doesn't do anything.