804827 - can't access dead object in forms.js line 49

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

I can't exactly reproduce this but saw this in some logs. If the focusedElement is set and its document goes away in the meantime (crash or close?), the wrapper will be dead and this will yield an error on next access.

Requesting blocking+ as this might render any input element interaction and other stuff impossible until the device is rebooted.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: check if the wrapper is dead before accessing it

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Attachment: check if the wrapper is dead before accessing it

Small correction.

Comment 3

[David Flanagan [:djf]]

Comment on attachment 674439 [details] [diff] [review]
check if the wrapper is dead before accessing it

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

Nice catch. I can see how this.focusedElement would have a longer lifetime than the element to which it refers.  I have no experience with Cu.isDeadWrapper(), but it sounds like this is exactly what you need to detect that issue.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Thanks for the review! Just to reassure you:

http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/idl/xpccomponents.idl#319

We also use it here already:

http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementParent.js

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Waiting for an approval-mozilla-aurora flag.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 674439 [details] [diff] [review]
check if the wrapper is dead before accessing it

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: This might render any input element interaction and other stuff impossible until the device is rebooted.
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): Small, self-contained patch that adds error checking. Low risk.
String or UUID changes made by this patch: None

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 674439 [details] [diff] [review]
check if the wrapper is dead before accessing it

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

Make sure to mark as status-firefox18:fixed when landing

Comment 8

[Andrew Overholt [:overholt]]

Fabrice says we should take this fix but not block on it.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/6c5e1a6e2e71
https://hg.mozilla.org/releases/mozilla-aurora/rev/5d42b4fd95e6

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Had to push a little bustage fix to not pass null values to Cu.isDeadWrapper():

https://hg.mozilla.org/integration/fx-team/rev/a60c5629e19b
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc78bc1f81af

Comment 11

[[:fabrice] Fabrice Desré]

Tim, we usually push to inbound for b2g, not to fx-team.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

I know but I don't have an inbound checkout. I usually push to fx-team and merge to m-c from there. Shouldn't make a difference.

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/6c5e1a6e2e71
https://hg.mozilla.org/mozilla-central/rev/a60c5629e19b