11507 - nsWalletlibService has bad refcounting and QI

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P3)

Description

[John Bandhauer]

nsWalletlibService::QueryInterface fails if given the IID for the nsISupports

That can be fixed with:

Index: nsWalletService.cpp
===================================================================
RCS file: /cvsroot/mozilla/extensions/wallet/src/nsWalletService.cpp,v
retrieving revision 1.24
diff -r1.24 nsWalletService.cpp
71c71,72
<   if (iid.Equals(kIWalletServiceIID)) {
---
>   if (iid.Equals(NS_GET_IID(nsISupports)) ||
>       iid.Equals(kIWalletServiceIID)) {


In WalletFactoryImpl::CreateInstance if the QI fails it does a 'delete inst'
(without setting 'inst' to null) and then the next line does
NS_IF_RELEASE(inst) which will crash.

Normally, the code should look like:

    rv = inst->QueryInterface(aIID, aResult);
    NS_IF_RELEASE(inst);
    return rv;

You are going to release the 'inst' anyway. However, I see that inst comes into
this with a refcount of 2 (rather than 1), so this would leak. The code sets
this object up as an observer of something else in its ctor and removes that
relationship in its dtor. So, I'm not seeing how the refcount would ever go to 0
and have the dtor get called anyway.

Comment 1

[Stephen P. Morse]

Just checked in indicated changes.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07873693ee46
[components] Issue https://github.com/mozilla-mobile/android-components/issues/11507: Use @color/mozac_ui_icons_fill as fill color for mozac_ic_web_extension_default_icon