Closed Bug 949907 Opened 10 years ago Closed 10 years ago

webrtcUI.jsm (handleRequest): can't access dead object

Categories

(Core :: WebRTC, defect)

x86_64
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox28 --- fixed
firefox29 --- fixed
firefox30 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: jruderman, Assigned: jib)

References

Details

(Keywords: testcase)

Attachments

(3 files, 2 obsolete files)

Attached file testcase
JavaScript error: resource://app/modules/webrtcUI.jsm, line 73: can't access dead object
Note that when I run the testcase, I get a different result: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIURI.host] , so it looks like there's a race here.

But going by your findings:

Looks like your testcase changes window.location to something
 nonsensical, and this asynchronous callback in webrtcUI.jsm uses the aSubject.windowId that was originally passed in in line 67, in line 73 which is a callback that happens later after the window is gone because of your location change:

67> function handleRequest(aSubject, aTopic, aData) {
68    let constraints = aSubject.getConstraints();
69 
70    Services.wm.getMostRecentWindow(null).navigator.mozGetUserMediaDevices(
71      constraints,
72      function (devices) {
73>       prompt(aSubject.windowID, aSubject.callID, constraints.audio,
74               constraints.video || constraints.picture, devices);

While we could patch the code to check for this http://stackoverflow.com/questions/18401890/know-if-an-object-is-dead I suppose the question is if there's a larger problem here of all our asynchronous callbacks being vulnerable to DOM changes in a way that could cause misbehavior or mischief.

Perhaps there's a more general fix possible to somehow make our asynchronous callbacks fall to the floor when the window is torn down. E.g. make a callback-class that checks synchronously for teardown right before calling the callback.
I suppose this problem is limited to browser-chome callbacks, since all content callbacks are gone with the old window, so perhaps checking just this instance is sufficient.
This turned out to be unrelated to Bug 919244 in the end.
Cleanup that avoids the dead object situation.
Attachment #8375840 - Flags: review?(rjesup)
Attachment #8375840 - Flags: review?(rjesup) → review+
Updated commit message. Carrying forward r+ from jesup.
Attachment #8375840 - Attachment is obsolete: true
Attachment #8375895 - Flags: review+
Fixes android bustage. Carrying forward r+ from jesup.

Try - https://tbpl.mozilla.org/?tree=Try&rev=c9faa100481d
Attachment #8375895 - Attachment is obsolete: true
Attachment #8376483 - Flags: review+
Blocks: 973318
https://hg.mozilla.org/mozilla-central/rev/3576a62195ec
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Comment on attachment 8376483 [details] [diff] [review]
GetUserMediaDevices cleanup + untie GetUserMediaRequest obj from DOM (2). r=jesup

[Approval Request Comment]
Bug caused by (feature/regressing bug #): gUM permission prompt
User impact if declined: Dead object.

Testing completed (on m-c, etc.): 
Fix verified by schien (b2g) and myself (desktop) independently. Landed on m-c a week ago.

Risk to taking this patch (and alternatives if risky): Low

String or IDL/UUID changes made by this patch: none
Attachment #8376483 - Flags: approval-mozilla-aurora?
Attachment #8376483 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8376483 [details] [diff] [review]
GetUserMediaDevices cleanup + untie GetUserMediaRequest obj from DOM (2). r=jesup

[Approval Request Comment]
Bug caused by (feature/regressing bug #): gUM permission prompt
User impact if declined: Dead object.

Testing completed (on m-c, etc.): 
Fix verified by schien (b2g) and myself (desktop) independently. Landed on m-c a week ago, aurora this morning.

Risk to taking this patch (and alternatives if risky): Low
Attachment #8376483 - Flags: approval-mozilla-beta?
Attachment #8376483 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite?
Keywords: testcase
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: