1329512 - Make IsRefcountedSmartPointer and RemoveSmartPointer const&volatile-resistant

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Currently IsRefcountedSmartPointer and RemoveSmartPointer only recognize RefPtr<T> and nsCOMPtr<T>, so if given a const and/or volatile smart pointer, it wouldn't be handled correctly.

The simplest solution would be to copy what was done for RemovePointer in mfbt/TypeTraits.h.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1329512 - Move *SmartPointer classes higher in the file -

Just a straight move, no code changes.

This will be needed in bug 1329319, as some new code higher in the file will
rely on RemoveSmartPointer. Since the next patch will pretty much rewrite
RemoveSmartPointer anyway, I thought I might as well make the big move
right now, to keep better history/blame of the upcoming changes.

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

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1329512 - Make IsRefcountedSmartPointer and RemoveSmartPointer const&volatile-resistant -

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

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8824830 [details]
Bug 1329512 - Move *SmartPointer classes higher in the file -

https://reviewboard.mozilla.org/r/103100/#review103888

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8824831 [details]
Bug 1329512 - Make IsRefcountedSmartPointer and RemoveSmartPointer const&volatile-resistant -

https://reviewboard.mozilla.org/r/103102/#review103890

Can you add `static_assert`-style tests for these in `xpcom/tests/gtest/TestThreadUtils.cpp`?

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8824830 [details]
Bug 1329512 - Move *SmartPointer classes higher in the file -

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

Comment 6

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8824831 [details]
Bug 1329512 - Make IsRefcountedSmartPointer and RemoveSmartPointer const&volatile-resistant -

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2690a46c1f7
Move *SmartPointer classes higher in the file - r=froydnj
https://hg.mozilla.org/integration/autoland/rev/db57e285593d
Make IsRefcountedSmartPointer and RemoveSmartPointer const&volatile-resistant - r=froydnj

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d2690a46c1f7
https://hg.mozilla.org/mozilla-central/rev/db57e285593d