Closed Bug 808983 Opened 8 years ago Closed 8 years ago
Should not call hal
_sandbox::Vibrate when ipc is broken
No description provided.
Summary: Should not call hal_sandbox::Vibrate when child process is dead → Should not call hal_sandbox::Vibrate when ipc is broken
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.
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+
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
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Attachment #679517 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.