Closed Bug 968031 Opened 6 years ago Closed 6 years ago

Remove threadsafe refcounting from ContentParent

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: khuey, Assigned: khuey)

References

Details

Attachments

(1 file)

This has been unnecessary since bug 876989.
Attached patch PatchSplinter Review
This adds ContentParent to the CC graph but doesn't actually traverse or unlink anything.  The ContentParent memory reporter AddRef/Releases every ContentParent which ensures that these show up in logs from get_about_memory.py.
Attachment #8393641 - Flags: review?(continuation)
Attachment #8393641 - Flags: review?(bent.mozilla)
Comment on attachment 8393641 [details] [diff] [review]
Patch

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

CC stuff looks good to me, modulo the explanatory comments below.

::: dom/ipc/ContentParent.cpp
@@ +1953,4 @@
>  #endif
>  }
>  
> +NS_IMPL_CYCLE_COLLECTION_0(ContentParent)

Please add a comment explaining that you are doing this to ensure that ContentParent shows up in CC logs.  This will let future readers of this code understand why you are doing this weird thing without doing version control archaeology.

::: xpcom/glue/nsCycleCollectionParticipant.h
@@ +713,4 @@
>  #define NS_IMPL_CYCLE_COLLECTION_CLASS(_class) \
>   _class::NS_CYCLE_COLLECTION_INNERCLASS _class::NS_CYCLE_COLLECTION_INNERNAME;
>  
> +#define NS_IMPL_CYCLE_COLLECTION_0(_class)                                     \

Please add back in a variant of the deleted comment mentioning that most of the time you don't actually want to make something that would use the _0 macro a cycle collected class.  IIRC, this macro used to exist, and people used it for no good reason.
Attachment #8393641 - Flags: review?(continuation) → review+
Attachment #8393641 - Flags: review?(bent.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/c35101b1c6fd
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.