Closed Bug 1445994 Opened 2 years ago Closed 10 months ago

Name variable to make it clear it's keeping a reference alive

Categories

(Core :: DOM: Content Processes, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- fixed

People

(Reporter: Alex_Gaynor, Assigned: dpino, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file, 1 obsolete file)

https://searchfox.org/mozilla-central/source/dom/ipc/ContentParent.cpp#1519 it is not clear that this RefPtr is being used to keep the reference alive. We should name it |kungFuDeathGrip|, which is apparently our usual name for this.
Priority: -- → P3
Mentor: spohl.mozilla.bugs
Keywords: good-first-bug
Hey, I was wondering if this is still open? And if renaming it to kungFuDeathGrip was a joke?

-Thanks
Flags: needinfo?(spohl.mozilla.bugs)
(In reply to Rahevin from comment #1)
> Hey, I was wondering if this is still open?

The line number from comment 0 has changed, but this is still an issue:

https://dxr.mozilla.org/mozilla-central/rev/dc1868d255be744a7d2d462216be205086cc60af/dom/ipc/ContentParent.cpp#1513

We should fix all similar issues in this file, if there are any.

> And if renaming it to kungFuDeathGrip was a joke?

An explanation for this variable name was given in comment 0.
Flags: needinfo?(spohl.mozilla.bugs)
So all that needs to be done is changing the name of the variable?
I am sorry if I sound lame, I am quite new to this.
Flags: needinfo?(agaynor)
That's right.
Flags: needinfo?(agaynor)
So is there anything I can do to help? If yes, can you please guide me on how to start.
It seems rhapsodic123 has been inactive since last time showed interest in fixing this issue, so I decided to send a patch.

In the patch I renamed the spotted variable in the bug report as well as another `RefPtr` in `DelayedDeleteContentParentTask`. It seems that variable is used just for keeping a reference, so I thought is a similar case (but I'm not sure).
Attachment #9016752 - Flags: review?(spohl.mozilla.bugs)
Comment on attachment 9016752 [details] [diff] [review]
Bug-1445994-Rename-variables-to-kungFuDeathPtr-to-ma.patch

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

nit: `kungFuDeathGrip`, not `kungFuDeathPtr`.

Thank you for taking this on. This looks good to me but will need a review from someone on the IPC module owner/peers list. Sending to Jed for a review.
Attachment #9016752 - Flags: review?(spohl.mozilla.bugs)
Attachment #9016752 - Flags: review?(jld)
Attachment #9016752 - Flags: feedback+
Thanks for the review Stephen. Updated the patch replacing 'KungFuDeathPtr' for 'KungFuDeathGrip'. Updated commit message too.
Attachment #9016752 - Attachment is obsolete: true
Attachment #9016752 - Flags: review?(jld)
Attachment #9017346 - Flags: review?(jld)
Attachment #9017346 - Flags: review?(jld) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee8274a9b6c09241f0da9b6ed3b83a95b8091574
Bug 1445994: Rename variables to kungFuDeathGrip to make it clear it's keeping a reference alive. r=jld
https://hg.mozilla.org/mozilla-central/rev/ee8274a9b6c0
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Assignee: nobody → dpino
You need to log in before you can comment on or make changes to this bug.