1430297 - Make NS_ADDREF & "Release()" calls consistent in nsCSSFrameConstructor.cpp

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Daniel Holbert [:dholbert]]

In Bug 1400618, we added this line:
>     item.mContent->Release();

...which is paired up with this earlier older line:
>     NS_ADDREF(item->mContent);


We should make these consistent (either both use the macro, or both *not* use the macro), to avoid confusion[1].  Based on my instincts & on bug 804206, I think it's better to move away from the macro -- so let's replace NS_ADDREF with an AddRef() call.

[1] In particular -- this confused me today because I ran across the NS_ADDREF and then was surprised that I wasn't able to find a matching NS_RELEASE(...) call.

Comment 1

[Daniel Holbert [:dholbert]]

Links for the code-quotes in comment 0: the NS_ADDREF is here:
 https://hg.mozilla.org/mozilla-central/rev/9a72b179c10d#l2.11
...and the Release() is here:
 https://hg.mozilla.org/mozilla-central/rev/9a72b179c10d#l2.75

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: Bug 1430297: Use AddRef() instead of NS_ADDREF (when keeping generated content alive in nsCSSFrameConstructor).

Review commit: https://reviewboard.mozilla.org/r/212652/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212652/

Comment 3

[Mats Palmgren (inactive)]

Comment on attachment 8942365 [details]
Bug 1430297: Use AddRef() instead of NS_ADDREF (when keeping generated content alive in nsCSSFrameConstructor).

https://reviewboard.mozilla.org/r/212652/#review218356

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 8942365 [details]
Bug 1430297: Use AddRef() instead of NS_ADDREF (when keeping generated content alive in nsCSSFrameConstructor).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/212652/diff/1-2/

Comment 5

[Daniel Holbert [:dholbert]]

Thanks for the review!

Upon reflection, I realized that my first commit message was ambiguous (my wording "...to keep generated content alive..." sounded a bit like *this changeset* was making us start to keep stuff alive).

I changed to instead say "(when keeping...)" which I think is a bit of a clearer modifier.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c5094851201b
Use AddRef() instead of NS_ADDREF (when keeping generated content alive in nsCSSFrameConstructor). r=mats

Comment 7

[Cristina Coroiu [:ccoroiu]]

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