Closed Bug 967330 Opened 6 years ago Closed 6 years ago

Faulty: assert failure "updated region lies across rotation boundaries!" in ContentHostSingleBuffered::UpdateThebes


(Core :: Graphics, defect)

Not set





(Reporter: bjacob, Assigned: bjacob)


(Blocks 1 open bug)



(2 files)

Found by Christoph Diehl's "Faulty" fuzzer, see bug 777067
Argh, forgot to attach session. But this easy to reproduce, happens all the time. Hold on...
Attached file Faulty session
Classification: PLayerTransactionParent protocol, bad assertion on message parameters, easy
UpdateThebes needs to do graceful runtime error handling, because the region it receives can be an arbitrarily bad one from an untrusted client.
Attachment #8373799 - Flags: review?(nical.bugzilla)
Comment on attachment 8373799 [details] [diff] [review]
UpdateThebes full of grace

Review of attachment 8373799 [details] [diff] [review]:

::: gfx/layers/composite/ContentHost.cpp
@@ +520,5 @@
>    if (!mTextureHost) {
>      mInitialised = false;
> +    return true; // FIXME should we return false? Returning true for now
> +  }              // to preserve existing behavior of NOT causing IPC errors.

It could be worth landing as is first and try to return false in a separate bug to see what we get. This is definitely something that we don't want to wind up hitting, but I suspect that sometimes it's just very hard to avoid it. For instance if suddenly we need to use a different Compositor/GLContext, we tend to get rid of textures and may miss a frame (which is probably ok when loosing a context). So far the strategy we seem to have followed on the compositor thread is to avoid crashing at all costs, even if it means breaking correctness for a frame. Also, for layer that may be affected by ImageBridge (so not in this case), it is very hard to ensure that your layer has a texture, because the first frame may arrive before or after the layer (textures almost always arrive before layers because the content thread is a busy guy). So when async video is involved, returning early may be a valid thing to do.
Attachment #8373799 - Flags: review?(nical.bugzilla) → review+
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.