337324 - move Firefox-specific microsummary directory defs to nsBrowserDirectoryProvider

Status: RESOLVED FIXED

(Firefox Graveyard :: Microsummaries, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Microsummary directories are defined in nsDirectoryServiceDefs, but on IRC bsmedberg says "we have nsBrowserDirectoryProvider for firefox-specific keys,and the #define should be in a firefox-specific place, either in some nsMicroSummaries.h or nsBrowserCompsCID.h".

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: moves defs to nsBrowserDirectoryProvider

This patch moves the microsummary directories to nsBrowserDirectoryProvider.  For filename consistency with nsDirectoryServiceDefs.h and nsAppDirectoryServiceDefs.h, I put the microsummary defines into the new file nsBrowserDirectoryServiceDefs.h.

I also copied nsAppFileLocationProvider::CloneMozBinDirectory() into nsBrowserDirectoryProvider.  And I fixed a hardcoded reference to the UsrMicsumDefs directory in nsMicrosummaryService.js.in.

Do I still need to provide a Mac OS 9-specific (i.e. XP_MAC) name for the microsummary generators directory, given that Firefox no longer supports that OS?

Comment 2

[Benjamin Smedberg]

Attachment: patch v2

myk, I don't think you want the copying-from-defaults behavior of the lower section of browserdirectory, so I remade your patch to shortcut everything in the first set of if/else blocks. You don't need CloneMozBinDirectory, you can safely just use NS_XPCOM_CURRENT_PROCESS_DIR

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3

> myk, I don't think you want the copying-from-defaults behavior of the lower
> section of browserdirectory, so I remade your patch to shortcut everything in
> the first set of if/else blocks. You don't need CloneMozBinDirectory, you can
> safely just use NS_XPCOM_CURRENT_PROCESS_DIR

Yeah, that all seems right to me and has the corollary benefit of making the logic simpler.

This patch looks good, except that we probably don't need to do #if defined(XP_MAC), since browser doesn't support Mac OS 9-, and it almost certainly never will.  Also, we don't need to #include nsILocalFile.h now that we aren't adding CloneMozBinDirectory.

Here's an updated patch with these changes.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Fix checked in to trunk and 1.8 branch.