1651578 - Missing "else" in nsMozIconURI::QueryInterface()

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P4)

Description

[Alexey Izbyshev]

It seems there is missing else after https://searchfox.org/mozilla-central/rev/85ae3b91/image/decoders/icon/nsIconURI.cpp#63.

Comment 1

[Kris Taeleman (:ktaeleman)]

@Christoph: I think the code still executes as expected, but since the macros add the else it would be consistent.

Comment 2

[Alexey Izbyshev]

I'm not familiar with XPCOM and FF codebase, but without else foundInterface is always overwritten by subsequent if/else statements. So even if it's not a bug, it looks strange.

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

I don't think the missing else is a bug. Taking a closer look, the following statement:

if (aIID.Equals(kThisIconURIImplementationCID))
foundInterface = static_cast<nsIURI*>(this);
NS_INTERFACE_MAP_ENTRY(nsIMozIconURI)

When following the macro translation chain of NS_IMPL_QUERY_BODY will be translated to.

if (aIID.Equals(kThisIconURIImplementationCID))
foundInterface = static_cast<nsIURI*>(this);
if (aIID.Equals(NS_GET_IID(_interface)))
foundInterface = static_cast<_interface*>(this);
else

In other words, having an if instead of an else if will not influence the result here.

However, I realized we have a duplicate entry for NS_INTERFACE_MAP_ENTRY(nsIMozIconURI) which I think was a mistake of Bug 1583044. Since we are already on it, we can add that else statement so it's more consistent with other code in the tree, e..g within NullPrincipalURI.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1651578: Eliminate duplicate entry of nsIMozIconURI within nsIconURI and add and 'else' to be concistent with other code in the tree. r=valentin

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d0ef7321d81
Eliminate duplicate entry of nsIMozIconURI within nsIconURI and add and 'else' to be concistent with other code in the tree. r=valentin

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6d0ef7321d81