967528 - Assert failure "component alpha textures should be the same size" in ContentHostBase::Composite on www.lemonde.fr

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Faulty session

Found by Christoph Diehl's "Faulty" fuzzer, see bug 777067

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Wait... I seem to be getting this crash all the time now, even without Faulty.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

This is caused by the patch in bug 962041: reverting it removes the crash.

Comment 3

[Nicolas Silva [:nical]]

I haven't been able to reproduce so far, even on lemonde.fr. We should check in ContentClient that:
* when we call UseTexture on mTextureClientOnWhite, mTextureClient has the same size
* we are not switching between component alpha and non-component alpha for a given CompositableClient (otherwise we could have the next frame only update the TextureClient and the host would think it has component alpha because it still has the old texture on white).
That's all i cna think of so far.

Comment 5

[Alexandre Poirot [:ochameau]]

This assertion prevents b2g desktop to be run in debug mode. (at least on linux)

Step to reproduce:

* Build b2g desktop with debug mode on:

  ac_add_options --enable-application=b2g
  ac_add_options --enable-debug

* Make a gaia profile:

  cd ~/gaia/
  rm -rf profile
  NOFTU=1 make

* Run b2g desktop with the gaia profile:

  ./objdir/dist/bin/b2g-bin -profile ~/gaia/profile

* Open an app, BAM. Parent process crashes with:

  Assertion failure: iterOnWhite->GetTileRect() == tileIter->GetTileRect() (component alpha textures
  should be the same size.), at /mnt/desktop/gecko/gfx/layers/composite/ContentHost.cpp:143

Comment 6

[Nicolas Silva [:nical]]

Attachment: Use a specific message for component alpha

This patch will fix the bug if the cause was that we used the same compositable both with and without component alpha. I don't know if that can happen but we don't have code to guard against it so it doesn't hurt to be handle it without crashing. Even if it doesn't fix this issue, it's still good to take because it makes us more robust against ipdl messages arriving in the wrong order.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 8370873 [details] [diff] [review]
Use a specific message for component alpha

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

How about having component-alpha code reviewed by someone who knows component-alpha instead ;-)

Comment 8

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8370873 [details] [diff] [review]
Use a specific message for component alpha

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

::: gfx/layers/composite/ContentHost.cpp
@@ +221,5 @@
>  ContentHostBase::UseTextureHost(TextureHost* aTexture)
>  {
> +  mTextureHost = aTexture;
> +  mTextureHostOnWhite = nullptr;
> +  mTextureHost->SetCompositor(GetCompositor());

Don't we want to call into the base class here too? Nothing is calling SetCompositableBackendSpecificData

Comment 9

[Nicolas Silva [:nical]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #8)
> Comment on attachment 8370873 [details] [diff] [review]
> Use a specific message for component alpha
> 
> Review of attachment 8370873 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/composite/ContentHost.cpp
> @@ +221,5 @@
> >  ContentHostBase::UseTextureHost(TextureHost* aTexture)
> >  {
> > +  mTextureHost = aTexture;
> > +  mTextureHostOnWhite = nullptr;
> > +  mTextureHost->SetCompositor(GetCompositor());
> 
> Don't we want to call into the base class here too? Nothing is calling
> SetCompositableBackendSpecificData

I guess so, It wasn't there before the patch so I tossed the idea aside to do it in a separate bug. I can add it to this patch.

Comment 10

[Alexandre Poirot [:ochameau]]

\o/ Thanks for fixing that so promply. I confirm that it stops asserting on b2g.

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

Now it's my turn: I can't reproduce the bug anymore (without the patch), so I'm not able to test at the moment. Sorry!

Comment 12

[Nicolas Silva [:nical]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d7c0ce1b376

Comment 13

[Ryan VanderMeulen [:RyanVM]]

This landed with the wrong bug # in the commit message. Please double-check before pushing next time.
https://hg.mozilla.org/mozilla-central/rev/8d7c0ce1b376