115085 - GTKMozEmbed does not support custom directory service providers

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: GTK Widget, defect)

Description

[John Corwin]

gtkmozembed does not currently allow one to pass a custom 
nsIDirectoryServiceProvider to NS_InitEmbedding. This patch adds support for 
this feature.

Changes to gtkmozembed_internal.h:

25a26
> #include <nsXPCOM.h>
35c36
< 
---
> extern void gtk_moz_embed_set_directory_service_provider 
(nsIDirectoryServiceProvider * appFileLocProvider);

Changes to gtkmozembed2.cpp:

41a42,43
> class nsIDirectoryServiceProvider;
> 
707a710,714
> }
> 
> void
> gtk_moz_embed_set_directory_service_provider(nsIDirectoryServiceProvider * 
appFileLocProvider) {
> 	 EmbedPrivate::SetDirectoryServiceProvider

Changes to EmbedPrivate.h:
47a48
> class nsIDirectoryServiceProvider;
69a71
>   static void SetDirectoryServiceProvider (nsIDirectoryServiceProvider * 
appFileLocProvider);
135a138,139
> 
>   static nsIDirectoryServiceProvider * sAppFileLocProvider

Changes to EmbedPrivate.cpp:

74a75
> nsIDirectoryServiceProvider *EmbedPrivate::sAppFileLocProvider = nsnull;
347c348
<     rv = NS_InitEmbedding(binDir, nsnull);
---
>     rv = NS_InitEmbedding(binDir, sAppFileLocProvider);
428a430,441
> }
> 
> void 
> EmbedPrivate::SetDirectoryServiceProvider(nsIDirectoryServiceProvider * 
appFileLocProvider) 
> {
> 	 if (sAppFileLocProvider)
> 		  NS_IF_RELEASE(sAppFileLocProvider);
> 
> 	 if (appFileLocProvider) {
> 		  sAppFileLocProvider = appFileLocProvider;
> 		  NS_IF_ADDREF(sAppFileLocProvider);
> 	 }

Comment 1

[Chase Tingley]

confirm + keywords.

jcorwin, would you mind reattaching your diff using "create a new attachment"? 
thanks.

Comment 2

[John Corwin]

Attachment: gtkmozembed_internal.h.diff

Comment 3

[John Corwin]

Attachment: gtkmozembed2.cpp.diff

Comment 4

[John Corwin]

Attachment: EmbedPrivate.h.diff

Comment 5

[John Corwin]

Attachment: EmbedPrivate.cpp.diff

Comment 6

[Christopher Blizzard (:blizzard)]

Wow, I can't read that at all.  Try using 'cvs diff -u' in the
embedding/browser/gtk/src directory and you don't have to do it once for each
file.  Plus, it makes it much more readable.

Comment 7

[John Corwin]

Attachment: gtkmozembed diff

Sorry about that -- let me know if this file works.

Comment 8

[Christopher Blizzard (:blizzard)]

You don't need to use NS_IF_* in SetDirectoryServiceProvider since the checks
are already in an if() statement.  Use NS_ADDREF and NS_RELEASE instead.

Also, should we be releasing the directory service provider after it is used in
NS_InitEmbedding()?  If we don't then we've got an extra, useless handle to it
that isn't ever released.

Comment 9

[John Corwin]

Attachment: gtkmozembed diff

Yes, I believe you're right on those points.
Here's an updated patch.

Comment 10

[Christopher Blizzard (:blizzard)]

Comment on attachment 61787 [details] [diff] [review]
gtkmozembed diff

r/sr=blizzard

Comment 11

[Mike Kaply [:mkaply]]

Comment on attachment 61787 [details] [diff] [review]
gtkmozembed diff

Please use indententation consistent with the rest of the code (did you use
tabs?)

otherwise r=mkaply

Comment 12

[Christopher Blizzard (:blizzard)]

I'll fix any tabbing before I check it in.

Comment 13

[Christopher Blizzard (:blizzard)]

Checked in.