1038617 - B2G NFC: can't access dead object thrown from nsNfc.js

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

log from https://bugzilla.mozilla.org/show_bug.cgi?id=1034405#c3
It seems there is something wrong in notifyPeerEvent in Nfc.js or maybe the peerEventCallbackMap has problem, so in the end a closure from an previous leaked nsNfc.js is called and throw this error when the closure tried to get __DOM_IMPL__.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch

remove DEBUG.

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8459439 [details] [diff] [review]
Patch

Hi, Kyle
When mozNfc.onpeerready is assigned, registerTarget in nsNfc.js will be called.

We discovered this bug from Bug 1034405.
When Contacts app is ready to share its content, it will set onpeerready callback,
However if a mozContact is received through NFC from another device at this moment, this contact would be dispatched through MozActivity and then an Activity Window will try to show this contact and set the onpeeready callback again.

So there are two calls to set onpeeready, one is from the Contacts app, and the other is from the Activity Window.
When the callback nsINfcPeerCallback is called, the window object and __DOM_IMPL are already invalid so the error "can't access dead object" will be thrown.

So I add the check isDeadWrapper for these two objects.

Can you help to review this?

Thanks

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch. v2

Move it into a function.

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8462293 [details] [diff] [review]
Patch. v2

I have sent r? for two weeks and it seems Kyle is busy recently. :P

Change r? to smaug.

Hi Smaug
Do you have time to review this patch for me?
Or if you're busy I'll find other reviewer.

Thanks

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8462293 [details] [diff] [review]
Patch. v2

isDeadWrapper sounds a bit wrong.
Perhaps hasDeadWrapper.

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch. v3

rename to hasDeadWrapper

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/b2g-inbound/rev/447fd3d98aa4

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I think this has been landed on m-c

http://hg.mozilla.org/mozilla-central/rev/447fd3d98aa4

Tom, can we close this ?

Thank you.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/447fd3d98aa4