295247 - Allow extensions to ship plugin DLLs

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect, P2)

Description

[Benjamin Smedberg]

Extension should be able to ship plugin DLLs. This bug will be divided into two
patches: the first is a core patch to the directory service which allows
dirserviceprovider2.getFiles to union its results with those returned by "lower"
providers. The second are the nsXREDirProvider bits necessary to add an
<extension>/plugins location.

Jens.b, I cc'ed you on this so you could see how I add an extra extension location.

Comment 1

[Benjamin Smedberg]

Attachment: Add NS_SUCCESS_AGGREGATE_RESULT to nsIDirectoryServiceProvider2.getFiles, rev. 1

Comment 2

[Benjamin Smedberg]

Attachment: Add <xreapp>/plugins and <extension>/plugins, rev. 1

This also cleans up and unifies some of the GetFiles code for codesize, and
adds <xulappdir>/chrome.manifest to match extension packaging.

Comment 3

[Benjamin Smedberg]

Whitespace is a little screwy in the XPCOM patch, I have fixed that locally.

Comment 4

[Darin Fisher]

Comment on attachment 184327 [details] [diff] [review]
Add NS_SUCCESS_AGGREGATE_RESULT to nsIDirectoryServiceProvider2.getFiles, rev. 1

>Index: xpcom/io/nsDirectoryService.cpp

>+          nsCOMPtr<nsISimpleEnumerator> newFiles;
>+          rv = prov2->GetFiles(fileData->property, getter_AddRefs(newFiles));
>+          if (NS_SUCCEEDED(rv) && newFiles) {
>+              if (fileData->data) {
>+                  NS_NewUnionEnumerator((nsISimpleEnumerator**) &fileData->data,
>+                                        (nsISimpleEnumerator*) fileData->data, newFiles);

shouldn't you check for NS_SUCCESS_AGGREGATE_RESULT?


>+              }
>+                  
>+          	  fileData->persistent = PR_FALSE; // Enumerators can never be persistent
>+              return rv == NS_SUCCESS_AGGREGATE_RESULT;
>           }

indentation is off.  kill tabs.


>Index: xpcom/io/nsIDirectoryService.idl

>+  * @returnCode         NS_SUCCESS_AGGREGATE_RESULT if this result should be
>+  *                     aggregated with other "lower" providers.
>   */
>   nsISimpleEnumerator getFiles(in string prop);
> };

"@returnCode" -- ok.  "@throws" is obviously wrong.  hope people won't
be confused with "@returns"

Comment 5

[Benjamin Smedberg]

Attachment: Add NS_SUCCESS_AGGREGATE_RESULT to nsIDirectoryServiceProvider2.getFiles, rev. 1.1

This patch fixes the whitespace.

I shouldn't check for the NS_SUCCESS_AGGREGATE_RESULT code against a
pre-existing fileData->data because it's the *previous* provider that has
already returned NS_SUCCESS_AGGREGATE_RESULT.

Comment 6

[Darin Fisher]

> I shouldn't check for the NS_SUCCESS_AGGREGATE_RESULT code against a
> pre-existing fileData->data because it's the *previous* provider that has
> already returned NS_SUCCESS_AGGREGATE_RESULT.

OK, I see what you mean.  This is somewhat troubling.  Returning
NS_SUCCESS_AGGREGATE_RESULT is not enough to ensure that your files are
aggregated into the return value from the Directory Service query.  It only
works if you are inspected before other providers.  But, how can a provider
ensure its position relative to other providers?

Perhaps all providers should be inspected, and then merge the results of all
that return NS_SUCCESS_AGGREGATE_RESULT with the topmost one that doesn't.

Comment 7

[Benjamin Smedberg]

Well, we don't have a good way to add dirserviceproviders, since most of them
need to be added before XPCOM registration in order to be effective. It happens
that they are registered in the order

[default nsAppDirServiceProvider]
[provider passed to NS_InitXPCOM2]
[provider registered with nsIDirectoryService.registerProvider]
[more providers registered with nsIDirectoryService.registerProvider]

Very few people currently use the registerProvider method, and those do not have
a GetFiles method. In the future I plan to add a category from which providers
are read in order, but this at least meets our current needs.

Comment 8

[Darin Fisher]

I was worried not so much about startup, but rather about what happens later on.
 Maybe we can punt on the issue since getFiles being called would mean that the
"aggregate return code" would be honored given the current patch.  OK.

Comment 9

[Benjamin Smedberg]

Attachment: Add NS_SUCCESS_AGGREGATE_RESULT to nsIDirectoryServiceProvider2.getFiles, rev. 2

Whoops, my previous patch caused a leak by never releasing fileData->data when
a union enumerator was successfully created.

Comment 10

[Darin Fisher]

Comment on attachment 184352 [details] [diff] [review]
Add NS_SUCCESS_AGGREGATE_RESULT to nsIDirectoryServiceProvider2.getFiles, rev. 2

>Index: xpcom/io/nsDirectoryService.cpp

>+                  if (unionFiles)
>+                      unionFiles.swap(* (nsISimpleEnumerator**) &fileData->data);

FYI, I recall having trouble compiling something like this on Windows
with MS VC6.  I ended up making a temporary variable to get around the
problem.

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 184352 [details] [diff] [review]
Add NS_SUCCESS_AGGREGATE_RESULT to nsIDirectoryServiceProvider2.getFiles, rev. 2

a=shaver (x2)

Comment 12

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 184331 [details] [diff] [review]
Add <xreapp>/plugins and <extension>/plugins, rev. 1

a=shaver

Comment 13

[Benjamin Smedberg]

Fixed on trunk for 1.8b3