1295951 - Crash in SnowWhiteKiller::Trace

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is 
report bp-5f56ebaa-0b3c-46e3-a849-92eab2160817.
=============================================================

Seen while looking at nightly crash stats: http://bit.ly/2aZoGZ6

Some part of the code crashes in dom/vr/VRDisplay.cpp:257. Adding ni on kgilbert.

Comment 1

[Andrew McCreight [:mccr8]]

This class is not properly rooting the JS objects. You need to ensure that |mozilla::HoldJSObjects(this);| has been called any time you hold JS objects (and call DropJSObjects in the dtor). The easiest way to do this is to put it in the ctor.

The class is also wrapper cached, so it will work out fine if the wrapper is preserved, but not otherwise.

Comment 2

[Andrew McCreight [:mccr8]]

I'm not entirely sure this is the cause of the crash, but it might be.

As a side note, you don't need MOZ_COUNT_CTOR/_DTOR for a refcounted class.

Comment 3

[:kip (Kearwood Gilbert)]

I am investigating now, thanks!

Comment 4

[Andrew McCreight [:mccr8]]

Ask smaug or I for help if if you need it (though I'm going to be on PTO for the rest of the morning).

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Is this likely to be nightly-only if it's VR related?

Comment 6

[:kip (Kearwood Gilbert)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #5)
> Is this likely to be nightly-only if it's VR related?

Yes, this is likely to be localized to nightly-only and for people with Oculus (VR Headset) hardware only.  The "VRDisplay.cpp" file is only present in nightly as well.

Comment 7

[:kip (Kearwood Gilbert)]

I suspect that HoldJSObjects / DropJSObjects calls would be needed for the VRPose, VRDisplayCapabilities, VREyeParameters, and VRDisplay classes, all in this file.  I'll add a patch shortly to this effect.

Comment 8

[:kip (Kearwood Gilbert)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4921e457d4b7

Comment 9

[:kip (Kearwood Gilbert)]

Attachment: Bug 1295951 - Fix JS object rooting for WebVR DOM API classes

Please advise if this is the right thing to do, or if I missed anything?  Thanks for all your help!

Comment 10

[Andrew McCreight [:mccr8]]

Yes, that looks fine to me. I'm sorry this isn't more automatic.

Comment 11

[:kip (Kearwood Gilbert)]

I'm doing a sanity test with VR hardware and a try push, then can commit if you are happy with the patch

Comment 12

[:kip (Kearwood Gilbert)]

(In reply to :kip (Kearwood Gilbert) from comment #11)
> I'm doing a sanity test with VR hardware and a try push, then can commit if
> you are happy with the patch
Changes didn't appear to affect WebVR running on real hardware.  If the patch is r+, I can commit once the try run completes.

Comment 13

[Andrew McCreight [:mccr8]]

Comment on attachment 8782132 [details] [diff] [review]
Bug 1295951 - Fix JS object rooting for WebVR DOM API classes

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

Looks good to me. Note that this will cause an extra hashtable lookup in the ctor and dtor, but hopefully that's not an issue.

::: dom/vr/VRDisplay.cpp
@@ +120,5 @@
>  {
>    return VRFieldOfViewBinding::Wrap(aCx, this, aGivenProto);
>  }
>  
> +NS_IMPL_CYCLE_COLLECTION_CLASS(VREyeParameters)

Why are all these lines showing up that don't look changed? Is it line endings or something?

Comment 14

[:kip (Kearwood Gilbert)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/69d47888c5c81bd020cbc43a1e1048e761315c8f
Bug 1295951 - Fix JS object rooting for WebVR DOM API classes,r=mccr8

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/69d47888c5c8