1291970 - Use MOZ_MUST_USE in nsMemoryReporterManager.cpp

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nicholas Nethercote [inactive]]

Let's use MOZ_MUST_USE as much as is sensible in nsMemoryReporterManager.cpp.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Use MOZ_MUST_USE in nsMemoryReporterManager.cpp

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Use MOZ_MUST_USE in nsMemoryReporterManager.cpp

(Previous patch included an unintentional change.)

Comment 3

[Eric Rahm [:erahm]]

Comment on attachment 8777634 [details] [diff] [review]
Use MOZ_MUST_USE in nsMemoryReporterManager.cpp

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

This seems fine, but I'm not sure it's have the effect you want. Either I'm guessing you need another patch that fixes all the bustage from calls to |RegisterWeakMemoryReporter| or tacking |MOZ_MUST_USE| to the definition (vs declaration) doesn't propagate outside of the file?

So if the intent is just local r+, if not f+.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Use MOZ_MUST_USE in nsMemoryReporterManager

I've kept the MOZ_MUST_USE annotations on static functions within the .cpp
file, and added some to the header. Thanks for catching the oversight in the
previous patch!

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Tweak nsCategoryManager memory reporter handling

This patch adds an assertion that makes sure that the nsCategoryManager is
destroyed after the nsMemoryReporterManager, because bad things would happen
otherwise.

Also, nsCategoryManager uses manual memory management (it's AddRef/Release are
hardwired to always return 2 and 1 respectively) so it doesn't matter if we
register it as a strong or weak memory reporter. But it's more common to use
RegisterWeakMemoryReporter when the argument is |this|, so this patch does
that.

Comment 6

[Eric Rahm [:erahm]]

Comment on attachment 8779919 [details] [diff] [review]
(part 1) - Use MOZ_MUST_USE in nsMemoryReporterManager

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

r=me

::: xpcom/base/nsIMemoryReporter.idl
@@ -472,5 @@
>  // the following functions for those components to be registered with the
>  // manager.
>  
>  typedef int64_t (*InfallibleAmountFn)();
> -typedef nsresult (*FallibleAmountFn)(int64_t* aAmount);

Is this just extra cleanup of an unused type?

Comment 7

[Eric Rahm [:erahm]]

Comment on attachment 8779920 [details] [diff] [review]
(part 2) - Tweak nsCategoryManager memory reporter handling

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

This seems fine, although I'm not sure what it has to do with this bug :)

Comment 8

[Nicholas Nethercote [inactive]]

> Is this just extra cleanup of an unused type?

Yes.

Comment 9

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6612a73409a9029aff2461b9ab9f451b4677f51d
Bug 1291970 (part 1) - Use MOZ_MUST_USE in nsMemoryReporterManager. r=erahm.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b8117f4a3659120291ecbfcc07650fed632dda02
Bug 1291970 (part 2) - Tweak nsCategoryManager memory reporter handling. r=erahm.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6612a73409a9
https://hg.mozilla.org/mozilla-central/rev/b8117f4a3659