Clean up ContentParent cruft

RESOLVED FIXED in mozilla5



9 years ago
7 years ago


(Reporter: jdm, Assigned: jdm)


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment, 2 obsolete attachments)

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.
Posted 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-
Posted 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
Closed: 8 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.