Summary: Should not call hal_sandbox::Vibrate when child process is dead → Should not call hal_sandbox::Vibrate when ipc is broken
Created attachment 679070 [details] [diff] [review] patch Hi Justin, This patch is to fix the problem that when the ipc is disconnected and content process still tries to call Vibrate/CancelVibrate. Would you please review it? If you don't have time, can you suggest someone to review it? BTW, could you suggest how to do auto test in this case? Thanks.
Attachment #679070 - Flags: review?(justin.lebar+bug)
Comment on attachment 679070 [details] [diff] [review] patch This is good, but I'd like the comments to be clearer. There's no longer an explicit call to hal_sandbox, so can you please say something like "Don't forward our ID if we're not in the sandbox, because hal_impl doesn't need it. ..." or something?
Attachment #679070 - Flags: review?(justin.lebar+bug) → feedback+
I'd like to have a look at the patch with updated comments, if you don't mind.
Created attachment 679516 [details] [diff] [review] patch v2 Justin, Please check the comment of this patch. Thanks for your suggestion.
Comment on attachment 679516 [details] [diff] [review] patch v2 > + // Don't forward our ID if we are not in the sandbox, because hal_impl > + // doesn't need it. And we don't want it to be tempted to read it. The > + // empty identifier will assert if it's used. r=me if you change "it. And" to "it, and" in the two instances you have this. Thanks for revising the comments!
Attachment #679516 - Flags: review?(justin.lebar+bug) → review+
Created attachment 679517 [details] [diff] [review] patch v3 Address comment 5.
If you think we should take this on Aurora (seems like we should), please nom your patch.
Comment on attachment 679517 [details] [diff] [review] patch v3 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 793759 User impact if declined: Content processes will crash when the ipc connection is broken and they try to call vibrate related functions. Testing completed (on m-c, etc.): no Risk to taking this patch (and alternatives if risky): low String or UUID changes made by this patch: no
Attachment #679517 - Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Attachment #679517 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox18: --- → fixed
status-firefox19: --- → fixed
You need to log in before you can comment on or make changes to this bug.