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

Status: RESOLVED FIXED

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

Description

[Alex Gaynor [:Alex_Gaynor]]

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.

Comment 1

[Rahevin]

Hey, I was wondering if this is still open? And if renaming it to kungFuDeathGrip was a joke?

-Thanks

Comment 2

[Stephen A Pohl [:spohl]]

(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.

Comment 3

[rhapsodic123]

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.

Comment 4

[Alex Gaynor [:Alex_Gaynor]]

That's right.

Comment 5

[rhapsodic123]

So is there anything I can do to help? If yes, can you please guide me on how to start.

Comment 6

[Stephen A Pohl [:spohl]]

Here are instructions on how to build Firefox:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build#Building_Firefox_for_the_Desktop

... and here are instructions on how to write your first patch:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Let me know if you have any questions.

Comment 7

[Diego Pino]

Attachment: Bug-1445994-Rename-variables-to-kungFuDeathPtr-to-ma.patch

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).

Comment 8

[Stephen A Pohl [:spohl]]

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.

Comment 9

[Diego Pino]

Attachment: Bug-1445994-Rename-variables-to-kungFuDeathGrip-to-m.patch

Thanks for the review Stephen. Updated the patch replacing 'KungFuDeathPtr' for 'KungFuDeathGrip'. Updated commit message too.

Comment 10

[Stephen A Pohl [:spohl]]

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

Comment 11

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/ee8274a9b6c0