312367 - forward GI to the the wbp's listener

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Christian Persch (GNOME) (away; not receiving bug mail)]

(Setting security sensitive as precaution not to reveal the patch from bug 304286.)

My problem is similar to the one in bug 304286: I use nsWebBrowserPersist to
download something in the background and don't want those certificate dialogues
to pop up unexpectedly.

The patch in bug 304286 allows notification callbacks to provide
nsIBadCertListener. nsWebBrowserPersist::GI however only returns interfaces
which nsWebBrowserPersist implements itself, and nsI[Auth]Prompt. After
discussion with biesi on irc, I came up with the following patch:
always forward the GI to the wbp's listener, while keeping the QI for the
nsI[Auth]Prompt special case, to be backwards compatible.

Comment 1

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: proposed fix

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 199465 [details] [diff] [review]
proposed fix

r=biesi

Comment 3

[Darin Fisher]

Comment on attachment 199465 [details] [diff] [review]
proposed fix

>     if (mProgressListener && (aIID.Equals(NS_GET_IID(nsIAuthPrompt)) 
>                              || aIID.Equals(NS_GET_IID(nsIPrompt))))
>     {
>         nsCOMPtr<nsISupports> sup = do_QueryInterface(mProgressListener);
>         if (sup)
>         {
>             sup->QueryInterface(aIID, aIFace);
>             if (*aIFace)
>                 return NS_OK;
>         }
>+    }

How about this instead?  (i.e., no need to QI to nsISupports!)

	  mProgressListener->QueryInterface(aIID, aIFace);
	  if (*aIFace)
	       return NS_OK;


sr=darin with that change :)

Comment 4

[Boris Zbarsky [:bzbarsky]]

biesi, are you handling checking this in, or should I?

Comment 5

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: updated patch with change requested by sr

I'd like to get this on the 1.8 branch. It's a simple change allowing embedders
to implement more interfaces in their progress listeners and important to them
for the same reasons as bug 304286; and the patch has no risk.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Has this landed on trunk?  That needs to happen before requesting approval... 
See 1.8 tinderbox.

Comment 7

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 199527 [details] [diff] [review]
updated patch with change requested by sr

Not landed on trunk yet, no. Sorry, I didn't know about that rule. Clearing
flag.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in on trunk

Checking in embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp;
/cvsroot/mozilla/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp,v
 <--  nsWebBrowserPersist.cpp
new revision: 1.110; previous revision: 1.109
done

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #5)
> allowing embedders
> to implement more interfaces in their progress listeners and important to them
> for the same reasons as bug 304286; and the patch has no risk.

I want to mention that it's not only embeddors for whom this patch is
beneficial, extensions, too, can benefit from the patch.

Comment 10

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 199527 [details] [diff] [review]
updated patch with change requested by sr

Rerequesting a1.8rc1. Patch has no risk and is important for embedders and
extension authors (see comment 5 and comment 9).

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

fixed on MOZILLA_1_8_BRANCH:
Checking in embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp;
/cvsroot/mozilla/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp,v
 <--  nsWebBrowserPersist.cpp
new revision: 1.109.8.1; previous revision: 1.109
done

Comment 12

[Christian Persch (GNOME) (away; not receiving bug mail)]

Can this bug be made public now, please?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Daniel, see comment 12?  Should bug 304286 be public too?