1296446 - Crash in mozilla::ipc::FatalError | mozilla::gfx::PVRLayerParent::OnMessageReceived

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Andrew McCreight [:mccr8]]

This bug was filed from the Socorro interface and is 
report bp-7cf74ab4-6451-4fb0-818b-6e03b2160818.
=============================================================

I see 13 of these crashes on Nightly, which isn't a ton, but given that I don't imagine there are so many people using VR, I figured Kip might be interested.

The "graphics critical error" field for almost all of these is "Attempt to remove a texture from a CompositableForwarder." which was added in bug 1276811. The IPC fatal error message for all of them is "Error deserializing 'PTextureParent'".

Comment 1

[:kip (Kearwood Gilbert)]

Thanks, I will take this bug and investigate.  I have managed to reproduce this after hearing reports from WebVR developers in our WebVR Slack channel.

Bug 1295951 has landed in inbound, which should help with some crashes, but is not confirmed to be the cause of all of these.

Comment 2

[:kip (Kearwood Gilbert)]

Entering VR mode on the "monoscopic rendering" WebVR demo consistently crashes:

https://toji.github.io/webvr-samples/XX-monoscopic-rendering.html

This may be the same issue or have the same root.

This is now my front-burner task.  I'll investigate further.

Comment 3

[:kip (Kearwood Gilbert)]

Attachment: Bug 1296446 - Do not render VR frames until a WebGLContext has returned a mirroring layer

- Fixes a crash that occurred when WebVR frames were rendered prior to
  setting up the WebGLContext for mirroring back to the 2d display.

Review commit: https://reviewboard.mozilla.org/r/77346/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/77346/

Comment 4

[:kip (Kearwood Gilbert)]

The attached patch may also fix Bug 1300972.

Comment 5

[:kip (Kearwood Gilbert)]

Comment on attachment 8789110 [details]
Bug 1296446 - Do not render VR frames until a WebGLContext has returned a mirroring layer

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/77346/diff/1-2/

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8789110 [details]
Bug 1296446 - Do not render VR frames until a WebGLContext has returned a mirroring layer

https://reviewboard.mozilla.org/r/77346/#review76014

::: dom/canvas/WebGLContext.cpp:134
(Diff revision 2)
>      mGeneration = 0;
>      mInvalidated = false;
>      mCapturedFrameInvalidated = false;
>      mShouldPresent = true;
>      mResetLayer = true;
> +    mLayerIsMirror = false;

Prefer setting in the initializer list.

::: dom/canvas/WebGLContext.cpp:1341
(Diff revision 2)
>      if (IsContextLost())
>          return nullptr;
>  
>      if (!mResetLayer && oldLayer &&
>          oldLayer->HasUserData(aMirror ? &gWebGLMirrorLayerUserData : &gWebGLLayerUserData)) {
>          RefPtr<layers::Layer> ret = oldLayer;

[1]

::: dom/canvas/WebGLContext.cpp:1387
(Diff revision 2)
>      uint32_t flags = gl->Caps().alpha ? 0 : Layer::CONTENT_OPAQUE;
>      canvasLayer->SetContentFlags(flags);
>      canvasLayer->Updated();
>  
>      mResetLayer = false;
> +    mLayerIsMirror = aMirror;

This doesn't seem to handle the layer caching mechanism above.[1] If intentional, leave a comment there.

::: dom/canvas/WebGLContext.cpp:2360
(Diff revision 2)
> +      /**
> +       * Do not allow VR frame submission until a mirroring canvas layer has
> +       * been returned by GetCanvasLayer
> +       */
> +      return nullptr;

4-space please.

Comment 7

[:kip (Kearwood Gilbert)]

Comment on attachment 8789110 [details]
Bug 1296446 - Do not render VR frames until a WebGLContext has returned a mirroring layer

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/77346/diff/2-3/

Comment 8

[:kip (Kearwood Gilbert)]

(In reply to Jeff Gilbert [:jgilbert] from comment #6)
> Comment on attachment 8789110 [details]
> Bug 1296446 - Do not render VR frames until a WebGLContext has returned a
> mirroring layer
> 
> https://reviewboard.mozilla.org/r/77346/#review76014
> 
> ::: dom/canvas/WebGLContext.cpp:134
> (Diff revision 2)
> >      mGeneration = 0;
> >      mInvalidated = false;
> >      mCapturedFrameInvalidated = false;
> >      mShouldPresent = true;
> >      mResetLayer = true;
> > +    mLayerIsMirror = false;
> 
> Prefer setting in the initializer list.
> 
> ::: dom/canvas/WebGLContext.cpp:1341
> (Diff revision 2)
> >      if (IsContextLost())
> >          return nullptr;
> >  
> >      if (!mResetLayer && oldLayer &&
> >          oldLayer->HasUserData(aMirror ? &gWebGLMirrorLayerUserData : &gWebGLLayerUserData)) {
> >          RefPtr<layers::Layer> ret = oldLayer;
> 
> [1]
> 
> ::: dom/canvas/WebGLContext.cpp:1387
> (Diff revision 2)
> >      uint32_t flags = gl->Caps().alpha ? 0 : Layer::CONTENT_OPAQUE;
> >      canvasLayer->SetContentFlags(flags);
> >      canvasLayer->Updated();
> >  
> >      mResetLayer = false;
> > +    mLayerIsMirror = aMirror;
> 
> This doesn't seem to handle the layer caching mechanism above.[1] If
> intentional, leave a comment there.
> 
> ::: dom/canvas/WebGLContext.cpp:2360
> (Diff revision 2)
> > +      /**
> > +       * Do not allow VR frame submission until a mirroring canvas layer has
> > +       * been returned by GetCanvasLayer
> > +       */
> > +      return nullptr;
> 
> 4-space please.

Thanks for the review!

I have updated the patch with your feedback.

Comment 9

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8789110 [details]
Bug 1296446 - Do not render VR frames until a WebGLContext has returned a mirroring layer

https://reviewboard.mozilla.org/r/77346/#review76036

Comment 10

[:kip (Kearwood Gilbert)]

I'm doing a sanity check try build, then will commit this change.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1abd03e07bf5
Do not render VR frames until a WebGLContext has returned a mirroring layer r=jgilbert

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1abd03e07bf5