Closed Bug 960774 Opened 6 years ago Closed 6 years ago

Require ActorDestroy methods to be implemented for parent-side actors

Categories

(Core :: IPC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: bent.mozilla, Assigned: jaas)

References

Details

Attachments

(4 files, 3 obsolete files)

ActorDestroy is currently optional in both child and parent actor implementations. We almost always need to handle crashes in the parent so I think we should just bite the bullet and start requiring it to be implemented on the parent side.
Assignee: nobody → joshmoz
Roughly the same as bug 610803 (parent-only is correct). Please dup whichever way makes more sense.
Duplicate of this bug: 610803
Attached patch fix v1.0 (obsolete) — Splinter Review
This makes ActorDestroy pure virtual on the parent side. Will attach another patch that adds ActorDestroy to everything missing it now.
This works on Linux. Will probably need a bit more work to catch everything on all platforms.
Fixes for B2G and Windows.
Attachment #8415763 - Attachment is obsolete: true
Attachment #8415762 - Flags: review?(bent.mozilla)
Attachment #8416032 - Flags: review?(bent.mozilla)
Ben asked me to post example copies of files generated with my patch.
Attached patch fix v1.1Splinter Review
Fix bug in example code generation. Don't return false from void ActorDestroy method.
Attachment #8415762 - Attachment is obsolete: true
Attachment #8415762 - Flags: review?(bent.mozilla)
Attachment #8416286 - Flags: review?(bent.mozilla)
Attachment #8416286 - Flags: review?(bent.mozilla) → review+
Comment on attachment 8416032 [details] [diff] [review]
add ActorDestroy where missing, v1.1

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

This looks fine, though I'd change the comments in each of these to point to a *new* bug. We should just file one for each component to get people to investigate whether they need to do anything in these overrides.
Attachment #8416032 - Flags: review?(bent.mozilla) → review+
Filed specific bugs for everything, this patch includes specific bug #s.
Attachment #8416032 - Attachment is obsolete: true
pushed the patch adding ActorDestroy methods to all parent-side actors that don't have one

https://hg.mozilla.org/integration/mozilla-inbound/rev/023830b8ad98
pushed patch requiring ActorDestroy on the parent side

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ece9c3282b7
You need to log in before you can comment on or make changes to this bug.