1904160 - Add new method `GetPreferredDownloadsDirectory()` to `nsIExternalHelperAppService`

Status: RESOLVED FIXED

(Firefox :: File Handling, task, P1)

Description

[Ray Kraesig [:rkraesig]]

As discussed in D214189, refactor GetDownloadDirectory() in nsExternalHelperAppService.cpp, and expose a slice of its functionality as an XPCOM method in support of bug 1884426.

(This should also allow us to write a test for the hitherto-untested requirement that it (conditionally) has the same output as DownloadIntegration.getPreferredDownloadsDirectory().)

Comment 1

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1904160 - [1/7] Convert GetDownloadDirectory() to use Result r?Gijs

This greatly simplifies future refactorings, mostly by obviating
dir.forget(_directory).

There is one deliberate functional change: in SetDownloadToLaunch(),
we now check the error-return value of GetDownloadDirectory() and
abort if it failed, rather than proceeding.

Comment 2

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1904160 - [2/7] Simplify dataflow r?Gijs

Scope dir more tightly; return early when possible.

No functional changes.

Comment 3

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1904160 - [3/7] Collect checks into a helper function r?nika,emilio

No functional changes.

Comment 4

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1904160 - [4/7] Factor out GetOsTmpDownloadDirectory() r?Gijs

No functional changes.

Comment 5

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1904160 - [5/7] Assorted local refactoring and cleanup r?Gijs

Scope various temporary variables more tightly. Remove unneeded
conditionals.

There is one minor functional change: we no longer abort entirely if
bundleService could not be acquired, but instead fall back to just
using the unlocalized string.

Comment 6

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1904160 - [6/7] Factor out GetPreferredDownloadsDirectory() [sic] r?Gijs

Split GetDownloadDirectory into GetInitialDownloadDirectory and
GetPreferredDownloadsDirectory. (The latter is named for the function
in DownloadIntegration.sys.mjs whose behavior it should match.)

Unlike previous refactors in the patchset, when considered in isolation,
this patch doesn't really improve the code; it's just a necessary
prerequisite for exposing the function for use elsewhere.

The naming inconsistency here ("DownloadsDirectory") is intentional
to match the name of its coordinate function in DownloadIntegration.

No functional changes.

Comment 7

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1904160 - [7/7] Add method to get downloads directory r?Gijs

nsIExternalHelperAppService computes, but does not expose, the preferred
downloads directory's location. Expose it, for use by default-save-location
fallback code when the file-dialog fails.

Additionally, add a test to confirm that the value produced by this
method is, as the comment nearby implies, the same as that produced by
the eponymous method on DownloadIntegration.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f300b099684
[1/7] Convert GetDownloadDirectory() to use Result  r=Gijs
https://hg.mozilla.org/integration/autoland/rev/12b29c64833c
[2/7] Simplify dataflow  r=Gijs
https://hg.mozilla.org/integration/autoland/rev/e5af096e1328
[3/7] Collect checks into a helper function  r=nika
https://hg.mozilla.org/integration/autoland/rev/3f3f570423a5
[4/7] Factor out GetOsTmpDownloadDirectory()  r=Gijs
https://hg.mozilla.org/integration/autoland/rev/42bf1f9c360a
[5/7] Assorted local refactoring and cleanup  r=Gijs
https://hg.mozilla.org/integration/autoland/rev/6b78f7a15420
[6/7] Factor out GetPreferredDownloadsDirectory() [sic]  r=Gijs
https://hg.mozilla.org/integration/autoland/rev/1bf3d72d31d2
[7/7] Add method to get downloads directory  r=geckoview-reviewers,mak,m_kato

Comment 9

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/1f300b099684
https://hg.mozilla.org/mozilla-central/rev/12b29c64833c
https://hg.mozilla.org/mozilla-central/rev/e5af096e1328
https://hg.mozilla.org/mozilla-central/rev/3f3f570423a5
https://hg.mozilla.org/mozilla-central/rev/42bf1f9c360a
https://hg.mozilla.org/mozilla-central/rev/6b78f7a15420
https://hg.mozilla.org/mozilla-central/rev/1bf3d72d31d2