Closed Bug 621453 Opened 10 years ago Closed 10 years ago

Clean up ContentParent cruft

Categories

(Core :: IPC, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: jdm, Assigned: jdm)

References

Details

Attachments

(1 file, 2 obsolete files)

ContentParent has accumulated some weird bits in GetSingleton that don't really make sense to me.  OnAccelerationChange is also funky, as it attempts to get a singleton for what is presumably the current object.
Attached patch Clean up ContentParent cruft. (obsolete) — Splinter Review
Attachment #499775 - Flags: review?(benjamin)
Assignee: nobody → josh
Comment on attachment 499775 [details] [diff] [review]
Clean up ContentParent cruft.

I really hate that all the observer-adding is done in GetSingleton anyway. If you're going to refactor this, please move all that into an Init() method. That way when we have multiple content processes we don't have to touch this all yet again.
Attachment #499775 - Flags: review?(benjamin) → review-
Attached patch Clean up ContentParent cruft. (obsolete) — Splinter Review
Here's the Init() version.  I also noticed that the current thread observer mechanism will break when we move to multiple content processes, but that's not something I want to address right now.
Attachment #511038 - Flags: review?(benjamin)
Attachment #511038 - Flags: review?(benjamin) → review+
Comment on attachment 511038 [details] [diff] [review]
Clean up ContentParent cruft.

This is good cleanup of some code that's become a bit of a dumping ground.
Attachment #511038 - Flags: approval2.0?
Comment on attachment 511038 [details] [diff] [review]
Clean up ContentParent cruft.

Can you provide a risk/reward analysis? I really don't want to go taking cleanup patches at this point unless they fix some known issue.
Attachment #511038 - Flags: approval2.0?
Nevermind, there's no real need to take this right now.
Depends on: post2.0
Attachment #499775 - Attachment is obsolete: true
This patch doesn't apply cleanly on trunk anymore.
Whiteboard: not-ready
Attachment #511038 - Attachment is obsolete: true
Whiteboard: not-ready
http://hg.mozilla.org/mozilla-central/rev/f008fc60b3d4
Status: NEW → RESOLVED
Closed: 10 years ago
No longer depends on: post2.0
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
Duplicate of this bug: 639867
Blocks: 645359
Duplicate of this bug: 639865
You need to log in before you can comment on or make changes to this bug.